Re: [PATCH v2] drm/display: fix MST branch device refcount leak on DPCD write failure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Permanent memory leak of the initial `drm_dp_mst_branch` device on DPCD write failure.
- [Medium] Inconsistent MST state leading to a kernel warning in `drm_dp_mst_topology_queue_probe()` and silent functional breakage.
--

commit 169a81a07028ce3444674cdec29dcdd6268008f7
Author: WenTao Liang <vulab@iscas.ac.cn>

    drm/display: fix MST branch device refcount leak on DPCD write failure
    
    drm_dp_add_mst_branch_device initializes mstb with refcount 1, and
    drm_dp_mst_topology_get_mstb increments it to 2. When
    drm_dp_dpcd_write_byte fails, out_unlock performs only one
    drm_dp_mst_topology_put_mstb, leaving the other reference stored in
    mgr->mst_primary. Since MST was not successfully enabled, no disable path
    will clean it up.
    
    Suggested-by: Greg KH <gregkh@linuxfoundation.org>
    Fixes: 7a3cbf590e63 ("drm/mst: Some style improvements in drm_dp_mst_topology_mgr_set_mst()")
    Cc: stable@vger.kernel.org
    Signed-off-by: WenTao Liang <vulab@iscas.ac.cn>

> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> index 4de36fda05440..6a8c9dc217e53 100644
> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> @@ -3681,8 +3681,10 @@ int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr, bool ms
>  					     DP_MST_EN |
>  					     DP_UP_REQ_EN |
>  					     DP_UPSTREAM_IS_SRC);
> -		if (ret < 0)
> +		if (ret < 0) {
> +			mgr->mst_primary = NULL;

[Severity: Medium]
Does clearing mgr->mst_primary here cause a permanent memory leak?

Since drm_dp_mst_topology_get_mstb() previously incremented the refcount to 2, 
clearing this pointer without first calling drm_dp_mst_topology_put_mstb()
leaves one reference orphaned. When the code jumps to out_unlock, it will drop
the remaining reference (down to 1), but the memory will never be freed.

Furthermore, any subsequent cleanup calls to 
drm_dp_mst_topology_mgr_set_mst(..., false) will also fail to clean it up
because they expect to find the pointer in mgr->mst_primary.

[Severity: Medium]
Does this also leave the manager in an inconsistent state?

Earlier in drm_dp_mst_topology_mgr_set_mst(), mgr->mst_state is set to true.
If the DPCD write fails and we clear mgr->mst_primary without reverting
mgr->mst_state to false, we end up with mst_state == true but
mst_primary == NULL.

If a driver later calls drm_dp_mst_topology_queue_probe(), would it trigger
this warning due to the mismatched state?

    if (drm_WARN_ON(mgr->dev, !mgr->mst_state || !mgr->mst_primary))

Additionally, it seems future attempts to enable MST would silently return
success because the early check (mst_state == mgr->mst_state) would evaluate
to true without configuring the hardware.

>  			goto out_unlock;
> +		}
>  
>  		/* Write reset payload */
>  		drm_dp_dpcd_clear_payload(mgr->aux);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260628133344.46188-1-vulab@iscas.ac.cn?part=1