Re: [RFC PATCH v2 2/2] drm/bridge: cdns-mhdp8546: exclude producers before teardown

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] drm/bridge: cdns-mhdp8546: unvalidated connector in modeset
  retry work
--

commit 2451c0294329178177f8dd09e32efe0d3bf442ae
Author: Hongyan Xu <getshell@seu.edu.cn>

drm/bridge: cdns-mhdp8546: exclude producers before teardown

This commit guards bridge callbacks and work functions with
drm_bridge_enter() and drm_bridge_exit(), synchronizes teardown with
drm_bridge_unplug(), and serializes connector references in modeset retry
and HDCP property work items.

Link: https://lore.kernel.org/all/20260728135214.1850-1-getshell@seu.edu.cn/

> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> index f47717bd10798..6c108090c2caa 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
[ ... ]
> @@ -2172,14 +2334,20 @@ static int cdns_mhdp_update_link_status(struct cdns_mhdp_device *mhdp)
>  	return ret;
>  }
>  
> -static void cdns_mhdp_modeset_retry_fn(struct work_struct *work)
> +static void cdns_mhdp_modeset_retry_fn_active(struct work_struct *work)
>  {
>  	struct cdns_mhdp_device *mhdp;
>  	struct drm_connector *conn;
>  
>  	mhdp = container_of(work, typeof(*mhdp), modeset_retry_work);
>  
> +	mutex_lock(&mhdp->link_mutex);
>  	conn = mhdp->connector;
> +	if (conn)
> +		drm_connector_get(conn);
> +	mutex_unlock(&mhdp->link_mutex);
> +	if (!conn)
> +		return;
>  
>  	/* Grab the locks before changing connector property */
>  	mutex_lock(&conn->dev->mode_config.mutex);
> @@ -2192,7 +2360,21 @@ static void cdns_mhdp_modeset_retry_fn(struct work_struct *work)
>  	mutex_unlock(&conn->dev->mode_config.mutex);
>  
>  	/* Send Hotplug uevent so userspace can reprobe */
> -	drm_kms_helper_hotplug_event(mhdp->bridge.dev);
> +	drm_kms_helper_hotplug_event(conn->dev);
> +	drm_connector_put(conn);
> +}

[Severity: Medium]
Can cdns_mhdp_modeset_retry_fn_active() update a stale connector after
dropping mhdp->link_mutex?

CPU0                                  CPU1
cdns_mhdp_modeset_retry_fn_active()
  mutex_lock(&mhdp->link_mutex)
  conn = mhdp->connector
  drm_connector_get(conn)
  mutex_unlock(&mhdp->link_mutex)
                                      cdns_mhdp_atomic_disable_active()
                                        mutex_lock(&mhdp->link_mutex)
                                        ...
                                        mhdp->connector = NULL
                                        mutex_unlock(&mhdp->link_mutex)
  mutex_lock(&conn->dev->mode_config.mutex)
  drm_connector_set_link_status_property(conn,
      DRM_MODE_LINK_STATUS_BAD)       <-- updates detached connector
  mutex_unlock(&conn->dev->mode_config.mutex)
  drm_kms_helper_hotplug_event(conn->dev)

Unlike cdns_mhdp_hdcp_prop_work_active(), which checks conn->dev and
re-acquires mhdp->link_mutex under connection_mutex to verify
mhdp->connector == conn && conn->state, cdns_mhdp_modeset_retry_fn_active()
never revalidates mhdp->connector before setting link_status to BAD and
sending a hotplug uevent (or after a subsequent modeset has already
recovered the link).  If drm_connector_cleanup() also runs after
mhdp->connector is cleared, conn->dev or conn->state can be NULL when
dereferenced.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/179474328603.103840.4363768155390012601@seu.edu.cn?part=2