Messages in this thread |  | | Date | Mon, 21 Nov 2022 11:37:29 +0800 | From | Zeng Heng <> | Subject | Re: [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
|  |