Re: [PATCH v8 2/5] drm/scdc-helper: Add scdc_status debugfs entry

[Nicolas Frattaroli <nicolas.frattaroli@collabora.com>]

Hi Maxime,

On Wednesday, 22 July 2026 11:27:25 Central European Summer Time sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The function `scdc_status_show` leaks `connector->dev->mode_config.mutex` on the disconnected error path.
> --
> 
> --- Patch [2]: [PATCH v8 2/5] drm/scdc-helper: Add scdc_status debugfs entry ---
> commit 524b3064a7221dcef8708b2269e2ccaa3ede6629
> Author: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> 
> drm/scdc-helper: Add scdc_status debugfs entry
> 
> This commit adds a debugfs entry under the connector to expose SCDC status
> information. It introduces drm_scdc_read_state to read and parse the SCDC
> registers and a debugfs init function for connectors to use.
> 
> > diff --git a/drivers/gpu/drm/display/drm_scdc_helper.c b/drivers/gpu/drm/display/drm_scdc_helper.c
> > index 8403f2390ab6a..97256b272329a 100644
> > --- a/drivers/gpu/drm/display/drm_scdc_helper.c
> > +++ b/drivers/gpu/drm/display/drm_scdc_helper.c
> [ ... ]
> > @@ -276,3 +278,187 @@ bool drm_scdc_set_high_tmds_clock_ratio(struct drm_connector *connector,
> [ ... ]
> > +static int scdc_status_show(struct seq_file *m, void *data)
> > +{
> > +	struct drm_connector *connector = m->private;
> > +	struct drm_scdc *scdc = &connector->display_info.hdmi.scdc;
> > +	struct drm_scdc_state *st;
> > +	int i, ret;
> > +
> > +	drm_connector_get(connector);
> > +
> > +	ret = mutex_lock_interruptible(&connector->dev->mode_config.mutex);
> > +	if (ret)
> > +		goto err_conn_put;
> > +
> > +	if (connector->status != connector_status_connected) {
> > +		ret = -ENODEV;
> > +		goto err_conn_put;
> 
> [Severity: High]
> Does this error path leak connector->dev->mode_config.mutex?
> 
> The mutex is successfully acquired just above this check. If the connector
> is not connected, jumping directly to err_conn_put bypasses the err_unlock
> label where the mutex is supposed to be released.

This one is real, so I'll need to send a v9 to go to the right label here.

Feel free to provide any feedback you've got on top of that problem, so
that I don't needlessly spam the list with a 1-line-oopsie every day.

Kind regards,
Nicolas Frattaroli

> > +	}
> > +
> > +	if (scdc->supported) {
> [ ... ]
> > +
> > +err_free_state:
> > +	kfree(st);
> > +err_unlock:
> > +	mutex_unlock(&connector->dev->mode_config.mutex);
> > +err_conn_put:
> > +	drm_connector_put(connector);
> > +
> > +	return ret;
> > +}
> 
>