[PATCH v3] of: Fix unbalanced of node refcount and memory leaks

Jinjie Ruan posted 1 patch 1 month, 2 weeks ago
drivers/of/overlay_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH v3] of: Fix unbalanced of node refcount and memory leaks
Posted by Jinjie Ruan 1 month, 2 weeks ago
Got following report when doing overlay_test:

	OF: ERROR: memory leak, expected refcount 1 instead of 2,
	of_node_get()/of_node_put() unbalanced - destroy cset entry:
	attach overlay node            /kunit-test

	OF: ERROR: memory leak before free overlay changeset,  /kunit-test

In of_overlay_apply_kunit_cleanup(), the "np" should be associated with
fake instead of test to call of_node_put(), so the node is put before
the overlay is removed.

It also fix the following memory leaks:

	unreferenced object 0xffffff80c7d22800 (size 256):
	  comm "kunit_try_catch", pid 236, jiffies 4294894764
	  hex dump (first 32 bytes):
	    d0 26 d4 c2 80 ff ff ff 00 00 00 00 00 00 00 00  .&..............
	    60 19 75 c1 80 ff ff ff 00 00 00 00 00 00 00 00  `.u.............
	  backtrace (crc ee0a471c):
	    [<0000000058ea1340>] kmemleak_alloc+0x34/0x40
	    [<00000000c538ac7e>] __kmalloc_cache_noprof+0x26c/0x2f4
	    [<00000000119f34f3>] __of_node_dup+0x4c/0x328
	    [<00000000b212ca39>] build_changeset_next_level+0x2cc/0x4c0
	    [<00000000eb208e87>] of_overlay_fdt_apply+0x930/0x1334
	    [<000000005bdc53a3>] of_overlay_fdt_apply_kunit+0x54/0x10c
	    [<00000000143acd5d>] of_overlay_apply_kunit_cleanup+0x12c/0x524
	    [<00000000a813abc8>] kunit_try_run_case+0x13c/0x3ac
	    [<00000000d77ab00c>] kunit_generic_run_threadfn_adapter+0x80/0xec
	    [<000000000b296be1>] kthread+0x2e8/0x374
	    [<0000000007bd1c51>] ret_from_fork+0x10/0x20
	unreferenced object 0xffffff80c1751960 (size 16):
	  comm "kunit_try_catch", pid 236, jiffies 4294894764
	  hex dump (first 16 bytes):
	    6b 75 6e 69 74 2d 74 65 73 74 00 c1 80 ff ff ff  kunit-test......
	  backtrace (crc 18196259):
	    [<0000000058ea1340>] kmemleak_alloc+0x34/0x40
	    [<0000000071006e2c>] __kmalloc_node_track_caller_noprof+0x300/0x3e0
	    [<00000000b16ac6cb>] kstrdup+0x48/0x84
	    [<0000000050e3373b>] __of_node_dup+0x60/0x328
	    [<00000000b212ca39>] build_changeset_next_level+0x2cc/0x4c0
	    [<00000000eb208e87>] of_overlay_fdt_apply+0x930/0x1334
	    [<000000005bdc53a3>] of_overlay_fdt_apply_kunit+0x54/0x10c
	    [<00000000143acd5d>] of_overlay_apply_kunit_cleanup+0x12c/0x524
	    [<00000000a813abc8>] kunit_try_run_case+0x13c/0x3ac
	    [<00000000d77ab00c>] kunit_generic_run_threadfn_adapter+0x80/0xec
	    [<000000000b296be1>] kthread+0x2e8/0x374
	    [<0000000007bd1c51>] ret_from_fork+0x10/0x20
	unreferenced object 0xffffff80c2e96e00 (size 192):
	  comm "kunit_try_catch", pid 236, jiffies 4294894764
	  hex dump (first 32 bytes):
	    80 19 75 c1 80 ff ff ff 0b 00 00 00 00 00 00 00  ..u.............
	    a0 19 75 c1 80 ff ff ff 00 6f e9 c2 80 ff ff ff  ..u......o......
	  backtrace (crc 1924cba4):
	    [<0000000058ea1340>] kmemleak_alloc+0x34/0x40
	    [<00000000c538ac7e>] __kmalloc_cache_noprof+0x26c/0x2f4
	    [<000000009fdd35ad>] __of_prop_dup+0x7c/0x2ec
	    [<00000000aa4e0111>] add_changeset_property+0x548/0x9e0
	    [<000000004777e25b>] build_changeset_next_level+0xd4/0x4c0
	    [<00000000a9c93f8a>] build_changeset_next_level+0x3a8/0x4c0
	    [<00000000eb208e87>] of_overlay_fdt_apply+0x930/0x1334
	    [<000000005bdc53a3>] of_overlay_fdt_apply_kunit+0x54/0x10c
	    [<00000000143acd5d>] of_overlay_apply_kunit_cleanup+0x12c/0x524
	    [<00000000a813abc8>] kunit_try_run_case+0x13c/0x3ac
	    [<00000000d77ab00c>] kunit_generic_run_threadfn_adapter+0x80/0xec
	    [<000000000b296be1>] kthread+0x2e8/0x374
	    [<0000000007bd1c51>] ret_from_fork+0x10/0x20
	unreferenced object 0xffffff80c1751980 (size 16):
	  comm "kunit_try_catch", pid 236, jiffies 4294894764
	  hex dump (first 16 bytes):
	    63 6f 6d 70 61 74 69 62 6c 65 00 c1 80 ff ff ff  compatible......
	  backtrace (crc 42df3c87):
	    [<0000000058ea1340>] kmemleak_alloc+0x34/0x40
	    [<0000000071006e2c>] __kmalloc_node_track_caller_noprof+0x300/0x3e0
	    [<00000000b16ac6cb>] kstrdup+0x48/0x84
	    [<00000000a8888fd8>] __of_prop_dup+0xb0/0x2ec
	    [<00000000aa4e0111>] add_changeset_property+0x548/0x9e0
	    [<000000004777e25b>] build_changeset_next_level+0xd4/0x4c0
	    [<00000000a9c93f8a>] build_changeset_next_level+0x3a8/0x4c0
	    [<00000000eb208e87>] of_overlay_fdt_apply+0x930/0x1334
	    [<000000005bdc53a3>] of_overlay_fdt_apply_kunit+0x54/0x10c
	    [<00000000143acd5d>] of_overlay_apply_kunit_cleanup+0x12c/0x524
	    [<00000000a813abc8>] kunit_try_run_case+0x13c/0x3ac
	    [<00000000d77ab00c>] kunit_generic_run_threadfn_adapter+0x80/0xec
	    [<000000000b296be1>] kthread+0x2e8/0x374
	unreferenced object 0xffffff80c2e96f00 (size 192):
	  comm "kunit_try_catch", pid 236, jiffies 4294894764
	  hex dump (first 32 bytes):
	    40 f7 bb c6 80 ff ff ff 0b 00 00 00 00 00 00 00  @...............
	    c0 19 75 c1 80 ff ff ff 00 00 00 00 00 00 00 00  ..u.............
	  backtrace (crc f2f57ea7):
	    [<0000000058ea1340>] kmemleak_alloc+0x34/0x40
	    [<00000000c538ac7e>] __kmalloc_cache_noprof+0x26c/0x2f4
	    [<000000009fdd35ad>] __of_prop_dup+0x7c/0x2ec
	    [<00000000aa4e0111>] add_changeset_property+0x548/0x9e0
	    [<000000004777e25b>] build_changeset_next_level+0xd4/0x4c0
	    [<00000000a9c93f8a>] build_changeset_next_level+0x3a8/0x4c0
	    [<00000000eb208e87>] of_overlay_fdt_apply+0x930/0x1334
	    [<000000005bdc53a3>] of_overlay_fdt_apply_kunit+0x54/0x10c
	    [<00000000143acd5d>] of_overlay_apply_kunit_cleanup+0x12c/0x524
	    [<00000000a813abc8>] kunit_try_run_case+0x13c/0x3ac
	    [<00000000d77ab00c>] kunit_generic_run_threadfn_adapter+0x80/0xec
	    [<000000000b296be1>] kthread+0x2e8/0x374
	    [<0000000007bd1c51>] ret_from_fork+0x10/0x20
	......

How to reproduce:
	CONFIG_OF_OVERLAY_KUNIT_TEST=y, CONFIG_DEBUG_KMEMLEAK=y
	and CONFIG_DEBUG_KMEMLEAK_AUTO_SCAN=y, launch the kernel.

Fixes: 5c9dd72d8385 ("of: Add a KUnit test for overlays and test managed APIs")
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
v3:
- Change the fix way by replacing test with fake.
- Add Reviewed-by.
v2:
- Add memory leak stack.
- Update the commit message.
---
 drivers/of/overlay_test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/of/overlay_test.c b/drivers/of/overlay_test.c
index 19a292cdeee3..19695bdf77be 100644
--- a/drivers/of/overlay_test.c
+++ b/drivers/of/overlay_test.c
@@ -73,7 +73,7 @@ static void of_overlay_apply_kunit_cleanup(struct kunit *test)
 
 	np = of_find_node_by_name(NULL, kunit_node_name);
 	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np);
-	of_node_put_kunit(test, np);
+	of_node_put_kunit(&fake, np);
 
 	pdev = of_find_device_by_node(np);
 	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);
-- 
2.34.1
Re: [PATCH v3] of: Fix unbalanced of node refcount and memory leaks
Posted by Rob Herring (Arm) 1 month, 2 weeks ago
On Thu, 10 Oct 2024 11:44:16 +0800, Jinjie Ruan wrote:
> Got following report when doing overlay_test:
> 
> 	OF: ERROR: memory leak, expected refcount 1 instead of 2,
> 	of_node_get()/of_node_put() unbalanced - destroy cset entry:
> 	attach overlay node            /kunit-test
> 
> 	OF: ERROR: memory leak before free overlay changeset,  /kunit-test
> 
> In of_overlay_apply_kunit_cleanup(), the "np" should be associated with
> fake instead of test to call of_node_put(), so the node is put before
> the overlay is removed.
> 
> It also fix the following memory leaks:
> 
> 	unreferenced object 0xffffff80c7d22800 (size 256):
> 	  comm "kunit_try_catch", pid 236, jiffies 4294894764
> 	  hex dump (first 32 bytes):
> 	    d0 26 d4 c2 80 ff ff ff 00 00 00 00 00 00 00 00  .&..............
> 	    60 19 75 c1 80 ff ff ff 00 00 00 00 00 00 00 00  `.u.............
> 	  backtrace (crc ee0a471c):
> 	    [<0000000058ea1340>] kmemleak_alloc+0x34/0x40
> 	    [<00000000c538ac7e>] __kmalloc_cache_noprof+0x26c/0x2f4
> 	    [<00000000119f34f3>] __of_node_dup+0x4c/0x328
> 	    [<00000000b212ca39>] build_changeset_next_level+0x2cc/0x4c0
> 	    [<00000000eb208e87>] of_overlay_fdt_apply+0x930/0x1334
> 	    [<000000005bdc53a3>] of_overlay_fdt_apply_kunit+0x54/0x10c
> 	    [<00000000143acd5d>] of_overlay_apply_kunit_cleanup+0x12c/0x524
> 	    [<00000000a813abc8>] kunit_try_run_case+0x13c/0x3ac
> 	    [<00000000d77ab00c>] kunit_generic_run_threadfn_adapter+0x80/0xec
> 	    [<000000000b296be1>] kthread+0x2e8/0x374
> 	    [<0000000007bd1c51>] ret_from_fork+0x10/0x20
> 	unreferenced object 0xffffff80c1751960 (size 16):
> 	  comm "kunit_try_catch", pid 236, jiffies 4294894764
> 	  hex dump (first 16 bytes):
> 	    6b 75 6e 69 74 2d 74 65 73 74 00 c1 80 ff ff ff  kunit-test......
> 	  backtrace (crc 18196259):
> 	    [<0000000058ea1340>] kmemleak_alloc+0x34/0x40
> 	    [<0000000071006e2c>] __kmalloc_node_track_caller_noprof+0x300/0x3e0
> 	    [<00000000b16ac6cb>] kstrdup+0x48/0x84
> 	    [<0000000050e3373b>] __of_node_dup+0x60/0x328
> 	    [<00000000b212ca39>] build_changeset_next_level+0x2cc/0x4c0
> 	    [<00000000eb208e87>] of_overlay_fdt_apply+0x930/0x1334
> 	    [<000000005bdc53a3>] of_overlay_fdt_apply_kunit+0x54/0x10c
> 	    [<00000000143acd5d>] of_overlay_apply_kunit_cleanup+0x12c/0x524
> 	    [<00000000a813abc8>] kunit_try_run_case+0x13c/0x3ac
> 	    [<00000000d77ab00c>] kunit_generic_run_threadfn_adapter+0x80/0xec
> 	    [<000000000b296be1>] kthread+0x2e8/0x374
> 	    [<0000000007bd1c51>] ret_from_fork+0x10/0x20
> 	unreferenced object 0xffffff80c2e96e00 (size 192):
> 	  comm "kunit_try_catch", pid 236, jiffies 4294894764
> 	  hex dump (first 32 bytes):
> 	    80 19 75 c1 80 ff ff ff 0b 00 00 00 00 00 00 00  ..u.............
> 	    a0 19 75 c1 80 ff ff ff 00 6f e9 c2 80 ff ff ff  ..u......o......
> 	  backtrace (crc 1924cba4):
> 	    [<0000000058ea1340>] kmemleak_alloc+0x34/0x40
> 	    [<00000000c538ac7e>] __kmalloc_cache_noprof+0x26c/0x2f4
> 	    [<000000009fdd35ad>] __of_prop_dup+0x7c/0x2ec
> 	    [<00000000aa4e0111>] add_changeset_property+0x548/0x9e0
> 	    [<000000004777e25b>] build_changeset_next_level+0xd4/0x4c0
> 	    [<00000000a9c93f8a>] build_changeset_next_level+0x3a8/0x4c0
> 	    [<00000000eb208e87>] of_overlay_fdt_apply+0x930/0x1334
> 	    [<000000005bdc53a3>] of_overlay_fdt_apply_kunit+0x54/0x10c
> 	    [<00000000143acd5d>] of_overlay_apply_kunit_cleanup+0x12c/0x524
> 	    [<00000000a813abc8>] kunit_try_run_case+0x13c/0x3ac
> 	    [<00000000d77ab00c>] kunit_generic_run_threadfn_adapter+0x80/0xec
> 	    [<000000000b296be1>] kthread+0x2e8/0x374
> 	    [<0000000007bd1c51>] ret_from_fork+0x10/0x20
> 	unreferenced object 0xffffff80c1751980 (size 16):
> 	  comm "kunit_try_catch", pid 236, jiffies 4294894764
> 	  hex dump (first 16 bytes):
> 	    63 6f 6d 70 61 74 69 62 6c 65 00 c1 80 ff ff ff  compatible......
> 	  backtrace (crc 42df3c87):
> 	    [<0000000058ea1340>] kmemleak_alloc+0x34/0x40
> 	    [<0000000071006e2c>] __kmalloc_node_track_caller_noprof+0x300/0x3e0
> 	    [<00000000b16ac6cb>] kstrdup+0x48/0x84
> 	    [<00000000a8888fd8>] __of_prop_dup+0xb0/0x2ec
> 	    [<00000000aa4e0111>] add_changeset_property+0x548/0x9e0
> 	    [<000000004777e25b>] build_changeset_next_level+0xd4/0x4c0
> 	    [<00000000a9c93f8a>] build_changeset_next_level+0x3a8/0x4c0
> 	    [<00000000eb208e87>] of_overlay_fdt_apply+0x930/0x1334
> 	    [<000000005bdc53a3>] of_overlay_fdt_apply_kunit+0x54/0x10c
> 	    [<00000000143acd5d>] of_overlay_apply_kunit_cleanup+0x12c/0x524
> 	    [<00000000a813abc8>] kunit_try_run_case+0x13c/0x3ac
> 	    [<00000000d77ab00c>] kunit_generic_run_threadfn_adapter+0x80/0xec
> 	    [<000000000b296be1>] kthread+0x2e8/0x374
> 	unreferenced object 0xffffff80c2e96f00 (size 192):
> 	  comm "kunit_try_catch", pid 236, jiffies 4294894764
> 	  hex dump (first 32 bytes):
> 	    40 f7 bb c6 80 ff ff ff 0b 00 00 00 00 00 00 00  @...............
> 	    c0 19 75 c1 80 ff ff ff 00 00 00 00 00 00 00 00  ..u.............
> 	  backtrace (crc f2f57ea7):
> 	    [<0000000058ea1340>] kmemleak_alloc+0x34/0x40
> 	    [<00000000c538ac7e>] __kmalloc_cache_noprof+0x26c/0x2f4
> 	    [<000000009fdd35ad>] __of_prop_dup+0x7c/0x2ec
> 	    [<00000000aa4e0111>] add_changeset_property+0x548/0x9e0
> 	    [<000000004777e25b>] build_changeset_next_level+0xd4/0x4c0
> 	    [<00000000a9c93f8a>] build_changeset_next_level+0x3a8/0x4c0
> 	    [<00000000eb208e87>] of_overlay_fdt_apply+0x930/0x1334
> 	    [<000000005bdc53a3>] of_overlay_fdt_apply_kunit+0x54/0x10c
> 	    [<00000000143acd5d>] of_overlay_apply_kunit_cleanup+0x12c/0x524
> 	    [<00000000a813abc8>] kunit_try_run_case+0x13c/0x3ac
> 	    [<00000000d77ab00c>] kunit_generic_run_threadfn_adapter+0x80/0xec
> 	    [<000000000b296be1>] kthread+0x2e8/0x374
> 	    [<0000000007bd1c51>] ret_from_fork+0x10/0x20
> 	......
> 
> How to reproduce:
> 	CONFIG_OF_OVERLAY_KUNIT_TEST=y, CONFIG_DEBUG_KMEMLEAK=y
> 	and CONFIG_DEBUG_KMEMLEAK_AUTO_SCAN=y, launch the kernel.
> 
> Fixes: 5c9dd72d8385 ("of: Add a KUnit test for overlays and test managed APIs")
> Reviewed-by: Stephen Boyd <sboyd@kernel.org>
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> ---
> v3:
> - Change the fix way by replacing test with fake.
> - Add Reviewed-by.
> v2:
> - Add memory leak stack.
> - Update the commit message.
> ---
>  drivers/of/overlay_test.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Applied, thanks!