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

["Maíra Canal" <mcanal@igalia.com>]

On 09/11/26 09:39, Cristian Ciocaltea wrote:
> 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.
> 

These are very small nits, not worth a v2. Feel free to push the patches
to drm-misc-fixes.

Best regards,
- Maíra

> Kind regards,
> Cristian