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

Pre-existing issues:
- [High] DRM mode objects allocated with devm_kzalloc are freed before 
drmm_mode_config_cleanup() executes, causing a use-after-free during teardown.
--

--- Patch [1]: [PATCH] drm: logicvc: fix device_node refcount leak in 
logicvc_layers_init() ---
commit 287cb4b6b0c0c79eff11b1f6730b1461931e959d
Author: Manush Prajwal <[email protected]>

drm: logicvc: fix device_node refcount leak in logicvc_layers_init()

On logicvc_layer_init() failure, logicvc_layers_init() releases the
parent layers_node reference before jumping to the error path, but
never releases layer_node, the child node held by the current
for_each_child_of_node() iteration. Add the missing of_node_put()
for layer_node alongside the existing one for layers_node.

> diff --git a/drivers/gpu/drm/logicvc/logicvc_layer.c 
> b/drivers/gpu/drm/logicvc/logicvc_layer.c
> index 7f5ba000188ab..e83a2aafed779 100644
> --- a/drivers/gpu/drm/logicvc/logicvc_layer.c
> +++ b/drivers/gpu/drm/logicvc/logicvc_layer.c
> @@ -613,6 +613,7 @@ int logicvc_layers_init(struct logicvc_drm *logicvc)
>  
>               ret = logicvc_layer_init(logicvc, layer_node, index);
>               if (ret) {
> +                     of_node_put(layer_node);
>                       of_node_put(layers_node);
>                       goto error;

[Severity: High]
This isn't a bug introduced by this patch, but does taking this error path
lead to a use-after-free during driver teardown?

When we jump out of the loop to the error label in logicvc_layers_init(),
logicvc_layer_fini() is called, which frees the layer memory using
devm_kfree().

However, in drivers/gpu/drm/logicvc/logicvc_layer.c, logicvc_layer_init()
allocates the layer with devm_kzalloc() and then registers an unmanaged
drm_plane using drm_universal_plane_init():

        layer = devm_kzalloc(dev, sizeof(*layer), GFP_KERNEL);
        ...
        ret = drm_universal_plane_init(drm_dev, &layer->drm_plane, 0,
                                       &logicvc_plane_funcs, formats->formats,
                                       formats_count, NULL, type, NULL);

Because the plane is not unregistered before devm_kfree() is called, does this
leave a dangling plane pointer in the DRM core list?

Additionally, when devres unwinds during a probe failure or driver unbind,
won't it release the devm_kzalloc() memory before the drmm teardown action
calls drm_mode_config_cleanup() (since drmm_mode_config_init() was used in
logicvc_drm.c:logicvc_drm_probe())?

If so, drm_mode_config_cleanup() would iterate over the freed plane and call
its ->funcs->destroy callback, triggering a use-after-free.

A similar devm_kzalloc() and unmanaged DRM object lifecycle mismatch appears
to exist for logicvc_crtc_init() in drivers/gpu/drm/logicvc/logicvc_crtc.c
and logicvc_interface_init() in drivers/gpu/drm/logicvc/logicvc_interface.c.

>               }

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

Reply via email to