Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] The patch introduces a permanent, unrecoverable memory leak of the MST 
branch device by clearing the `mgr->mst_primary` pointer without decrementing 
its reference count.
--

commit ecf0dbf046159857ec56d411e54fc1853de6778c
Author: WenTao Liang <[email protected]>

fix: drm/display: drm_dp_mst_topology_mgr_set_mst: error path after DPCD write 
failure leaks MST branch device reference

This commit attempts to fix an issue in the DPCD write failure path of 
drm_dp_mst_topology_mgr_set_mst() by clearing the mgr->mst_primary pointer 
before unlocking.

> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c 
> b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> --- 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;
>                       goto out_unlock;
> +             }

[Severity: High]
Does clearing this pointer without dropping its reference permanently leak 
the MST branch device?

When drm_dp_add_mst_branch_device() returns mstb earlier in this function, 
it has an initial topology_kref of 1.

Shortly after, drm_dp_mst_topology_get_mstb(mgr->mst_primary) increments the 
reference count to 2.

If drm_dp_dpcd_write_byte() fails, this patch sets mgr->mst_primary = NULL 
and jumps to out_unlock.

At out_unlock, drm_dp_mst_topology_put_mstb(mstb) is called once, 
decrementing the reference count to 1.

Because mgr->mst_primary was set to NULL, future calls to disable MST 
(such as drm_dp_mst_topology_mgr_set_mst(..., false)) won't be able to 
find the primary branch device to clean up the final reference, which means 
the struct drm_dp_mst_branch object will be permanently leaked.

>  
>               /* Write reset payload */
>               drm_dp_dpcd_clear_payload(mgr->aux);

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to