(EXT) Re: [PATCH 1/1] drm: mxsfb: Fix NULL pointer dereference

[Alexander Stein]

Am Freitag, 21. Januar 2022, 14:14:01 CET schrieb Marek Vasut:
> On 1/21/22 14:12, Alexander Stein wrote:
> > Do not deference the NULL pointer if the bridge does not return a
> > bridge state. Assume a fixed format instead.
> > 
> > Fixes: commit b776b0f00f24 ("drm: mxsfb: Use bus_format from the nearest
> > bridge if present") Signed-off-by: Alexander Stein
> > <alexander.stein at ew.tq-group.com>
> > ---
> > This can happen if a "ti,sn75lvds83", "lvds-encoder" bridge is attached
> > to it. atomic_get_input_bus_fmts is only implemented for the
> > lvds-decoder case.
> > 
> >   drivers/gpu/drm/mxsfb/mxsfb_kms.c | 6 +++++-
> >   1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> > b/drivers/gpu/drm/mxsfb/mxsfb_kms.c index 0655582ae8ed..4cfb6c001679
> > 100644
> > --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> > +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
> > @@ -361,7 +361,11 @@ static void mxsfb_crtc_atomic_enable(struct drm_crtc
> > *crtc,> 
> >   		bridge_state =
> >   		
> >   			drm_atomic_get_new_bridge_state(state,
> >   			
> >   							
mxsfb->bridge);
> > 
> > -		bus_format = bridge_state->input_bus_cfg.format;
> > +		if (!bridge_state)
> > +			bus_format = MEDIA_BUS_FMT_FIXED;
> > +		else
> > +			bus_format = bridge_state-
>input_bus_cfg.format;
> > +
> > 
> >   		if (bus_format == MEDIA_BUS_FMT_FIXED) {
> >   		
> >   			dev_warn_once(drm->dev,
> >   			
> >   				      "Bridge does not provide bus 
format, assuming
> >   				      MEDIA_BUS_FMT_RGB888_1X24.
\n"
> 
> Shouldn't this be fixed on the bridge driver side instead ?
> 
> Which bridge driver do you use ?

It's drivers/gpu/drm/bridge/lvds-codec.c. I thought naming the compatibles 
would suffice. I consider a patch for the bridge driver as a separate issue, 
hence the warning from mxsfb. Although I'm unsure how/what to implement. 
Similar to the encode case where the bus format is specified in DT? 

Anyway, mxsfb should not never dereference the NULL pointer which 
drm_atomic_get_new_bridge_state is allowed to return.

Best regards,
Alexander