Re: [PATCH -next] kunit: fix possible memory leak in kunit_filter_suites()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2023/8/2 5:22, Rae Moar wrote:
> On Tue, Aug 1, 2023 at 3:37 AM 'Ruan Jinjie' via KUnit Development
> <kunit-dev@xxxxxxxxxxxxxxxx> wrote:
>>
>> Inject fault while probing drm_kunit_helpers.ko, if one of
>> kunit_next_attr_filter(), kunit_filter_glob_tests() and
>> kunit_filter_attr_tests() fails, parsed_filters,
>> parsed_glob.suite_glob/test_glob alloced in
>> kunit_parse_glob_filter() is leaked.
>> And the filtered_suite->test_cases alloced in kunit_filter_glob_tests()
>> or kunit_filter_attr_tests() may also be leaked.
>>
>> unreferenced object 0xff110001067e4800 (size 1024):
>>   comm "kunit_try_catch", pid 96, jiffies 4294671796 (age 763.547s)
>>   hex dump (first 32 bytes):
>>     73 75 69 74 65 32 00 00 00 00 00 00 00 00 00 00  suite2..........
>>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>>   backtrace:
>>     [<00000000116e8eba>] __kmalloc_node_track_caller+0x4e/0x140
>>     [<00000000e2f9cce9>] kmemdup+0x2c/0x60
>>     [<000000002a36710b>] kunit_filter_suites+0x3e4/0xa50
>>     [<0000000045779fb9>] filter_suites_test+0x1b7/0x440
>>     [<00000000cd1104a7>] kunit_try_run_case+0x119/0x270
>>     [<00000000c654c917>] kunit_generic_run_threadfn_adapter+0x4e/0xa0
>>     [<00000000d195ac13>] kthread+0x2c7/0x3c0
>>     [<00000000b79c1ee9>] ret_from_fork+0x2c/0x70
>>     [<000000001167f7e6>] ret_from_fork_asm+0x1b/0x30
>> unreferenced object 0xff11000105d79b00 (size 192):
>>   comm "kunit_try_catch", pid 96, jiffies 4294671796 (age 763.547s)
>>   hex dump (first 32 bytes):
>>     f0 e1 5a 88 ff ff ff ff 60 59 bb 8a ff ff ff ff  ..Z.....`Y......
>>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>>   backtrace:
>>     [<000000000d6e4891>] __kmalloc+0x4d/0x140
>>     [<000000006afe50bd>] kunit_filter_suites+0x424/0xa50
>>     [<0000000045779fb9>] filter_suites_test+0x1b7/0x440
>>     [<00000000cd1104a7>] kunit_try_run_case+0x119/0x270
>>     [<00000000c654c917>] kunit_generic_run_threadfn_adapter+0x4e/0xa0
>>     [<00000000d195ac13>] kthread+0x2c7/0x3c0
>>     [<00000000b79c1ee9>] ret_from_fork+0x2c/0x70
>>     [<000000001167f7e6>] ret_from_fork_asm+0x1b/0x30
>> unreferenced object 0xff110001067e6000 (size 1024):
>>   comm "kunit_try_catch", pid 98, jiffies 4294671798 (age 763.545s)
>>   hex dump (first 32 bytes):
>>     73 75 69 74 65 32 00 00 00 00 00 00 00 00 00 00  suite2..........
>>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>>   backtrace:
>>     [<00000000116e8eba>] __kmalloc_node_track_caller+0x4e/0x140
>>     [<00000000e2f9cce9>] kmemdup+0x2c/0x60
>>     [<000000002a36710b>] kunit_filter_suites+0x3e4/0xa50
>>     [<00000000f452f130>] filter_suites_test_glob_test+0x1b7/0x660
>>     [<00000000cd1104a7>] kunit_try_run_case+0x119/0x270
>>     [<00000000c654c917>] kunit_generic_run_threadfn_adapter+0x4e/0xa0
>>     [<00000000d195ac13>] kthread+0x2c7/0x3c0
>>     [<00000000b79c1ee9>] ret_from_fork+0x2c/0x70
>>     [<000000001167f7e6>] ret_from_fork_asm+0x1b/0x30
>> unreferenced object 0xff11000103f3a800 (size 96):
>>   comm "kunit_try_catch", pid 98, jiffies 4294671798 (age 763.545s)
>>   hex dump (first 32 bytes):
>>     f0 e1 5a 88 ff ff ff ff 40 39 bb 8a ff ff ff ff  ..Z.....@9......
>>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>>   backtrace:
>>     [<000000000d6e4891>] __kmalloc+0x4d/0x140
>>     [<000000006afe50bd>] kunit_filter_suites+0x424/0xa50
>>     [<00000000f452f130>] filter_suites_test_glob_test+0x1b7/0x660
>>     [<00000000cd1104a7>] kunit_try_run_case+0x119/0x270
>>     [<00000000c654c917>] kunit_generic_run_threadfn_adapter+0x4e/0xa0
>>     [<00000000d195ac13>] kthread+0x2c7/0x3c0
>>     [<00000000b79c1ee9>] ret_from_fork+0x2c/0x70
>>     [<000000001167f7e6>] ret_from_fork_asm+0x1b/0x30
>> unreferenced object 0xff11000101a72ac0 (size 16):
>>   comm "kunit_try_catch", pid 104, jiffies 4294671814 (age 763.529s)
>>   hex dump (first 16 bytes):
>>     00 00 00 00 00 00 00 00 e0 2a a7 01 01 00 11 ff  .........*......
>>   backtrace:
>>     [<000000000d6e4891>] __kmalloc+0x4d/0x140
>>     [<00000000c7b724e7>] kunit_filter_suites+0x108/0xa50
>>     [<00000000bad5427d>] filter_attr_test+0x1e9/0x6a0
>>     [<00000000cd1104a7>] kunit_try_run_case+0x119/0x270
>>     [<00000000c654c917>] kunit_generic_run_threadfn_adapter+0x4e/0xa0
>>     [<00000000d195ac13>] kthread+0x2c7/0x3c0
>>     [<00000000b79c1ee9>] ret_from_fork+0x2c/0x70
>>     [<000000001167f7e6>] ret_from_fork_asm+0x1b/0x30
>> unreferenced object 0xff11000103caf880 (size 32):
>>   comm "kunit_try_catch", pid 104, jiffies 4294671814 (age 763.547s)
>>   hex dump (first 32 bytes):
>>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>>   backtrace:
>>     [<000000000d6e4891>] __kmalloc+0x4d/0x140
>>     [<00000000c47b0f75>] kunit_filter_suites+0x189/0xa50
>>     [<00000000bad5427d>] filter_attr_test+0x1e9/0x6a0
>>     [<00000000cd1104a7>] kunit_try_run_case+0x119/0x270
>>     [<00000000c654c917>] kunit_generic_run_threadfn_adapter+0x4e/0xa0
>>     [<00000000d195ac13>] kthread+0x2c7/0x3c0
>>     [<00000000b79c1ee9>] ret_from_fork+0x2c/0x70
>>     [<000000001167f7e6>] ret_from_fork_asm+0x1b/0x30
>> unreferenced object 0xff11000101a72ae0 (size 16):
>>   comm "kunit_try_catch", pid 106, jiffies 4294671823 (age 763.538s)
>>   hex dump (first 16 bytes):
>>     00 00 00 00 00 00 00 00 00 2b a7 01 01 00 11 ff  .........+......
>>   backtrace:
>>     [<000000000d6e4891>] __kmalloc+0x4d/0x140
>>     [<00000000c7b724e7>] kunit_filter_suites+0x108/0xa50
>>     [<0000000096255c51>] filter_attr_empty_test+0x1b0/0x310
>>     [<00000000cd1104a7>] kunit_try_run_case+0x119/0x270
>>     [<00000000c654c917>] kunit_generic_run_threadfn_adapter+0x4e/0xa0
>>     [<00000000d195ac13>] kthread+0x2c7/0x3c0
>>     [<00000000b79c1ee9>] ret_from_fork+0x2c/0x70
>>     [<000000001167f7e6>] ret_from_fork_asm+0x1b/0x30
>> unreferenced object 0xff11000103caf9c0 (size 32):
>>   comm "kunit_try_catch", pid 106, jiffies 4294671823 (age 763.538s)
>>   hex dump (first 32 bytes):
>>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>>   backtrace:
>>     [<000000000d6e4891>] __kmalloc+0x4d/0x140
>>     [<00000000c47b0f75>] kunit_filter_suites+0x189/0xa50
>>     [<0000000096255c51>] filter_attr_empty_test+0x1b0/0x310
>>     [<00000000cd1104a7>] kunit_try_run_case+0x119/0x270
>>     [<00000000c654c917>] kunit_generic_run_threadfn_adapter+0x4e/0xa0
>>     [<00000000d195ac13>] kthread+0x2c7/0x3c0
>>     [<00000000b79c1ee9>] ret_from_fork+0x2c/0x70
>>     [<000000001167f7e6>] ret_from_fork_asm+0x1b/0x30
>> unreferenced object 0xff11000101a72b00 (size 16):
>>   comm "kunit_try_catch", pid 108, jiffies 4294671832 (age 763.529s)
>>   hex dump (first 16 bytes):
>>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>>   backtrace:
>>     [<000000000d6e4891>] __kmalloc+0x4d/0x140
>>     [<00000000c47b0f75>] kunit_filter_suites+0x189/0xa50
>>     [<00000000881258cc>] filter_attr_skip_test+0x148/0x770
>>     [<00000000cd1104a7>] kunit_try_run_case+0x119/0x270
>>     [<00000000c654c917>] kunit_generic_run_threadfn_adapter+0x4e/0xa0
>>     [<00000000d195ac13>] kthread+0x2c7/0x3c0
>>     [<00000000b79c1ee9>] ret_from_fork+0x2c/0x70
>>     [<000000001167f7e6>] ret_from_fork_asm+0x1b/0x30
>>
>> Fixes: 5d31f71efcb6 ("kunit: add kunit.filter_glob cmdline option to filter suites")
>> Fixes: 529534e8cba3 ("kunit: Add ability to filter attributes")
>> Signed-off-by: Ruan Jinjie <ruanjinjie@xxxxxxxxxx>
> 
> Hello!
> 
> I have tested this and this all looks good to me. Sorry for the delay
> in responding to the patches addressing the bugs in filtering. I have
> been out of the office for a few days. Thanks for addressing this!

I find when open kmemleak and some kunit test config, the above memory
leak info always occur.

> 
> Reviewed-by: Rae Moar <rmoar@xxxxxxxxxx>
> 
> -Rae
> 
>> ---
>>  lib/kunit/executor.c | 17 ++++++++++++-----
>>  1 file changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
>> index 5b5bed1efb93..481901d245d0 100644
>> --- a/lib/kunit/executor.c
>> +++ b/lib/kunit/executor.c
>> @@ -151,7 +151,7 @@ static struct suite_set kunit_filter_suites(const struct suite_set *suite_set,
>>                 for (j = 0; j < filter_count; j++)
>>                         parsed_filters[j] = kunit_next_attr_filter(&filters, err);
>>                 if (*err)
>> -                       return filtered;
>> +                       goto err;
>>         }
>>
>>         for (i = 0; &suite_set->start[i] != suite_set->end; i++) {
>> @@ -163,7 +163,7 @@ static struct suite_set kunit_filter_suites(const struct suite_set *suite_set,
>>                                         parsed_glob.test_glob);
>>                         if (IS_ERR(filtered_suite)) {
>>                                 *err = PTR_ERR(filtered_suite);
>> -                               return filtered;
>> +                               goto err;
>>                         }
>>                 }
>>                 if (filter_count) {
>> @@ -172,15 +172,18 @@ static struct suite_set kunit_filter_suites(const struct suite_set *suite_set,
>>                                                 parsed_filters[k], filter_action, err);
>>
>>                                 /* Free previous copy of suite */
>> -                               if (k > 0 || filter_glob)
>> +                               if (k > 0 || filter_glob) {
>> +                                       kfree(filtered_suite->test_cases);
>>                                         kfree(filtered_suite);
>> +                               }
>> +
>>                                 filtered_suite = new_filtered_suite;
>>
>>                                 if (*err)
>> -                                       return filtered;
>> +                                       goto err;
>>                                 if (IS_ERR(filtered_suite)) {
>>                                         *err = PTR_ERR(filtered_suite);
>> -                                       return filtered;
>> +                                       goto err;
>>                                 }
>>                                 if (!filtered_suite)
>>                                         break;
>> @@ -194,6 +197,10 @@ static struct suite_set kunit_filter_suites(const struct suite_set *suite_set,
>>         }
>>         filtered.end = copy;
>>
>> +err:
>> +       if (*err)
>> +               kfree(copy);
>> +
>>         if (filter_glob) {
>>                 kfree(parsed_glob.suite_glob);
>>                 kfree(parsed_glob.test_glob);
>> --
>> 2.34.1
>>
>> --
>> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@xxxxxxxxxxxxxxxx.
>> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20230801073700.3740895-1-ruanjinjie%40huawei.com.



[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux