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

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

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.

> +	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.

Anyway, these are just a few nits.

Reviewed-by: Maíra Canal <mcanal@igalia.com>

Best regards,
- Maíra