[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <DG5INIQWWPJV.31PV8URRWUZF6@bootlin.com>
Date: Tue, 03 Feb 2026 18:51:47 +0100
From: "Luca Ceresoli" <luca.ceresoli@...tlin.com>
To: "Kory Maincent" <kory.maincent@...tlin.com>, "Felix Gu"
<ustc.gu@...il.com>
Cc: "Paul Kocialkowski" <paulk@...-base.io>, "Maarten Lankhorst"
<maarten.lankhorst@...ux.intel.com>, "Maxime Ripard" <mripard@...nel.org>,
"Thomas Zimmermann" <tzimmermann@...e.de>, "David Airlie"
<airlied@...il.com>, "Simona Vetter" <simona@...ll.ch>, "Paul Kocialkowski"
<paul.kocialkowski@...tlin.com>, <dri-devel@...ts.freedesktop.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] drm/logicvc: Fix device node reference leak in
logicvc_drm_config_parse()
On Tue Feb 3, 2026 at 2:35 PM CET, Kory Maincent wrote:
> On Tue, 3 Feb 2026 10:57:47 +0100
> Kory Maincent <kory.maincent@...tlin.com> wrote:
>
>> On Fri, 30 Jan 2026 00:21:19 +0800
>> Felix Gu <ustc.gu@...il.com> wrote:
>>
>> > The logicvc_drm_config_parse() function calls of_get_child_by_name() to
>> > find the "layers" node but fails to release the reference, leading to a
>> > device node reference leak.
>> >
>> > Fix this by using the __free(device_node) cleanup attribute to automatic
>> > release the reference when the variable goes out of scope.
>> >
>> > Fixes: efeeaefe9be5 ("drm: Add support for the LogiCVC display controller")
>> > Signed-off-by: Felix Gu <ustc.gu@...il.com>
>> > ---
>> > drivers/gpu/drm/logicvc/logicvc_drm.c | 4 ++--
>> > 1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/logicvc/logicvc_drm.c
>> > b/drivers/gpu/drm/logicvc/logicvc_drm.c index 204b0fee55d0..bbebf4fc7f51
>> > 100644 --- a/drivers/gpu/drm/logicvc/logicvc_drm.c
>> > +++ b/drivers/gpu/drm/logicvc/logicvc_drm.c
>> > @@ -92,7 +92,6 @@ static int logicvc_drm_config_parse(struct logicvc_drm
>> > *logicvc) struct device *dev = drm_dev->dev;
>> > struct device_node *of_node = dev->of_node;
>> > struct logicvc_drm_config *config = &logicvc->config;
>> > - struct device_node *layers_node;
>> > int ret;
>> >
>> > logicvc_of_property_parse_bool(of_node,
>> > LOGICVC_OF_PROPERTY_DITHERING, @@ -128,7 +127,8 @@ static int
>> > logicvc_drm_config_parse(struct logicvc_drm *logicvc) if (ret)
>> > return ret;
>> >
>> > - layers_node = of_get_child_by_name(of_node, "layers");
>> > + struct device_node *layers_node __free(device_node) =
>> > + of_get_child_by_name(of_node, "layers");
>>
>> We usualy don't declare variable in the middle of a function.
>> You should rather simply change the declaration line at the top like this:
>> struct device_node *layers_node __free(device_node) = NULL;
>
> It seems Linus disagree on this topic and I understand his arguments:
> https://lore.kernel.org/all/CAHk-=whPZoi03ZwphxiW6cuWPtC3nyKYS8_BThgztCdgPWP1WA@mail.gmail.com/
> https://lore.kernel.org/all/CAHk-=wiCOTW5UftUrAnvJkr6769D29tF7Of79gUjdQHS_TkF5A@mail.gmail.com/
I agree it makes code much more readable, espacially in the case of cleanup
actions.
> Then, ok to me:
> Reviewed-by: Kory Maincent <kory.maincent@...tlin.com>
Thanks! I'm applying this patch to drm-misc-fixes perhaps next week.
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Powered by blists - more mailing lists