Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the patch implements 
an incomplete and structurally flawed fix. It fixes an `of_node` reference leak 
on the `device_register()` error path by manually adding `of_node_put(np)`, but 
fails to fix the exact same leak on the success path during normal device 
teardown.
--

commit cae4b5af9cb7c08097233a2381f0d6210847f798
Author: Wentao Liang <[email protected]>

drm/display: fix of_node refcount leak in of_dp_aux_populate_bus()
    
This commit attempts to fix an of_node reference leak on the device_register()
error path by adding a manual of_node_put() after put_device().

> diff --git a/drivers/gpu/drm/display/drm_dp_aux_bus.c 
> b/drivers/gpu/drm/display/drm_dp_aux_bus.c
> index 6e5e3e5422905..9dd64cf25d871 100644
> --- a/drivers/gpu/drm/display/drm_dp_aux_bus.c
> +++ b/drivers/gpu/drm/display/drm_dp_aux_bus.c
> @@ -305,6 +305,7 @@ int of_dp_aux_populate_bus(struct drm_dp_aux *aux,
>                * of kfree() directly for error cases.
>                */
>               put_device(&aux_ep->dev);
> +             of_node_put(np);
>  
>               goto err_did_set_populated;
>       }

[Severity: High]
This isn't a bug introduced by this patch, but the fix appears to be
incomplete and could leave the leak unfixed on the success path.

The underlying issue seems to be that the device's .release callback doesn't
drop the dev->of_node reference:

drivers/gpu/drm/display/drm_dp_aux_bus.c:dp_aux_ep_dev_release() {
        ...
        kfree(aux_ep_with_data);
        // Missing of_node_put(dev->of_node) here
}

By fixing the leak locally here on the error path in of_dp_aux_populate_bus(),
doesn't the success path still leak the second reference when the device is
later unregistered?

drivers/gpu/drm/display/drm_dp_aux_bus.c:of_dp_aux_ep_destroy() {
        ...
        of_node_put(np); // Drops the first reference

        device_unregister(dev); // Calls release callback, missing second drop
        ...
}

If the missing of_node_put() is instead added to dp_aux_ep_dev_release(),
would it resolve the leak for both the success and error paths?

It looks like the current approach might also create a trap where fixing the
.release callback in the future could trigger a double-free on the error path
due to this manual of_node_put().

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

Reply via email to