dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/logicvc: fix device_node refcount leak in logicvc_layers_init()
@ 2026-08-26  6:54 Qingshuang Fu
  2026-08-26  7:09 ` sashiko-bot
  0 siblings, 1 reply; 3+ messages in thread
From: Qingshuang Fu @ 2026-08-26  6:54 UTC (permalink / raw)
  To: Paul Kocialkowski, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Andy Whitcroft,
	Joe Perches
  Cc: dri-devel, linux-kernel, Qingshuang Fu, Qingshuang Fu

From: Qingshuang Fu <fuqingshuang@kylinos.cn>

for_each_child_of_node() holds a reference to layer_node for the
duration of each iteration and only releases the previous node when
advancing to the next one.  When logicvc_layer_init() fails, the loop
exits via the error label without ever advancing, so the reference to
layer_node is never dropped and the device node leaks.  Add the missing
of_node_put() on the error path.

Fixes: efeeaefe9be5 ("drm: Add support for the LogiCVC display controller")
Signed-off-by: Qingshuang Fu <fuqingshuang@kylinos.cn>
---
 drivers/gpu/drm/logicvc/logicvc_layer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/logicvc/logicvc_layer.c b/drivers/gpu/drm/logicvc/logicvc_layer.c
index 7f5ba000188a..e83a2aafed77 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;
 		}

base-commit: 45c13f3f9e3bb15fd89ff2864c6f627a3b4b4229
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/logicvc: fix device_node refcount leak in logicvc_layers_init()
  2026-08-26  6:54 [PATCH] drm/logicvc: fix device_node refcount leak in logicvc_layers_init() Qingshuang Fu
@ 2026-08-26  7:09 ` sashiko-bot
  2026-08-28  2:54   ` Qingshuang Fu
  0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-08-26  7:09 UTC (permalink / raw)
  To: Qingshuang Fu; +Cc: dri-devel

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

Pre-existing issues:
- [High] Mixing devm_kzalloc for DRM objects (like planes and CRTCs) with drmm_mode_config_init causes a guaranteed Use-After-Free during driver unbind or probe failure.
--

commit 6e5820173d0237a07dd104b640171197be436f49
Author: Qingshuang Fu <fuqingshuang@kylinos.cn>

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

This commit fixes a device node reference leak in logicvc_layers_init(). It
adds a missing of_node_put() on the error path to ensure the reference
acquired by for_each_child_of_node() is properly released when
logicvc_layer_init() fails.

> 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 could this error path lead to a
use-after-free during mode config cleanup?

When logicvc_layer_init() fails and the code jumps to the error label,
logicvc_layers_init() cleans up previously initialized layers by calling
logicvc_layer_fini(). This function calls devm_kfree(dev, layer) without
first calling drm_plane_cleanup(), leaving dangling pointers in the drm_dev
plane_list.

Additionally, because logicvc_drm_probe() sets up mode config via
drmm_mode_config_init(), the mode config cleanup is registered as a DRM managed
action. The layer and CRTC structures are allocated with devres (devm_kzalloc()
in logicvc_layer_init() and logicvc_crtc_init()).

During a probe failure or driver unbind, devres runs in LIFO order and frees
the layers and CRTC before releasing the drm_dev. When the drm_dev is finally
released, drmm_mode_config_cleanup() iterates over mode_config.plane_list and
crtc_list to call their destroy hooks, dereferencing the already freed memory.

>  		}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826065408.617148-1-fffsqian@163.com?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re:Re: [PATCH] drm/logicvc: fix device_node refcount leak in logicvc_layers_init()
  2026-08-26  7:09 ` sashiko-bot
@ 2026-08-28  2:54   ` Qingshuang Fu
  0 siblings, 0 replies; 3+ messages in thread
From: Qingshuang Fu @ 2026-08-28  2:54 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: dri-devel


At 2026-08-26 15:09:53, sashiko-bot@kernel.org wrote:
>Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
>Pre-existing issues:
>- [High] Mixing devm_kzalloc for DRM objects (like planes and CRTCs) with drmm_mode_config_init causes a guaranteed Use-After-Free during driver unbind or probe failure.
>--
>
>commit 6e5820173d0237a07dd104b640171197be436f49
>Author: Qingshuang Fu <fuqingshuang@kylinos.cn>
>
>drm/logicvc: fix device_node refcount leak in logicvc_layers_init()
>
>This commit fixes a device node reference leak in logicvc_layers_init(). It
>adds a missing of_node_put() on the error path to ensure the reference
>acquired by for_each_child_of_node() is properly released when
>logicvc_layer_init() fails.
>
>> 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 could this error path lead to a
>use-after-free during mode config cleanup?
>
>When logicvc_layer_init() fails and the code jumps to the error label,
>logicvc_layers_init() cleans up previously initialized layers by calling
>logicvc_layer_fini(). This function calls devm_kfree(dev, layer) without
>first calling drm_plane_cleanup(), leaving dangling pointers in the drm_dev
>plane_list.
>
>Additionally, because logicvc_drm_probe() sets up mode config via
>drmm_mode_config_init(), the mode config cleanup is registered as a DRM managed
>action. The layer and CRTC structures are allocated with devres (devm_kzalloc()
>in logicvc_layer_init() and logicvc_crtc_init()).
>
>During a probe failure or driver unbind, devres runs in LIFO order and frees
>the layers and CRTC before releasing the drm_dev. When the drm_dev is finally
>released, drmm_mode_config_cleanup() iterates over mode_config.plane_list and
>crtc_list to call their destroy hooks, dereferencing the already freed memory.
>
>>  		}
>>
>
>-- 
>Sashiko AI review · https://sashiko.dev/#/patchset/20260826065408.617148-1-fffsqian@163.com?part=1



Thanks Sashiko AI for the careful review and raising this important pre‑existing use‑after‑free concern.
This change only adds a missing of_node_put(layer_node) to fix a device‑tree reference leak on the
error‑path inside logicvc_layers_init(). No plane, CRTC, devres or drmm_mode_config_init‑related
allocation or cleanup logic has been altered by this commit.

The devres vs drmm cleanup‑order UAF defect is an independent pre‑existing bug, completely unrelated
to the refcount leak being fixed here. As such, it falls outside the scope of this patch.

Qingshuang Fu

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-28  7:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-26  6:54 [PATCH] drm/logicvc: fix device_node refcount leak in logicvc_layers_init() Qingshuang Fu
2026-08-26  7:09 ` sashiko-bot
2026-08-28  2:54   ` Qingshuang Fu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox