Re: [PATCH v6 18/29] drm/msm/dp: introduce dp_mst_drm module

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing teardown for DRM MST topology manager via drm_dp_mst_topology_mgr_destroy(), leading to leaked internal resources (workqueues, mutexes, payload arrays) and a Use-After-Free vulnerability.
--

commit 27699c490812d953e1c2e18da505fffee4442084
Author: Yongxing Mou <yongxing.mou@oss.qualcomm.com>

drm/msm/dp: introduce dp_mst_drm module

This commit introduces the core MST manager data structures and the
initialization path for creating a DRM MST topology manager associated
with a DP controller. It also adds the registration hooks used to
initialize the MST manager during DP device setup.

> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
[ ... ]
> @@ -678,6 +678,16 @@ static int _dpu_kms_initialize_displayport(struct drm_device *dev,
>  			DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc);
>  			return rc;
>  		}
> +
> +		stream_cnt = msm_dp_get_mst_max_stream(priv->kms->dp[i]);
> +
> +		if (stream_cnt > 1) {
> +			rc = msm_dp_mst_register(priv->kms->dp[i]);
> +			if (rc) {
> +				DPU_ERROR("dp_mst_init failed for DP, rc = %d\n", rc);
> +				return rc;

[Severity: High]
If `msm_dp_mst_register()` fails on a subsequent DP port after successfully
registering a prior one in this loop, does returning early here leak the
successfully initialized MST managers from the previous iterations?

> +			}
> +		}
>  	}
>  
>  	return 0;

[ ... ]
> diff --git a/drivers/gpu/drm/msm/dp/dp_mst_drm.c b/drivers/gpu/drm/msm/dp/dp_mst_drm.c
[ ... ]
> @@ -0,0 +1,54 @@
[ ... ]
> +int msm_dp_mst_mgr_init(struct msm_dp *dp_display, u32 max_streams, struct drm_dp_aux *drm_aux)
> +{
> +	struct drm_device *dev = dp_display->drm_dev;
> +	struct msm_dp_mst *mst;
> +	int ret;
> +
> +	mst = devm_kzalloc(dev->dev, sizeof(*mst), GFP_KERNEL);
> +	if (!mst)
> +		return -ENOMEM;
> +
> +	mst->msm_dp = dp_display;
> +	mst->max_streams = max_streams;
> +	mst->dp_aux = drm_aux;
> +
> +	ret = drm_dp_mst_topology_mgr_init(&mst->mst_mgr, dev,
> +					   drm_aux,
> +					   16,
> +					   max_streams,
> +					   dp_display->connector->base.id);

[Severity: High]
Does this leak the MST topology manager resources?

`drm_dp_mst_topology_mgr_init()` creates non-devres-managed resources like
workqueues (`delayed_destroy_wq`) and background worker threads. Since `mst`
is allocated with `devm_kzalloc()`, it will be freed automatically upon
device unbind or probe failure.

Without a paired teardown action (e.g. via `devm_add_action_or_reset()`
calling `drm_dp_mst_topology_mgr_destroy()`), won't the active workqueues
and threads be left running?

Can this lead to a Use-After-Free when those background threads later
dereference the freed `mst` memory?

> +	if (ret) {
> +		drm_err(dev, "[MST] topology manager init failed\n");
> +		return ret;
> +	}
> +
> +	dp_display->msm_dp_mst = mst;
> +	return 0;
> +}
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-msm-dp-mst-v6-0-c91d35d6fb9e@oss.qualcomm.com?part=18