From a6074cf0126b0bee51ab77a15930dc24a4d5db90 Mon Sep 17 00:00:00 2001 From: Jinjie Ruan Date: Wed, 27 Sep 2023 17:03:47 +0800 Subject: kunit: Fix missed memory release in kunit_free_suite_set() modprobe cpumask_kunit and rmmod cpumask_kunit, kmemleak detect a suspected memory leak as below. If kunit_filter_suites() in kunit_module_init() succeeds, the suite_set.start will not be NULL and the kunit_free_suite_set() in kunit_module_exit() should free all the memory which has not been freed. However the test_cases in suites is left out. unreferenced object 0xffff54ac47e83200 (size 512): comm "modprobe", pid 592, jiffies 4294913238 (age 1367.612s) hex dump (first 32 bytes): 84 13 1a f0 d3 b6 ff ff 30 68 1a f0 d3 b6 ff ff ........0h...... 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ backtrace: [<000000008dec63a2>] slab_post_alloc_hook+0xb8/0x368 [<00000000ec280d8e>] __kmem_cache_alloc_node+0x174/0x290 [<00000000896c7740>] __kmalloc+0x60/0x2c0 [<000000007a50fa06>] kunit_filter_suites+0x254/0x5b8 [<0000000078cc98e2>] kunit_module_notify+0xf4/0x240 [<0000000033cea952>] notifier_call_chain+0x98/0x17c [<00000000973d05cc>] notifier_call_chain_robust+0x4c/0xa4 [<000000005f95895f>] blocking_notifier_call_chain_robust+0x4c/0x74 [<0000000048e36fa7>] load_module+0x1a2c/0x1c40 [<0000000004eb8a91>] init_module_from_file+0x94/0xcc [<0000000037dbba28>] idempotent_init_module+0x184/0x278 [<00000000161b75cb>] __arm64_sys_finit_module+0x68/0xa8 [<000000006dc1669b>] invoke_syscall+0x44/0x100 [<00000000fa87e304>] el0_svc_common.constprop.1+0x68/0xe0 [<000000009d8ad866>] do_el0_svc+0x1c/0x28 [<000000005b83c607>] el0_svc+0x3c/0xc4 Fixes: a127b154a8f2 ("kunit: tool: allow filtering test cases via glob") Signed-off-by: Jinjie Ruan Reviewed-by: Rae Moar Reviewed-by: David Gow Signed-off-by: Shuah Khan --- lib/kunit/executor.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'lib/kunit/executor.c') diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index a6348489d45f..a037a46fae5e 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -137,8 +137,10 @@ void kunit_free_suite_set(struct kunit_suite_set suite_set) { struct kunit_suite * const *suites; - for (suites = suite_set.start; suites < suite_set.end; suites++) + for (suites = suite_set.start; suites < suite_set.end; suites++) { + kfree((*suites)->test_cases); kfree(*suites); + } kfree(suite_set.start); } -- cgit From e44679515a7b803cf0143dc9de3d2ecbe907f939 Mon Sep 17 00:00:00 2001 From: Jinjie Ruan Date: Wed, 27 Sep 2023 17:03:48 +0800 Subject: kunit: Fix the wrong kfree of copy for kunit_filter_suites() If the outer layer for loop is iterated more than once and it fails not in the first iteration, the copy pointer has been moved. So it should free the original copy's backup copy_start. Fixes: abbf73816b6f ("kunit: fix possible memory leak in kunit_filter_suites()") Signed-off-by: Jinjie Ruan Reviewed-by: Rae Moar Reviewed-by: David Gow Signed-off-by: Shuah Khan --- lib/kunit/executor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/kunit/executor.c') diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index a037a46fae5e..9358ed2df839 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -243,7 +243,7 @@ free_parsed_glob: free_copy: if (*err) - kfree(copy); + kfree(copy_start); return filtered; } -- cgit From 24de14c98b37ea40a7e493dfd0d93b400b6efbca Mon Sep 17 00:00:00 2001 From: Jinjie Ruan Date: Wed, 27 Sep 2023 17:03:49 +0800 Subject: kunit: Fix possible memory leak in kunit_filter_suites() If the outer layer for loop is iterated more than once and it fails not in the first iteration, the filtered_suite and filtered_suite->test_cases allocated in the last kunit_filter_attr_tests() in last inner for loop is leaked. So add a new free_filtered_suite err label and free the filtered_suite and filtered_suite->test_cases so far. And change kmalloc_array of copy to kcalloc to Clear the copy to make the kfree safe. Fixes: 529534e8cba3 ("kunit: Add ability to filter attributes") Signed-off-by: Jinjie Ruan Reviewed-by: Rae Moar Reviewed-by: David Gow Signed-off-by: Shuah Khan --- lib/kunit/executor.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) (limited to 'lib/kunit/executor.c') diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index 9358ed2df839..1236b3cd2fbb 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -157,10 +157,11 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set, struct kunit_suite_set filtered = {NULL, NULL}; struct kunit_glob_filter parsed_glob; struct kunit_attr_filter *parsed_filters = NULL; + struct kunit_suite * const *suites; const size_t max = suite_set->end - suite_set->start; - copy = kmalloc_array(max, sizeof(*filtered.start), GFP_KERNEL); + copy = kcalloc(max, sizeof(*filtered.start), GFP_KERNEL); if (!copy) { /* won't be able to run anything, return an empty set */ return filtered; } @@ -195,7 +196,7 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set, parsed_glob.test_glob); if (IS_ERR(filtered_suite)) { *err = PTR_ERR(filtered_suite); - goto free_parsed_filters; + goto free_filtered_suite; } } if (filter_count > 0 && parsed_filters != NULL) { @@ -212,11 +213,11 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set, filtered_suite = new_filtered_suite; if (*err) - goto free_parsed_filters; + goto free_filtered_suite; if (IS_ERR(filtered_suite)) { *err = PTR_ERR(filtered_suite); - goto free_parsed_filters; + goto free_filtered_suite; } if (!filtered_suite) break; @@ -231,6 +232,14 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set, filtered.start = copy_start; filtered.end = copy; +free_filtered_suite: + if (*err) { + for (suites = copy_start; suites < copy; suites++) { + kfree((*suites)->test_cases); + kfree(*suites); + } + } + free_parsed_filters: if (filter_count) kfree(parsed_filters); -- cgit