lkml.org 
[lkml]   [2022]   [Nov]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] of: overlay: fix memory leak in add_changeset_node()

On 2022/11/21 10:00, Frank Rowand wrote:
> On 11/18/22 04:53, Zeng Heng wrote:
>> In of_changeset_action(), we have called of_node_get() to increase
>> refcount of a node.
>>
>> Therefore, when tchild (duplicated by __of_node_dup()) is done,
>> of_node_put() needs to call and release the device_node.
>>
>> Otherwise, there are some memory leak reported about the node:
>>
>> unreferenced object 0xffff88810cd1e800 (size 256):
>> backtrace:
>> kmalloc_trace
>> __of_node_dup
>> add_changeset_node (inlined)
>> build_changeset_next_level
>>
>> unreferenced object 0xffff888113721240 (size 16):
>> backtrace:
>> __kmalloc_node_track_caller
>> kstrdup
>> __of_node_dup
>> add_changeset_node (inlined)
>> build_changeset_next_level
>>
>> unreferenced object 0xffff88810a38d400 (size 128):
>> backtrace:
>> kmalloc_trace
>> __of_prop_dup
>> add_changeset_property
>> build_changeset_next_level
>>
>> Fixes: 7518b5890d8a ("of/overlay: Introduce DT overlay support")
>> Signed-off-by: Zeng Heng<zengheng4@huawei.com>
>> ---
>> drivers/of/overlay.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
>> index bd8ff4df723d..a5189a0ec0a3 100644
>> --- a/drivers/of/overlay.c
>> +++ b/drivers/of/overlay.c
>> @@ -436,8 +436,10 @@ static int add_changeset_node(struct overlay_changeset *ovcs,
>> of_node_set_flag(tchild, OF_OVERLAY);
>>
>> ret = of_changeset_attach_node(&ovcs->cset, tchild);
>> - if (ret)
>> + if (ret) {
>> + of_node_put(tchild);
>> return ret;
>> + }
> By visual inspection of the code (Linux 6.1-rc1), this does not appear to be
> correct. For the only case where of_changeset_action(),
> called by of_changeset_attach_node(), returns an error, of_node_get() has not
> yet occurred on tchild.

The correct explanation should be like:

When of_changeset_attach_node() returns fail and tchild is over of life
cycle which is duplicated by

__of_node_dup(), it needs to call of_node_put() to release tchild in
error handle route.


The patch is fine, but feel sorry to apologize for the incorrect comment
of the patch.

I would update the comment in the patch v2.


With Best Regards,

Zeng Heng

>>
>> target_child.np = tchild;
>> target_child.in_livetree = false;
> For which version of Linux were the memory leaks detected? Were any additional
> patches applied?
>
> -Frank

\
 
 \ /
  Last update: 2022-11-21 04:37    [W:0.642 / U:0.204 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site