Re: [PATCH 1/3] drm/display: hdmi: Prevent NULL pointer dereference in sync_scdc()

[Cristian Ciocaltea <cristian.ciocaltea@collabora.com>]

Hi Maíra,

On 11/9/26 2:00 PM, Maíra Canal wrote:
> Hi Cristian,
> 
> On 04/11/26 10:26, Cristian Ciocaltea wrote:
>> The connector pointer is used to initialize the local variables hdmi and
>> dev before the NULL check takes place, leading to the following Smatch
>> complaint:
>>
>>    drivers/gpu/drm/display/drm_hdmi_helper.c:663 drm_connector_hdmi_sync_scdc()
>>    warn: variable dereferenced before check 'connector' (see line 657)
>>
>> Move those initializations after the sanity check to avoid a potential
>> NULL pointer dereference.
>>
>> Fixes: d653818324ed ("drm/display: hdmi: Add HDMI 2.0 scrambling management helpers")
>> Reported-by: Dan Carpenter <error27@gmail.com>
>> Closes: https://lore.kernel.org/all/arJo7A7L5hud3tyc@stanley.mountain/
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>>   drivers/gpu/drm/display/drm_hdmi_helper.c | 7 +++++--
>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/display/drm_hdmi_helper.c b/drivers/gpu/drm/display/drm_hdmi_helper.c
>> index f85cce3d6254..872f7e32e8d2 100644
>> --- a/drivers/gpu/drm/display/drm_hdmi_helper.c
>> +++ b/drivers/gpu/drm/display/drm_hdmi_helper.c
>> @@ -653,9 +653,9 @@ EXPORT_SYMBOL(drm_connector_hdmi_disable_scrambling);
>>   int drm_connector_hdmi_sync_scdc(struct drm_connector *connector, bool plugged,
>>                    struct drm_modeset_acquire_ctx *ctx)
>>   {
>> -    struct drm_connector_hdmi *hdmi = &connector->hdmi;
>> -    struct drm_device *dev = connector->dev;
>> +    struct drm_connector_hdmi *hdmi;
>>       struct drm_connector_state *conn_state;
>> +    struct drm_device *dev;
>>       struct drm_crtc *crtc;
>>       u8 config;
>>       int ret;
>> @@ -663,6 +663,9 @@ int drm_connector_hdmi_sync_scdc(struct drm_connector *connector, bool plugged,
>>       if (!connector || !plugged || !ctx)
>>           return 0;
>>   +    hdmi = &connector->hdmi;
> 
> hdmi is only used once (and you are going to delete it in the next
> commit), so personally, I would prefer to delete this variable in this
> commit and use connector->hdmi directly when needed.

Ack.

> 
>> +    dev = connector->dev;
>> +
>>       /*
>>        * This is normally part of .detect_ctx() call path, which already holds
>>        * connection_mutex through @ctx.  However, re-acquiring it with the
>>
> 
> drm_modeset_lock() here could use the variable dev instead of
> &connector->dev.

Well spotted, I missed it.

> 
> Anyway, these are just a few nits.
> 
> Reviewed-by: Maíra Canal <mcanal@igalia.com>

Thank you for reviewing the series.  Please let me know if you'd still like me to
send a v2 to address your findings in this patch.

Kind regards,
Cristian