Re: [PATCH] drm: logicvc: fix device_node refcount leak in logicvc_layers_init()

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