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@xxxxxxxxxx>
---
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
