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

[Marek Vasut]

On 1/21/22 14:24, Alexander Stein wrote:
> 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?

I'm sorry, I missed the lvds-codec part. Laurent is already on CC.

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

That line ^ should be in the commit message.