From 39e92cb1e4a1f6a12097ea2aa9e9ca6f2d2f8a83 Mon Sep 17 00:00:00 2001 From: Rae Moar Date: Tue, 25 Jul 2023 21:25:12 +0000 Subject: kunit: Add test attributes API structure Add the basic structure of the test attribute API to KUnit, which can be used to save and access test associated data. Add attributes.c and attributes.h to hold associated structs and functions for the API. Create a struct that holds a variety of associated helper functions for each test attribute. These helper functions will be used to get the attribute value, convert the value to a string, and filter based on the value. This struct is flexible by design to allow for attributes of numerous types and contexts. Add a method to print test attributes in the format of "# [.]: ". Example for a suite: "# speed: slow" Example for a test case: "# test_case.speed: very_slow" Use this method to report attributes in the KTAP output (KTAP spec: https://docs.kernel.org/dev-tools/ktap.html) and _list_tests output when kernel's new kunit.action=list_attr option is used. Note this is derivative of the kunit.action=list option. In test.h, add fields and associated helper functions to test cases and suites to hold user-inputted test attributes. Reviewed-by: David Gow Signed-off-by: Rae Moar Signed-off-by: Shuah Khan --- lib/kunit/executor.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) (limited to 'lib/kunit/executor.c') diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index 74982b83707c..12e38a48a5cc 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -24,7 +25,8 @@ module_param_named(action, action_param, charp, 0); MODULE_PARM_DESC(action, "Changes KUnit executor behavior, valid values are:\n" ": run the tests like normal\n" - "'list' to list test names instead of running them.\n"); + "'list' to list test names instead of running them.\n" + "'list_attr' to list test names and attributes instead of running them.\n"); /* glob_match() needs NULL terminated strings, so we need a copy of filter_glob_param. */ struct kunit_test_filter { @@ -172,7 +174,7 @@ static void kunit_exec_run_tests(struct suite_set *suite_set) __kunit_test_suites_init(suite_set->start, num_suites); } -static void kunit_exec_list_tests(struct suite_set *suite_set) +static void kunit_exec_list_tests(struct suite_set *suite_set, bool include_attr) { struct kunit_suite * const *suites; struct kunit_case *test_case; @@ -180,10 +182,19 @@ static void kunit_exec_list_tests(struct suite_set *suite_set) /* Hack: print a ktap header so kunit.py can find the start of KUnit output. */ pr_info("KTAP version 1\n"); - for (suites = suite_set->start; suites < suite_set->end; suites++) + for (suites = suite_set->start; suites < suite_set->end; suites++) { + /* Print suite name and suite attributes */ + pr_info("%s\n", (*suites)->name); + if (include_attr) + kunit_print_attr((void *)(*suites), false, 0); + + /* Print test case name and attributes in suite */ kunit_suite_for_each_test_case((*suites), test_case) { pr_info("%s.%s\n", (*suites)->name, test_case->name); + if (include_attr) + kunit_print_attr((void *)test_case, true, 0); } + } } int kunit_run_all_tests(void) @@ -206,7 +217,9 @@ int kunit_run_all_tests(void) if (!action_param) kunit_exec_run_tests(&suite_set); else if (strcmp(action_param, "list") == 0) - kunit_exec_list_tests(&suite_set); + kunit_exec_list_tests(&suite_set, false); + else if (strcmp(action_param, "list_attr") == 0) + kunit_exec_list_tests(&suite_set, true); else pr_err("kunit executor: unknown action '%s'\n", action_param); -- cgit From 529534e8cba3e60f843a682e2a3149612b30d608 Mon Sep 17 00:00:00 2001 From: Rae Moar Date: Tue, 25 Jul 2023 21:25:15 +0000 Subject: kunit: Add ability to filter attributes Add filtering of test attributes. Users can filter tests using the module_param called "filter". Filters are imputed in the format: Example: kunit.filter="speed>slow" Operations include: >, <, >=, <=, !=, and =. These operations will act the same for attributes of the same type but may not between types. Note multiple filters can be inputted by separating them with a comma. Example: kunit.filter="speed=slow, module!=example" Since both suites and test cases can have attributes, there may be conflicts. The process of filtering follows these rules: - Filtering always operates at a per-test level. - If a test has an attribute set, then the test's value is filtered on. - Otherwise, the value falls back to the suite's value. - If neither are set, the attribute has a global "default" value, which is used. Filtered tests will not be run or show in output. The tests can instead be skipped using the configurable option "kunit.filter_action=skip". Note the default settings for running tests remains unfiltered. Finally, add "filter" methods for the speed and module attributes to parse and compare attribute values. Note this filtering functionality will be added to kunit.py in the next patch. Reviewed-by: David Gow Signed-off-by: Rae Moar Signed-off-by: Shuah Khan --- lib/kunit/executor.c | 93 +++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 74 insertions(+), 19 deletions(-) (limited to 'lib/kunit/executor.c') diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index 12e38a48a5cc..483f7b7873a7 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -17,6 +17,8 @@ extern struct kunit_suite * const __kunit_suites_end[]; static char *filter_glob_param; static char *action_param; +static char *filter_param; +static char *filter_action_param; module_param_named(filter_glob, filter_glob_param, charp, 0); MODULE_PARM_DESC(filter_glob, @@ -27,15 +29,23 @@ MODULE_PARM_DESC(action, ": run the tests like normal\n" "'list' to list test names instead of running them.\n" "'list_attr' to list test names and attributes instead of running them.\n"); +module_param_named(filter, filter_param, charp, 0); +MODULE_PARM_DESC(filter, + "Filter which KUnit test suites/tests run at boot-time using attributes, e.g. speed>slow"); +module_param_named(filter_action, filter_action_param, charp, 0); +MODULE_PARM_DESC(filter_action, + "Changes behavior of filtered tests using attributes, valid values are:\n" + ": do not run filtered tests as normal\n" + "'skip': skip all filtered tests instead so tests will appear in output\n"); /* glob_match() needs NULL terminated strings, so we need a copy of filter_glob_param. */ -struct kunit_test_filter { +struct kunit_glob_filter { char *suite_glob; char *test_glob; }; /* Split "suite_glob.test_glob" into two. Assumes filter_glob is not empty. */ -static void kunit_parse_filter_glob(struct kunit_test_filter *parsed, +static void kunit_parse_glob_filter(struct kunit_glob_filter *parsed, const char *filter_glob) { const int len = strlen(filter_glob); @@ -57,7 +67,7 @@ static void kunit_parse_filter_glob(struct kunit_test_filter *parsed, /* Create a copy of suite with only tests that match test_glob. */ static struct kunit_suite * -kunit_filter_tests(const struct kunit_suite *const suite, const char *test_glob) +kunit_filter_glob_tests(const struct kunit_suite *const suite, const char *test_glob) { int n = 0; struct kunit_case *filtered, *test_case; @@ -111,12 +121,15 @@ static void kunit_free_suite_set(struct suite_set suite_set) static struct suite_set kunit_filter_suites(const struct suite_set *suite_set, const char *filter_glob, + char *filters, + char *filter_action, int *err) { - int i; - struct kunit_suite **copy, *filtered_suite; + int i, j, k, filter_count; + struct kunit_suite **copy, *filtered_suite, *new_filtered_suite; struct suite_set filtered; - struct kunit_test_filter filter; + struct kunit_glob_filter parsed_glob; + struct kunit_attr_filter *parsed_filters; const size_t max = suite_set->end - suite_set->start; @@ -127,17 +140,52 @@ static struct suite_set kunit_filter_suites(const struct suite_set *suite_set, return filtered; } - kunit_parse_filter_glob(&filter, filter_glob); + if (filter_glob) + kunit_parse_glob_filter(&parsed_glob, filter_glob); - for (i = 0; &suite_set->start[i] != suite_set->end; i++) { - if (!glob_match(filter.suite_glob, suite_set->start[i]->name)) - continue; - - filtered_suite = kunit_filter_tests(suite_set->start[i], filter.test_glob); - if (IS_ERR(filtered_suite)) { - *err = PTR_ERR(filtered_suite); + /* Parse attribute filters */ + if (filters) { + filter_count = kunit_get_filter_count(filters); + parsed_filters = kcalloc(filter_count + 1, sizeof(*parsed_filters), GFP_KERNEL); + for (j = 0; j < filter_count; j++) + parsed_filters[j] = kunit_next_attr_filter(&filters, err); + if (*err) return filtered; + } + + for (i = 0; &suite_set->start[i] != suite_set->end; i++) { + filtered_suite = suite_set->start[i]; + if (filter_glob) { + if (!glob_match(parsed_glob.suite_glob, filtered_suite->name)) + continue; + filtered_suite = kunit_filter_glob_tests(filtered_suite, + parsed_glob.test_glob); + if (IS_ERR(filtered_suite)) { + *err = PTR_ERR(filtered_suite); + return filtered; + } } + if (filter_count) { + for (k = 0; k < filter_count; k++) { + new_filtered_suite = kunit_filter_attr_tests(filtered_suite, + parsed_filters[k], filter_action, err); + + /* Free previous copy of suite */ + if (k > 0 || filter_glob) + kfree(filtered_suite); + filtered_suite = new_filtered_suite; + + if (*err) + return filtered; + if (IS_ERR(filtered_suite)) { + *err = PTR_ERR(filtered_suite); + return filtered; + } + if (!filtered_suite) + break; + } + } + if (!filtered_suite) continue; @@ -145,8 +193,14 @@ static struct suite_set kunit_filter_suites(const struct suite_set *suite_set, } filtered.end = copy; - kfree(filter.suite_glob); - kfree(filter.test_glob); + if (filter_glob) { + kfree(parsed_glob.suite_glob); + kfree(parsed_glob.test_glob); + } + + if (filter_count) + kfree(parsed_filters); + return filtered; } @@ -206,8 +260,9 @@ int kunit_run_all_tests(void) goto out; } - if (filter_glob_param) { - suite_set = kunit_filter_suites(&suite_set, filter_glob_param, &err); + if (filter_glob_param || filter_param) { + suite_set = kunit_filter_suites(&suite_set, filter_glob_param, + filter_param, filter_action_param, &err); if (err) { pr_err("kunit executor: error filtering suites: %d\n", err); goto out; @@ -223,7 +278,7 @@ int kunit_run_all_tests(void) else pr_err("kunit executor: unknown action '%s'\n", action_param); - if (filter_glob_param) { /* a copy was made of each suite */ + if (filter_glob_param || filter_param) { /* a copy was made of each suite */ kunit_free_suite_set(suite_set); } -- cgit From 5a175d369c702ce08c9feb630125c9fc7a9e1370 Mon Sep 17 00:00:00 2001 From: Ruan Jinjie Date: Sat, 29 Jul 2023 09:00:03 +0800 Subject: kunit: fix wild-memory-access bug in kunit_filter_suites() As for kunit_filter_suites(), When the filters arg = NULL, such as the call of kunit_filter_suites(&suite_set, "suite2", NULL, NULL, &err) in filter_suites_test() tese case in kunit, both filter_count and parsed_filters will not be initialized. So it's possible to enter kunit_filter_attr_tests(), and the use of uninitialized parsed_filters will cause below wild-memory-access. RIP: 0010:kunit_filter_suites+0x780/0xa40 Code: fe ff ff e8 42 87 4d ff 41 83 c6 01 49 83 c5 10 49 89 dc 44 39 74 24 50 0f 8e 81 fe ff ff e8 27 87 4d ff 4c 89 e8 48 c1 e8 03 <66> 42 83 3c 38 00 0f 85 af 01 00 00 49 8b 75 00 49 8b 55 08 4c 89 RSP: 0000:ff1100010743fc38 EFLAGS: 00010203 RAX: 03fc4400041d0ff1 RBX: ff1100010389a900 RCX: ffffffff9f940ad9 RDX: ff11000107429740 RSI: 0000000000000000 RDI: ff110001037ec920 RBP: ff1100010743fd50 R08: 0000000000000000 R09: ffe21c0020e87f1e R10: 0000000000000003 R11: 0000000000032001 R12: ff110001037ec800 R13: 1fe2200020e87f8c R14: 0000000000000000 R15: dffffc0000000000 FS: 0000000000000000(0000) GS:ff1100011b000000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: ff11000115201000 CR3: 0000000113066001 CR4: 0000000000771ef0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 PKRU: 55555554 Call Trace: ? die_addr+0x3c/0xa0 ? exc_general_protection+0x148/0x220 ? asm_exc_general_protection+0x26/0x30 ? kunit_filter_suites+0x779/0xa40 ? kunit_filter_suites+0x780/0xa40 ? kunit_filter_suites+0x779/0xa40 ? __pfx_kunit_filter_suites+0x10/0x10 ? __pfx_kfree+0x10/0x10 ? kunit_add_action_or_reset+0x3d/0x50 filter_suites_test+0x1b7/0x440 ? __pfx_filter_suites_test+0x10/0x10 ? __pfx___schedule+0x10/0x10 ? try_to_wake_up+0xa8e/0x1210 ? _raw_spin_lock_irqsave+0x86/0xe0 ? __pfx__raw_spin_lock_irqsave+0x10/0x10 ? set_cpus_allowed_ptr+0x7c/0xb0 kunit_try_run_case+0x119/0x270 ? __kthread_parkme+0xdc/0x160 ? __pfx_kunit_try_run_case+0x10/0x10 kunit_generic_run_threadfn_adapter+0x4e/0xa0 ? __pfx_kunit_generic_run_threadfn_adapter+0x10/0x10 kthread+0x2c7/0x3c0 ? __pfx_kthread+0x10/0x10 ret_from_fork+0x2c/0x70 ? __pfx_kthread+0x10/0x10 ret_from_fork_asm+0x1b/0x30 Modules linked in: Dumping ftrace buffer: (ftrace buffer empty) ---[ end trace 0000000000000000 ]--- RIP: 0010:kunit_filter_suites+0x780/0xa40 Code: fe ff ff e8 42 87 4d ff 41 83 c6 01 49 83 c5 10 49 89 dc 44 39 74 24 50 0f 8e 81 fe ff ff e8 27 87 4d ff 4c 89 e8 48 c1 e8 03 <66> 42 83 3c 38 00 0f 85 af 01 00 00 49 8b 75 00 49 8b 55 08 4c 89 RSP: 0000:ff1100010743fc38 EFLAGS: 00010203 RAX: 03fc4400041d0ff1 RBX: ff1100010389a900 RCX: ffffffff9f940ad9 RDX: ff11000107429740 RSI: 0000000000000000 RDI: ff110001037ec920 RBP: ff1100010743fd50 R08: 0000000000000000 R09: ffe21c0020e87f1e R10: 0000000000000003 R11: 0000000000032001 R12: ff110001037ec800 R13: 1fe2200020e87f8c R14: 0000000000000000 R15: dffffc0000000000 FS: 0000000000000000(0000) GS:ff1100011b000000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: ff11000115201000 CR3: 0000000113066001 CR4: 0000000000771ef0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 PKRU: 55555554 Kernel panic - not syncing: Fatal exception Dumping ftrace buffer: (ftrace buffer empty) Kernel Offset: 0x1da00000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff) Rebooting in 1 seconds.. Fixes: 529534e8cba3 ("kunit: Add ability to filter attributes") Signed-off-by: Ruan Jinjie Reviewed-by: David Gow Tested-by: Guenter Roeck Signed-off-by: Shuah Khan --- lib/kunit/executor.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'lib/kunit/executor.c') diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index 483f7b7873a7..5b5bed1efb93 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -125,7 +125,8 @@ static struct suite_set kunit_filter_suites(const struct suite_set *suite_set, char *filter_action, int *err) { - int i, j, k, filter_count; + int i, j, k; + int filter_count = 0; struct kunit_suite **copy, *filtered_suite, *new_filtered_suite; struct suite_set filtered; struct kunit_glob_filter parsed_glob; -- cgit From abbf73816b6f5f4268fbfb3b3505003c2356d4a9 Mon Sep 17 00:00:00 2001 From: Ruan Jinjie Date: Tue, 1 Aug 2023 15:37:00 +0800 Subject: kunit: fix possible memory leak in kunit_filter_suites() 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 Reviewed-by: Rae Moar Reviewed-by: David Gow Signed-off-by: Shuah Khan --- lib/kunit/executor.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) (limited to 'lib/kunit/executor.c') 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); -- cgit From 1c9fd080dffe5e5ad763527fbc2aa3f6f8c653e9 Mon Sep 17 00:00:00 2001 From: Rae Moar Date: Thu, 3 Aug 2023 19:36:35 +0000 Subject: kunit: fix uninitialized variables bug in attributes filtering Fix smatch warnings regarding uninitialized variables in the filtering patch of the new KUnit Attributes feature. Fixes: 529534e8cba3 ("kunit: Add ability to filter attributes") Reported-by: kernel test robot Reported-by: Dan Carpenter Closes: https://lore.kernel.org/r/202307270610.s0w4NKEn-lkp@intel.com/ Signed-off-by: Rae Moar Reviewed-by: David Gow Signed-off-by: Shuah Khan --- lib/kunit/executor.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) (limited to 'lib/kunit/executor.c') diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index 481901d245d0..dc295150c4e5 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -127,19 +127,18 @@ static struct suite_set kunit_filter_suites(const struct suite_set *suite_set, { int i, j, k; int filter_count = 0; - struct kunit_suite **copy, *filtered_suite, *new_filtered_suite; - struct suite_set filtered; + struct kunit_suite **copy, **copy_start, *filtered_suite, *new_filtered_suite; + struct suite_set filtered = {NULL, NULL}; struct kunit_glob_filter parsed_glob; - struct kunit_attr_filter *parsed_filters; + struct kunit_attr_filter *parsed_filters = NULL; const size_t max = suite_set->end - suite_set->start; copy = kmalloc_array(max, sizeof(*filtered.start), GFP_KERNEL); - filtered.start = copy; if (!copy) { /* won't be able to run anything, return an empty set */ - filtered.end = copy; return filtered; } + copy_start = copy; if (filter_glob) kunit_parse_glob_filter(&parsed_glob, filter_glob); @@ -147,7 +146,11 @@ static struct suite_set kunit_filter_suites(const struct suite_set *suite_set, /* Parse attribute filters */ if (filters) { filter_count = kunit_get_filter_count(filters); - parsed_filters = kcalloc(filter_count + 1, sizeof(*parsed_filters), GFP_KERNEL); + parsed_filters = kcalloc(filter_count, sizeof(*parsed_filters), GFP_KERNEL); + if (!parsed_filters) { + kfree(copy); + return filtered; + } for (j = 0; j < filter_count; j++) parsed_filters[j] = kunit_next_attr_filter(&filters, err); if (*err) @@ -166,7 +169,7 @@ static struct suite_set kunit_filter_suites(const struct suite_set *suite_set, goto err; } } - if (filter_count) { + if (filter_count > 0 && parsed_filters != NULL) { for (k = 0; k < filter_count; k++) { new_filtered_suite = kunit_filter_attr_tests(filtered_suite, parsed_filters[k], filter_action, err); @@ -195,6 +198,7 @@ static struct suite_set kunit_filter_suites(const struct suite_set *suite_set, *copy++ = filtered_suite; } + filtered.start = copy_start; filtered.end = copy; err: -- cgit From c95e7c05c139b1a8a51d368bde57cf20ce931a98 Mon Sep 17 00:00:00 2001 From: Janusz Krzysztofik Date: Mon, 7 Aug 2023 12:23:54 +0200 Subject: kunit: Report the count of test suites in a module According to KTAP specification[1], results should always start from a header that provides a TAP protocol version, followed by a test plan with a count of items to be executed. That pattern should be followed at each nesting level. In the current implementation of the top-most, i.e., test suite level, those rules apply only for test suites built into the kernel, executed and reported on boot. Results submitted to dmesg from kunit test modules loaded later are missing those top-level headers. As a consequence, if a kunit test module provides more than one test suite then, without the top level test plan, external tools that are parsing dmesg for kunit test output are not able to tell how many test suites should be expected and whether to continue parsing after complete output from the first test suite is collected. Submit the top-level headers also from the kunit test module notifier initialization callback. v3: Fix new name of a structure moved to kunit namespace not updated in executor_test functions (lkp@intel.com). v2: Use kunit_exec_run_tests() (Mauro, Rae), but prevent it from emitting the headers when called on load of non-test modules. [1] https://docs.kernel.org/dev-tools/ktap.html# Signed-off-by: Janusz Krzysztofik Cc: Mauro Carvalho Chehab Cc: Rae Moar Reviewed-by: Rae Moar Reviewed-by: David Gow Signed-off-by: Shuah Khan --- lib/kunit/executor.c | 42 +++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) (limited to 'lib/kunit/executor.c') diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index dc295150c4e5..5ef90c334eb0 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -104,13 +104,7 @@ kunit_filter_glob_tests(const struct kunit_suite *const suite, const char *test_ static char *kunit_shutdown; core_param(kunit_shutdown, kunit_shutdown, charp, 0644); -/* Stores an array of suites, end points one past the end */ -struct suite_set { - struct kunit_suite * const *start; - struct kunit_suite * const *end; -}; - -static void kunit_free_suite_set(struct suite_set suite_set) +static void kunit_free_suite_set(struct kunit_suite_set suite_set) { struct kunit_suite * const *suites; @@ -119,16 +113,17 @@ static void kunit_free_suite_set(struct suite_set suite_set) kfree(suite_set.start); } -static struct suite_set kunit_filter_suites(const struct suite_set *suite_set, - const char *filter_glob, - char *filters, - char *filter_action, - int *err) +static struct kunit_suite_set +kunit_filter_suites(const struct kunit_suite_set *suite_set, + const char *filter_glob, + char *filters, + char *filter_action, + int *err) { int i, j, k; int filter_count = 0; struct kunit_suite **copy, **copy_start, *filtered_suite, *new_filtered_suite; - struct suite_set filtered = {NULL, NULL}; + struct kunit_suite_set filtered = {NULL, NULL}; struct kunit_glob_filter parsed_glob; struct kunit_attr_filter *parsed_filters = NULL; @@ -230,17 +225,24 @@ static void kunit_handle_shutdown(void) } -static void kunit_exec_run_tests(struct suite_set *suite_set) +#endif + +void kunit_exec_run_tests(struct kunit_suite_set *suite_set, bool builtin) { size_t num_suites = suite_set->end - suite_set->start; - pr_info("KTAP version 1\n"); - pr_info("1..%zu\n", num_suites); + if (builtin || num_suites) { + pr_info("KTAP version 1\n"); + pr_info("1..%zu\n", num_suites); + } __kunit_test_suites_init(suite_set->start, num_suites); } -static void kunit_exec_list_tests(struct suite_set *suite_set, bool include_attr) +#if IS_BUILTIN(CONFIG_KUNIT) + +static void kunit_exec_list_tests(struct kunit_suite_set *suite_set, + bool include_attr) { struct kunit_suite * const *suites; struct kunit_case *test_case; @@ -265,7 +267,9 @@ static void kunit_exec_list_tests(struct suite_set *suite_set, bool include_attr int kunit_run_all_tests(void) { - struct suite_set suite_set = {__kunit_suites_start, __kunit_suites_end}; + struct kunit_suite_set suite_set = { + __kunit_suites_start, __kunit_suites_end, + }; int err = 0; if (!kunit_enabled()) { pr_info("kunit: disabled\n"); @@ -282,7 +286,7 @@ int kunit_run_all_tests(void) } if (!action_param) - kunit_exec_run_tests(&suite_set); + kunit_exec_run_tests(&suite_set, true); else if (strcmp(action_param, "list") == 0) kunit_exec_list_tests(&suite_set, false); else if (strcmp(action_param, "list_attr") == 0) -- cgit From 18258c60f8a74b9c39b593d118f13f3265d44cd6 Mon Sep 17 00:00:00 2001 From: Janusz Krzysztofik Date: Mon, 7 Aug 2023 12:23:55 +0200 Subject: kunit: Make 'list' action available to kunit test modules Results from kunit tests reported via dmesg may be interleaved with other kernel messages. When parsing dmesg for modular kunit results in real time, external tools, e.g., Intel GPU tools (IGT), may want to insert their own test name markers into dmesg at the start of each test, before any kernel message related to that test appears there, so existing upper level test result parsers have no doubt which test to blame for a specific kernel message. Unfortunately, kunit reports names of tests only at their completion (with the exeption of a not standarized "# Subtest: " header above a test plan of each test suite or parametrized test). External tools could be able to insert their own "start of the test" markers with test names included if they new those names in advance. Test names could be learned from a list if provided by a kunit test module. There exists a feature of listing kunit tests without actually executing them, but it is now limited to configurations with the kunit module built in and covers only built-in tests, already available at boot time. Moreover, switching from list to normal mode requires reboot. If that feature was also available when kunit is built as a module, userspace could load the module with action=list parameter, load some kunit test modules they are interested in and learn about the list of tests provided by those modules, then unload them, reload the kunit module in normal mode and execute the tests with their lists already known. Extend kunit module notifier initialization callback with a processing path for only listing the tests provided by a module if the kunit action parameter is set to "list" or "list_attr". For user convenience, make the kunit.action parameter visible in sysfs. v2: Don't use a different format, use kunit_exec_list_tests() (Rae), - refresh on top of new attributes patches, handle newly introduced kunit.action=list_attr case (Rae). Signed-off-by: Janusz Krzysztofik Cc: Rae Moar Reviewed-by: David Gow Signed-off-by: Shuah Khan --- lib/kunit/executor.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) (limited to 'lib/kunit/executor.c') diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index 5ef90c334eb0..e877c1f1e75c 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -13,22 +13,29 @@ extern struct kunit_suite * const __kunit_suites_start[]; extern struct kunit_suite * const __kunit_suites_end[]; +static char *action_param; + +module_param_named(action, action_param, charp, 0400); +MODULE_PARM_DESC(action, + "Changes KUnit executor behavior, valid values are:\n" + ": run the tests like normal\n" + "'list' to list test names instead of running them.\n" + "'list_attr' to list test names and attributes instead of running them.\n"); + +const char *kunit_action(void) +{ + return action_param; +} + #if IS_BUILTIN(CONFIG_KUNIT) static char *filter_glob_param; -static char *action_param; static char *filter_param; static char *filter_action_param; module_param_named(filter_glob, filter_glob_param, charp, 0); MODULE_PARM_DESC(filter_glob, "Filter which KUnit test suites/tests run at boot-time, e.g. list* or list*.*del_test"); -module_param_named(action, action_param, charp, 0); -MODULE_PARM_DESC(action, - "Changes KUnit executor behavior, valid values are:\n" - ": run the tests like normal\n" - "'list' to list test names instead of running them.\n" - "'list_attr' to list test names and attributes instead of running them.\n"); module_param_named(filter, filter_param, charp, 0); MODULE_PARM_DESC(filter, "Filter which KUnit test suites/tests run at boot-time using attributes, e.g. speed>slow"); @@ -239,10 +246,7 @@ void kunit_exec_run_tests(struct kunit_suite_set *suite_set, bool builtin) __kunit_test_suites_init(suite_set->start, num_suites); } -#if IS_BUILTIN(CONFIG_KUNIT) - -static void kunit_exec_list_tests(struct kunit_suite_set *suite_set, - bool include_attr) +void kunit_exec_list_tests(struct kunit_suite_set *suite_set, bool include_attr) { struct kunit_suite * const *suites; struct kunit_case *test_case; @@ -265,6 +269,8 @@ static void kunit_exec_list_tests(struct kunit_suite_set *suite_set, } } +#if IS_BUILTIN(CONFIG_KUNIT) + int kunit_run_all_tests(void) { struct kunit_suite_set suite_set = { -- cgit From b67abaad4d25b5d9364a1d4f6bc18286ebaaa013 Mon Sep 17 00:00:00 2001 From: Janusz Krzysztofik Date: Mon, 7 Aug 2023 12:23:56 +0200 Subject: kunit: Allow kunit test modules to use test filtering External tools, e.g., Intel GPU tools (IGT), support execution of individual selftests provided by kernel modules. That could be also applicable to kunit test modules if they provided test filtering. But test filtering is now possible only when kunit code is built into the kernel. Moreover, a filter can be specified only at boot time, then reboot is required each time a different filter is needed. Build the test filtering code also when kunit is configured as a module, expose test filtering functions to other kunit source files, and use them in kunit module notifier callback functions. Userspace can then reload the kunit module with a value of the filter_glob parameter tuned to a specific kunit test module every time it wants to limit the scope of tests executed on that module load. Make the kunit.filter* parameters visible in sysfs for user convenience. v5: Refresh on tpp of attributes filtering fix v4: Refresh on top of newly applied attributes patches and changes introdced by new versions of other patches submitted in series with this one. v3: Fix CONFIG_GLOB, required by filtering functions, not selected when building as a module (lkp@intel.com). v2: Fix new name of a structure moved to kunit namespace not updated across all uses (lkp@intel.com). Signed-off-by: Janusz Krzysztofik Reviewed-by: David Gow Signed-off-by: Shuah Khan --- lib/kunit/executor.c | 63 ++++++++++++++++++++++++++++++---------------------- 1 file changed, 37 insertions(+), 26 deletions(-) (limited to 'lib/kunit/executor.c') diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index e877c1f1e75c..5181aa2e760b 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -27,24 +27,37 @@ const char *kunit_action(void) return action_param; } -#if IS_BUILTIN(CONFIG_KUNIT) - static char *filter_glob_param; static char *filter_param; static char *filter_action_param; -module_param_named(filter_glob, filter_glob_param, charp, 0); +module_param_named(filter_glob, filter_glob_param, charp, 0400); MODULE_PARM_DESC(filter_glob, "Filter which KUnit test suites/tests run at boot-time, e.g. list* or list*.*del_test"); -module_param_named(filter, filter_param, charp, 0); +module_param_named(filter, filter_param, charp, 0400); MODULE_PARM_DESC(filter, "Filter which KUnit test suites/tests run at boot-time using attributes, e.g. speed>slow"); -module_param_named(filter_action, filter_action_param, charp, 0); +module_param_named(filter_action, filter_action_param, charp, 0400); MODULE_PARM_DESC(filter_action, "Changes behavior of filtered tests using attributes, valid values are:\n" ": do not run filtered tests as normal\n" "'skip': skip all filtered tests instead so tests will appear in output\n"); +const char *kunit_filter_glob(void) +{ + return filter_glob_param; +} + +char *kunit_filter(void) +{ + return filter_param; +} + +char *kunit_filter_action(void) +{ + return filter_action_param; +} + /* glob_match() needs NULL terminated strings, so we need a copy of filter_glob_param. */ struct kunit_glob_filter { char *suite_glob; @@ -108,10 +121,7 @@ kunit_filter_glob_tests(const struct kunit_suite *const suite, const char *test_ return copy; } -static char *kunit_shutdown; -core_param(kunit_shutdown, kunit_shutdown, charp, 0644); - -static void kunit_free_suite_set(struct kunit_suite_set suite_set) +void kunit_free_suite_set(struct kunit_suite_set suite_set) { struct kunit_suite * const *suites; @@ -120,7 +130,7 @@ static void kunit_free_suite_set(struct kunit_suite_set suite_set) kfree(suite_set.start); } -static struct kunit_suite_set +struct kunit_suite_set kunit_filter_suites(const struct kunit_suite_set *suite_set, const char *filter_glob, char *filters, @@ -218,22 +228,6 @@ err: return filtered; } -static void kunit_handle_shutdown(void) -{ - if (!kunit_shutdown) - return; - - if (!strcmp(kunit_shutdown, "poweroff")) - kernel_power_off(); - else if (!strcmp(kunit_shutdown, "halt")) - kernel_halt(); - else if (!strcmp(kunit_shutdown, "reboot")) - kernel_restart(NULL); - -} - -#endif - void kunit_exec_run_tests(struct kunit_suite_set *suite_set, bool builtin) { size_t num_suites = suite_set->end - suite_set->start; @@ -271,6 +265,23 @@ void kunit_exec_list_tests(struct kunit_suite_set *suite_set, bool include_attr) #if IS_BUILTIN(CONFIG_KUNIT) +static char *kunit_shutdown; +core_param(kunit_shutdown, kunit_shutdown, charp, 0644); + +static void kunit_handle_shutdown(void) +{ + if (!kunit_shutdown) + return; + + if (!strcmp(kunit_shutdown, "poweroff")) + kernel_power_off(); + else if (!strcmp(kunit_shutdown, "halt")) + kernel_halt(); + else if (!strcmp(kunit_shutdown, "reboot")) + kernel_restart(NULL); + +} + int kunit_run_all_tests(void) { struct kunit_suite_set suite_set = { -- cgit From 4b00920da1dd2bbb33baeb2e7b9808af4c68de97 Mon Sep 17 00:00:00 2001 From: Jinjie Ruan Date: Sun, 3 Sep 2023 15:10:26 +0800 Subject: kunit: Fix the wrong err path and add goto labels in kunit_filter_suites() Take the last kfree(parsed_filters) and add it to be the first. Take the first kfree(copy) and add it to be the last. The Best practice is to return these errors reversely. And as David suggested, add several labels which target only the things which actually have been allocated so far. Fixes: 529534e8cba3 ("kunit: Add ability to filter attributes") Fixes: abbf73816b6f ("kunit: fix possible memory leak in kunit_filter_suites()") Signed-off-by: Jinjie Ruan Reviewed-by: Rae Moar Suggested-by: David Gow Reviewed-by: David Gow Signed-off-by: Shuah Khan --- lib/kunit/executor.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) (limited to 'lib/kunit/executor.c') diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index 5181aa2e760b..0eda42b0c9bb 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -166,7 +166,7 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set, for (j = 0; j < filter_count; j++) parsed_filters[j] = kunit_next_attr_filter(&filters, err); if (*err) - goto err; + goto free_parsed_filters; } for (i = 0; &suite_set->start[i] != suite_set->end; i++) { @@ -178,7 +178,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 err; + goto free_parsed_filters; } } if (filter_count > 0 && parsed_filters != NULL) { @@ -195,10 +195,11 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set, filtered_suite = new_filtered_suite; if (*err) - goto err; + goto free_parsed_filters; + if (IS_ERR(filtered_suite)) { *err = PTR_ERR(filtered_suite); - goto err; + goto free_parsed_filters; } if (!filtered_suite) break; @@ -213,17 +214,19 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set, filtered.start = copy_start; filtered.end = copy; -err: - if (*err) - kfree(copy); +free_parsed_filters: + if (filter_count) + kfree(parsed_filters); +free_parsed_glob: if (filter_glob) { kfree(parsed_glob.suite_glob); kfree(parsed_glob.test_glob); } - if (filter_count) - kfree(parsed_filters); +free_copy: + if (*err) + kfree(copy); return filtered; } -- cgit From 2b56a4b79b7b3086e842d39611db4e19b19dbe2a Mon Sep 17 00:00:00 2001 From: Jinjie Ruan Date: Sun, 3 Sep 2023 15:10:27 +0800 Subject: kunit: Fix possible null-ptr-deref in kunit_parse_glob_filter() Inject fault while probing kunit-example-test.ko, if kzalloc fails in kunit_parse_glob_filter(), strcpy() or strncpy() to NULL will cause below null-ptr-deref bug. So check NULL for kzalloc() and return int instead of void for kunit_parse_glob_filter(). Unable to handle kernel paging request at virtual address dfff800000000000 KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007] Mem abort info: ESR = 0x0000000096000005 EC = 0x25: DABT (current EL), IL = 32 bits SET = 0, FnV = 0 EA = 0, S1PTW = 0 FSC = 0x05: level 1 translation fault Data abort info: ISV = 0, ISS = 0x00000005, ISS2 = 0x00000000 CM = 0, WnR = 0, TnD = 0, TagAccess = 0 GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 [dfff800000000000] address between user and kernel address ranges Internal error: Oops: 0000000096000005 [#1] PREEMPT SMP Modules linked in: kunit_example_test cfg80211 rfkill 8021q garp mrp stp llc ipv6 [last unloaded: kunit_example_test] CPU: 4 PID: 6047 Comm: modprobe Tainted: G W N 6.5.0-next-20230829+ #141 Hardware name: linux,dummy-virt (DT) pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) pc : strncpy+0x58/0xc0 lr : kunit_filter_suites+0x15c/0xa84 sp : ffff800082a17420 x29: ffff800082a17420 x28: 0000000000000000 x27: 0000000000000004 x26: 0000000000000000 x25: ffffa847e40a5320 x24: 0000000000000001 x23: 0000000000000000 x22: 0000000000000001 x21: dfff800000000000 x20: 000000000000002a x19: 0000000000000000 x18: 00000000750b3b54 x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000 x14: 0000000000000000 x13: 34393178302f3039 x12: ffff7508fcea4ec1 x11: 1ffff508fcea4ec0 x10: ffff7508fcea4ec0 x9 : dfff800000000000 x8 : ffff6051b1a7f86a x7 : ffff800082a17270 x6 : 0000000000000002 x5 : 0000000000000098 x4 : ffff028d9817b250 x3 : 0000000000000000 x2 : 0000000000000000 x1 : ffffa847e40a5320 x0 : 0000000000000000 Call trace: strncpy+0x58/0xc0 kunit_filter_suites+0x15c/0xa84 kunit_module_notify+0x1b0/0x3ac blocking_notifier_call_chain+0xc4/0x128 do_init_module+0x250/0x594 load_module+0x37b0/0x44b4 init_module_from_file+0xd4/0x128 idempotent_init_module+0x2c8/0x524 __arm64_sys_finit_module+0xac/0x100 invoke_syscall+0x6c/0x258 el0_svc_common.constprop.0+0x160/0x22c do_el0_svc+0x44/0x5c el0_svc+0x38/0x78 el0t_64_sync_handler+0x13c/0x158 el0t_64_sync+0x190/0x194 Code: 5400028a d343fe63 12000a62 39400034 (38f56863) ---[ end trace 0000000000000000 ]--- Kernel panic - not syncing: Oops: Fatal exception SMP: stopping secondary CPUs Kernel Offset: 0x284761400000 from 0xffff800080000000 PHYS_OFFSET: 0xfffffd7380000000 CPU features: 0x88000203,3c020000,1000421b Memory Limit: none Rebooting in 1 seconds.. 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 | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) (limited to 'lib/kunit/executor.c') diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index 0eda42b0c9bb..28f144de748b 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -65,7 +65,7 @@ struct kunit_glob_filter { }; /* Split "suite_glob.test_glob" into two. Assumes filter_glob is not empty. */ -static void kunit_parse_glob_filter(struct kunit_glob_filter *parsed, +static int kunit_parse_glob_filter(struct kunit_glob_filter *parsed, const char *filter_glob) { const int len = strlen(filter_glob); @@ -73,16 +73,28 @@ static void kunit_parse_glob_filter(struct kunit_glob_filter *parsed, if (!period) { parsed->suite_glob = kzalloc(len + 1, GFP_KERNEL); + if (!parsed->suite_glob) + return -ENOMEM; + parsed->test_glob = NULL; strcpy(parsed->suite_glob, filter_glob); - return; + return 0; } parsed->suite_glob = kzalloc(period - filter_glob + 1, GFP_KERNEL); + if (!parsed->suite_glob) + return -ENOMEM; + parsed->test_glob = kzalloc(len - (period - filter_glob) + 1, GFP_KERNEL); + if (!parsed->test_glob) { + kfree(parsed->suite_glob); + return -ENOMEM; + } strncpy(parsed->suite_glob, filter_glob, period - filter_glob); strncpy(parsed->test_glob, period + 1, len - (period - filter_glob)); + + return 0; } /* Create a copy of suite with only tests that match test_glob. */ @@ -152,8 +164,11 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set, } copy_start = copy; - if (filter_glob) - kunit_parse_glob_filter(&parsed_glob, filter_glob); + if (filter_glob) { + *err = kunit_parse_glob_filter(&parsed_glob, filter_glob); + if (*err) + goto free_copy; + } /* Parse attribute filters */ if (filters) { -- cgit From 9076bc476d7ebf0565903c4b048442131825c1c3 Mon Sep 17 00:00:00 2001 From: Jinjie Ruan Date: Sun, 3 Sep 2023 15:10:28 +0800 Subject: kunit: Fix possible memory leak in kunit_filter_suites() If both filter_glob and filters are not NULL, and kunit_parse_glob_filter() succeed, but kcalloc parsed_filters fails, the suite_glob and test_glob of parsed kzalloc in kunit_parse_glob_filter() will be leaked. As Rae suggested, assign -ENOMEM to *err to correctly free copy and goto free_parsed_glob to free the suite/test_glob of parsed. Fixes: 1c9fd080dffe ("kunit: fix uninitialized variables bug in attributes filtering") Signed-off-by: Jinjie Ruan Suggested-by: Rae Moar Reviewed-by: David Gow Signed-off-by: Shuah Khan --- lib/kunit/executor.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/kunit/executor.c') diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index 28f144de748b..a6348489d45f 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -175,8 +175,8 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set, filter_count = kunit_get_filter_count(filters); parsed_filters = kcalloc(filter_count, sizeof(*parsed_filters), GFP_KERNEL); if (!parsed_filters) { - kfree(copy); - return filtered; + *err = -ENOMEM; + goto free_parsed_glob; } for (j = 0; j < filter_count; j++) parsed_filters[j] = kunit_next_attr_filter(&filters, err); -- cgit