August 2023 - Linux-kselftest-mirror

[PATCH -next v2] kunit: fix uninitialized variables bug in attributes filtering

by Rae Moar

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 <lkp(a)intel.com> Reported-by: Dan Carpenter <dan.carpenter(a)linaro.org> Closes: https://lore.kernel.org/r/202307270610.s0w4NKEn-lkp@intel.com/ Signed-off-by: Rae Moar <rmoar(a)google.com> --- Change since v1: - Changed initialization of filtered to be {NULL, NULL} at the start and only updated when ready to return. - Remove unnecessary +1 in memory allocation for parsed_filters in executor test. Note that this is rebased on top of the recent fix: ("kunit: fix possible memory leak in kunit_filter_suites()"). lib/kunit/attributes.c | 40 +++++++++++++++++---------------------- lib/kunit/executor.c | 18 +++++++++++------- lib/kunit/executor_test.c | 2 +- 3 files changed, 29 insertions(+), 31 deletions(-) diff --git a/lib/kunit/attributes.c b/lib/kunit/attributes.c index d37c40c0ce4f..5e3034b6be99 100644 --- a/lib/kunit/attributes.c +++ b/lib/kunit/attributes.c @@ -102,7 +102,7 @@ static int int_filter(long val, const char *op, int input, int *err) static int attr_enum_filter(void *attr, const char *input, int *err, const char * const str_list[], int max) { - int i, j, input_int; + int i, j, input_int = -1; long test_val = (long)attr; const char *input_val = NULL; @@ -124,7 +124,7 @@ static int attr_enum_filter(void *attr, const char *input, int *err, input_int = j; } - if (!input_int) { + if (input_int < 0) { *err = -EINVAL; pr_err("kunit executor: invalid filter input: %s\n", input); return false; @@ -186,8 +186,10 @@ static void *attr_module_get(void *test_or_suite, bool is_test) // Suites get their module attribute from their first test_case if (test) return ((void *) test->module_name); - else + else if (kunit_suite_num_test_cases(suite) > 0) return ((void *) suite->test_cases[0].module_name); + else + return (void *) ""; } /* List of all Test Attributes */ @@ -221,7 +223,7 @@ const char *kunit_attr_filter_name(struct kunit_attr_filter filter) void kunit_print_attr(void *test_or_suite, bool is_test, unsigned int test_level) { int i; - bool to_free; + bool to_free = false; void *attr; const char *attr_name, *attr_str; struct kunit_suite *suite = is_test ? NULL : test_or_suite; @@ -255,7 +257,7 @@ void kunit_print_attr(void *test_or_suite, bool is_test, unsigned int test_level int kunit_get_filter_count(char *input) { - int i, comma_index, count = 0; + int i, comma_index = 0, count = 0; for (i = 0; input[i]; i++) { if (input[i] == ',') { @@ -272,7 +274,7 @@ int kunit_get_filter_count(char *input) struct kunit_attr_filter kunit_next_attr_filter(char **filters, int *err) { struct kunit_attr_filter filter = {}; - int i, j, comma_index, new_start_index; + int i, j, comma_index = 0, new_start_index = 0; int op_index = -1, attr_index = -1; char op; char *input = *filters; @@ -316,7 +318,7 @@ struct kunit_attr_filter kunit_next_attr_filter(char **filters, int *err) filter.attr = &kunit_attr_list[attr_index]; } - if (comma_index) { + if (comma_index > 0) { input[comma_index] = '\0'; filter.input = input + op_index; input = input + new_start_index; @@ -356,31 +358,22 @@ struct kunit_suite *kunit_filter_attr_tests(const struct kunit_suite *const suit /* Save filtering result on default value */ default_result = filter.attr->filter(filter.attr->attr_default, filter.input, err); - if (*err) { - kfree(copy); - kfree(filtered); - return NULL; - } + if (*err) + goto err; /* Save suite attribute value and filtering result on that value */ suite_val = filter.attr->get_attr((void *)suite, false); suite_result = filter.attr->filter(suite_val, filter.input, err); - if (*err) { - kfree(copy); - kfree(filtered); - return NULL; - } + if (*err) + goto err; /* For each test case, save test case if passes filtering. */ kunit_suite_for_each_test_case(suite, test_case) { test_val = filter.attr->get_attr((void *) test_case, true); test_result = filter.attr->filter(filter.attr->get_attr(test_case, true), filter.input, err); - if (*err) { - kfree(copy); - kfree(filtered); - return NULL; - } + if (*err) + goto err; /* * If attribute value of test case is set, filter on that value. @@ -406,7 +399,8 @@ struct kunit_suite *kunit_filter_attr_tests(const struct kunit_suite *const suit } } - if (n == 0) { +err: + if (n == 0 || *err) { kfree(copy); kfree(filtered); return NULL; 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: diff --git a/lib/kunit/executor_test.c b/lib/kunit/executor_test.c index 01280cb8d451..3e0a1c99cb4e 100644 --- a/lib/kunit/executor_test.c +++ b/lib/kunit/executor_test.c @@ -119,7 +119,7 @@ static void parse_filter_attr_test(struct kunit *test) filter_count = kunit_get_filter_count(filters); KUNIT_EXPECT_EQ(test, filter_count, 2); - parsed_filters = kunit_kcalloc(test, filter_count + 1, sizeof(*parsed_filters), + parsed_filters = kunit_kcalloc(test, filter_count, sizeof(*parsed_filters), GFP_KERNEL); for (j = 0; j < filter_count; j++) { parsed_filters[j] = kunit_next_attr_filter(&filters, &err); base-commit: 3bffe185ad11e408903d2782727877388d08d94e -- 2.41.0.585.gd2178a4bd4-goog

1 year, 4 months

2
1
0 0

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

by Ruan Jinjie

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(a)huawei.com> --- 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

1 year, 4 months

3
3
0 0

[PATCH -next] kunit: fix uninitialized variables bug in attributes filtering

by Rae Moar

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 <lkp(a)intel.com> Reported-by: Dan Carpenter <dan.carpenter(a)linaro.org> Closes: https://lore.kernel.org/r/202307270610.s0w4NKEn-lkp@intel.com/ Signed-off-by: Rae Moar <rmoar(a)google.com> --- Note that this is rebased on top of the recent fix: ("kunit: fix possible memory leak in kunit_filter_suites()"). lib/kunit/attributes.c | 40 +++++++++++++++++----------------------- lib/kunit/executor.c | 10 +++++++--- 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/lib/kunit/attributes.c b/lib/kunit/attributes.c index d37c40c0ce4f..5e3034b6be99 100644 --- a/lib/kunit/attributes.c +++ b/lib/kunit/attributes.c @@ -102,7 +102,7 @@ static int int_filter(long val, const char *op, int input, int *err) static int attr_enum_filter(void *attr, const char *input, int *err, const char * const str_list[], int max) { - int i, j, input_int; + int i, j, input_int = -1; long test_val = (long)attr; const char *input_val = NULL; @@ -124,7 +124,7 @@ static int attr_enum_filter(void *attr, const char *input, int *err, input_int = j; } - if (!input_int) { + if (input_int < 0) { *err = -EINVAL; pr_err("kunit executor: invalid filter input: %s\n", input); return false; @@ -186,8 +186,10 @@ static void *attr_module_get(void *test_or_suite, bool is_test) // Suites get their module attribute from their first test_case if (test) return ((void *) test->module_name); - else + else if (kunit_suite_num_test_cases(suite) > 0) return ((void *) suite->test_cases[0].module_name); + else + return (void *) ""; } /* List of all Test Attributes */ @@ -221,7 +223,7 @@ const char *kunit_attr_filter_name(struct kunit_attr_filter filter) void kunit_print_attr(void *test_or_suite, bool is_test, unsigned int test_level) { int i; - bool to_free; + bool to_free = false; void *attr; const char *attr_name, *attr_str; struct kunit_suite *suite = is_test ? NULL : test_or_suite; @@ -255,7 +257,7 @@ void kunit_print_attr(void *test_or_suite, bool is_test, unsigned int test_level int kunit_get_filter_count(char *input) { - int i, comma_index, count = 0; + int i, comma_index = 0, count = 0; for (i = 0; input[i]; i++) { if (input[i] == ',') { @@ -272,7 +274,7 @@ int kunit_get_filter_count(char *input) struct kunit_attr_filter kunit_next_attr_filter(char **filters, int *err) { struct kunit_attr_filter filter = {}; - int i, j, comma_index, new_start_index; + int i, j, comma_index = 0, new_start_index = 0; int op_index = -1, attr_index = -1; char op; char *input = *filters; @@ -316,7 +318,7 @@ struct kunit_attr_filter kunit_next_attr_filter(char **filters, int *err) filter.attr = &kunit_attr_list[attr_index]; } - if (comma_index) { + if (comma_index > 0) { input[comma_index] = '\0'; filter.input = input + op_index; input = input + new_start_index; @@ -356,31 +358,22 @@ struct kunit_suite *kunit_filter_attr_tests(const struct kunit_suite *const suit /* Save filtering result on default value */ default_result = filter.attr->filter(filter.attr->attr_default, filter.input, err); - if (*err) { - kfree(copy); - kfree(filtered); - return NULL; - } + if (*err) + goto err; /* Save suite attribute value and filtering result on that value */ suite_val = filter.attr->get_attr((void *)suite, false); suite_result = filter.attr->filter(suite_val, filter.input, err); - if (*err) { - kfree(copy); - kfree(filtered); - return NULL; - } + if (*err) + goto err; /* For each test case, save test case if passes filtering. */ kunit_suite_for_each_test_case(suite, test_case) { test_val = filter.attr->get_attr((void *) test_case, true); test_result = filter.attr->filter(filter.attr->get_attr(test_case, true), filter.input, err); - if (*err) { - kfree(copy); - kfree(filtered); - return NULL; - } + if (*err) + goto err; /* * If attribute value of test case is set, filter on that value. @@ -406,7 +399,8 @@ struct kunit_suite *kunit_filter_attr_tests(const struct kunit_suite *const suit } } - if (n == 0) { +err: + if (n == 0 || *err) { kfree(copy); kfree(filtered); return NULL; diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index 481901d245d0..b6e07de2876a 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -130,7 +130,7 @@ static struct suite_set kunit_filter_suites(const struct suite_set *suite_set, struct kunit_suite **copy, *filtered_suite, *new_filtered_suite; struct suite_set filtered; 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; @@ -147,7 +147,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 +170,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); base-commit: 3bffe185ad11e408903d2782727877388d08d94e -- 2.41.0.585.gd2178a4bd4-goog

1 year, 4 months

2
2
0 0

[PATCH] selftests: cgroup: fix test_kmem_basic false positives

by Johannes Weiner

This test fails routinely in our prod testing environment, and I can reproduce it locally as well. The test allocates dcache inside a cgroup, then drops the memory limit and checks that usage drops correspondingly. The reason it fails is because dentries are freed with an RCU delay - a debugging sleep shows that usage drops as expected shortly after. Insert a 1s sleep after dropping the limit. This should be good enough, assuming that machines running those tests are otherwise not very busy. Signed-off-by: Johannes Weiner <hannes(a)cmpxchg.org> --- tools/testing/selftests/cgroup/test_kmem.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/testing/selftests/cgroup/test_kmem.c b/tools/testing/selftests/cgroup/test_kmem.c index 258ddc565deb..1b2cec9d18a4 100644 --- a/tools/testing/selftests/cgroup/test_kmem.c +++ b/tools/testing/selftests/cgroup/test_kmem.c @@ -70,6 +70,10 @@ static int test_kmem_basic(const char *root) goto cleanup; cg_write(cg, "memory.high", "1M"); + + /* wait for RCU freeing */ + sleep(1); + slab1 = cg_read_key_long(cg, "memory.stat", "slab "); if (slab1 <= 0) goto cleanup; -- 2.41.0

1 year, 4 months

3
3
0 0

[PATCH] selftests/riscv: fix potential build failure during the "emit_tests" step

by John Hubbard

The riscv selftests (which were modeled after the arm64 selftests) are improperly declaring the "emit_tests" target to depend upon the "all" target. This approach, when combined with commit 9fc96c7c19df ("selftests: error out if kernel header files are not yet built"), has caused build failures [1] on arm64, and is likely to cause similar failures for riscv. To fix this, simply remove the unnecessary "all" dependency from the emit_tests target. The dependency is still effectively honored, because again, invocation is via "install", which also depends upon "all". An alternative approach would be to harden the emit_tests target so that it can depend upon "all", but that's a lot more complicated and hard to get right, and doesn't seem worth it, especially given that emit_tests should probably not be overridden at all. [1] https://lore.kernel.org/20230710-kselftest-fix-arm64-v1-1-48e872844f25@kern… Fixes: 9fc96c7c19df ("selftests: error out if kernel header files are not yet built") Signed-off-by: John Hubbard <jhubbard(a)nvidia.com> --- Andrew, With this, and with my arm64 fix [2] that you've already put into mm-unstable, you should be able to safely drop commit 819187ab8741 ("selftests: fix arm64 test installation"). [2] https://lore.kernel.org/20230711005629.2547838-1-jhubbard@nvidia.com thanks, John Hubbard tools/testing/selftests/riscv/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/riscv/Makefile b/tools/testing/selftests/riscv/Makefile index 9dd629cc86aa..f4b3d5c9af5b 100644 --- a/tools/testing/selftests/riscv/Makefile +++ b/tools/testing/selftests/riscv/Makefile @@ -43,7 +43,7 @@ run_tests: all done # Avoid any output on non riscv on emit_tests -emit_tests: all +emit_tests: @for DIR in $(RISCV_SUBTARGETS); do \ BUILD_TARGET=$(OUTPUT)/$$DIR; \ $(MAKE) OUTPUT=$$BUILD_TARGET -C $$DIR $@; \ base-commit: 3f01e9fed8454dcd89727016c3e5b2fbb8f8e50c prerequisite-patch-id: 37c92f7425689ff069fb83996a25cd98e78d7242 -- 2.41.0

1 year, 4 months

5
4
0 0

[PATCH v3 00/17] iommufd: Add nesting infrastructure

by Yi Liu

Nested translation is a hardware feature that is supported by many modern IOMMU hardwares. It has two stages (stage-1, stage-2) address translation to get access to the physical address. stage-1 translation table is owned by userspace (e.g. by a guest OS), while stage-2 is owned by kernel. Changes to stage-1 translation table should be followed by an IOTLB invalidation. Take Intel VT-d as an example, the stage-1 translation table is I/O page table. As the below diagram shows, guest I/O page table pointer in GPA (guest physical address) is passed to host and be used to perform the stage-1 address translation. Along with it, modifications to present mappings in the guest I/O page table should be followed with an IOTLB invalidation. .-------------. .---------------------------. | vIOMMU | | Guest I/O page table | | | '---------------------------' .----------------/ | PASID Entry |--- PASID cache flush --+ '-------------' | | | V | | I/O page table pointer in GPA '-------------' Guest ------| Shadow |---------------------------|-------- v v v Host .-------------. .------------------------. | pIOMMU | | FS for GIOVA->GPA | | | '------------------------' .----------------/ | | PASID Entry | V (Nested xlate) '----------------\.----------------------------------. | | | SS for GPA->HPA, unmanaged domain| | | '----------------------------------' '-------------' Where: - FS = First stage page tables - SS = Second stage page tables <Intel VT-d Nested translation> In IOMMUFD, all the translation tables are tracked by hw_pagetable (hwpt) and each has an iommu_domain allocated from iommu driver. So in this series hw_pagetable and iommu_domain means the same thing if no special note. IOMMUFD has already supported allocating hw_pagetable that is linked with an IOAS. However, nesting requires IOMMUFD to allow allocating hw_pagetable with driver specific parameters and interface to sync stage-1 IOTLB as user owns the stage-1 translation table. This series is based on the iommu hw info reporting series [1]. It first introduces new iommu op for allocating domains with user data and the op for invalidate stage-1 IOTLB, and then extend the IOMMUFD internal infrastructure to accept user_data and parent hwpt, then relay the data to iommu core to allocate user iommu_domain. After it, extends the ioctl IOMMU_HWPT_ALLOC to accept user data and stage-2 hwpt ID to allocate hwpt. Along with it, ioctl IOMMU_HWPT_INVALIDATE is added to invalidate stage-1 IOTLB. This is needed for user-managed hwpts. Selftest is added as well to cover the new ioctls. Complete code can be found in [2], QEMU could can be found in [3]. At last, this is a team work together with Nicolin Chen, Lu Baolu. Thanks them for the help. ^_^. Look forward to your feedbacks. [1] https://lore.kernel.org/linux-iommu/20230724105936.107042-1-yi.l.liu@intel.… [2] https://github.com/yiliu1765/iommufd/tree/iommufd_nesting [3] https://github.com/yiliu1765/qemu/tree/wip/iommufd_rfcv4_nesting Change log: v3: - Add new uAPI things in alphabetical order - Pass in "enum iommu_hwpt_type hwpt_type" to op->domain_alloc_user for sanity, replacing the previous op->domain_alloc_user_data_len solution - Return ERR_PTR from domain_alloc_user instead of NULL - Only add IOMMU_RESV_SW_MSI to kernel-managed HWPT in nested translation (Kevin) - Add IOMMU_RESV_IOVA_RANGES to report resv iova ranges to userspace hence userspace is able to exclude the ranges in the stage-1 HWPT (e.g. guest I/O page table). (Kevin) - Add selftest coverage for the new IOMMU_RESV_IOVA_RANGES ioctl - Minor changes per Kevin's inputs v2: https://lore.kernel.org/linux-iommu/20230511143844.22693-1-yi.l.liu@intel.c… - Add union iommu_domain_user_data to include all user data structures to avoid passing void * in kernel APIs. - Add iommu op to return user data length for user domain allocation - Rename struct iommu_hwpt_alloc::data_type to be hwpt_type - Store the invalidation data length in iommu_domain_ops::cache_invalidate_user_data_len - Convert cache_invalidate_user op to be int instead of void - Remove @data_type in struct iommu_hwpt_invalidate - Remove out_hwpt_type_bitmap in struct iommu_hw_info hence drop patch 08 of v1 v1: https://lore.kernel.org/linux-iommu/20230309080910.607396-1-yi.l.liu@intel.… Thanks, Yi Liu Lu Baolu (2): iommu: Add new iommu op to create domains owned by userspace iommu: Add nested domain support Nicolin Chen (6): iommufd/hw_pagetable: Do not populate user-managed hw_pagetables iommufd: Only enforce IOMMU_RESV_SW_MSI when attaching user-managed HWPT iommufd/selftest: Add domain_alloc_user() support in iommu mock iommufd/selftest: Add coverage for IOMMU_HWPT_ALLOC with user data iommufd/selftest: Add IOMMU_TEST_OP_MD_CHECK_IOTLB test op iommufd/selftest: Add coverage for IOMMU_HWPT_INVALIDATE ioctl Yi Liu (9): iommufd/hw_pagetable: Use domain_alloc_user op for domain allocation iommufd: Pass in hwpt_type/parent/user_data to iommufd_hw_pagetable_alloc() iommufd: Add IOMMU_RESV_IOVA_RANGES iommufd: IOMMU_HWPT_ALLOC allocation with user data iommufd: Add IOMMU_HWPT_INVALIDATE iommufd/selftest: Add a helper to get test device iommufd/selftest: Add IOMMU_TEST_OP_DEV_[ADD|DEL]_RESERVED to add/del reserved regions to selftest device iommufd/selftest: Add .get_resv_regions() for mock_dev iommufd/selftest: Add coverage for IOMMU_RESV_IOVA_RANGES drivers/iommu/iommufd/device.c | 9 +- drivers/iommu/iommufd/hw_pagetable.c | 181 +++++++++++- drivers/iommu/iommufd/io_pagetable.c | 5 +- drivers/iommu/iommufd/iommufd_private.h | 20 +- drivers/iommu/iommufd/iommufd_test.h | 36 +++ drivers/iommu/iommufd/main.c | 59 +++- drivers/iommu/iommufd/selftest.c | 266 ++++++++++++++++-- include/linux/iommu.h | 34 +++ include/uapi/linux/iommufd.h | 96 ++++++- tools/testing/selftests/iommu/iommufd.c | 224 ++++++++++++++- tools/testing/selftests/iommu/iommufd_utils.h | 70 +++++ 11 files changed, 958 insertions(+), 42 deletions(-) -- 2.34.1

1 year, 4 months

5
65
0 0

[PATCH v2] selftests: cgroup: fix test_kmem_basic slab1 check

by Lucas Karpinski

test_kmem_basic creates 100,000 negative dentries, with each one mapping to a slab object. After memory.high is set, these are reclaimed through the shrink_slab function call which reclaims all 100,000 entries. The test passes the majority of the time because when slab1 is calculated, it is often above 0, however, 0 is also an acceptable value. Signed-off-by: Lucas Karpinski <lkarpins(a)redhat.com> --- https://lore.kernel.org/all/m6jbt5hzq27ygt3l4xyiaxxb7i5auvb2lahbcj4yaxxigqz… V2: Corrected title tools/testing/selftests/cgroup/test_kmem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/cgroup/test_kmem.c b/tools/testing/selftests/cgroup/test_kmem.c index 258ddc565deb..ba0a0bfc5a98 100644 --- a/tools/testing/selftests/cgroup/test_kmem.c +++ b/tools/testing/selftests/cgroup/test_kmem.c @@ -71,7 +71,7 @@ static int test_kmem_basic(const char *root) cg_write(cg, "memory.high", "1M"); slab1 = cg_read_key_long(cg, "memory.stat", "slab "); - if (slab1 <= 0) + if (slab1 < 0) goto cleanup; current = cg_read_long(cg, "memory.current"); -- 2.41.0

1 year, 4 months

2
1
0 0

[PATCH] selftests: riscv: Fix compilation error with vstate_exec_nolibc.c

by Alexandre Ghiti

The following error happens: In file included from vstate_exec_nolibc.c:2: /usr/include/riscv64-linux-gnu/sys/prctl.h:42:12: error: conflicting types for ‘prctl’; h ave ‘int(int, ...)’ 42 | extern int prctl (int __option, ...) __THROW; | ^~~~~ In file included from ./../../../../include/nolibc/nolibc.h:99, from <command-line>: ./../../../../include/nolibc/sys.h:892:5: note: previous definition of ‘prctl’ with type ‘int(int, long unsigned int, long unsigned int, long unsigned int, long unsigned int) ’ 892 | int prctl(int option, unsigned long arg2, unsigned long arg3, | ^~~~~ Fix this by not including <sys/prctl.h>, which is not needed here since prctl syscall is directly called using its number. Fixes: 7cf6198ce22d ("selftests: Test RISC-V Vector prctl interface") Signed-off-by: Alexandre Ghiti <alexghiti(a)rivosinc.com> --- tools/testing/selftests/riscv/vector/vstate_exec_nolibc.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/tools/testing/selftests/riscv/vector/vstate_exec_nolibc.c b/tools/testing/selftests/riscv/vector/vstate_exec_nolibc.c index 5cbc392944a6..2c0d2b1126c1 100644 --- a/tools/testing/selftests/riscv/vector/vstate_exec_nolibc.c +++ b/tools/testing/selftests/riscv/vector/vstate_exec_nolibc.c @@ -1,6 +1,4 @@ // SPDX-License-Identifier: GPL-2.0-only -#include <sys/prctl.h> - #define THIS_PROGRAM "./vstate_exec_nolibc" int main(int argc, char **argv) -- 2.39.2

1 year, 4 months

3
2
0 0

[PATCH v2 1/2] test_firmware: prevent race conditions by a correct implementation of locking

by Mirsad Todorovac

[ Upstream commit 4acfe3dfde685a5a9eaec5555351918e2d7266a1 ] Dan Carpenter spotted a race condition in a couple of situations like these in the test_firmware driver: static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg) { u8 val; int ret; ret = kstrtou8(buf, 10, &val); if (ret) return ret; mutex_lock(&test_fw_mutex); *(u8 *)cfg = val; mutex_unlock(&test_fw_mutex); /* Always return full write size even if we didn't consume all */ return size; } static ssize_t config_num_requests_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { int rc; mutex_lock(&test_fw_mutex); if (test_fw_config->reqs) { pr_err("Must call release_all_firmware prior to changing config\n"); rc = -EINVAL; mutex_unlock(&test_fw_mutex); goto out; } mutex_unlock(&test_fw_mutex); // NOTE: HERE is the race!!! Function can be preempted! // test_fw_config->reqs can change between the release of // the lock about and acquire of the lock in the // test_dev_config_update_u8() rc = test_dev_config_update_u8(buf, count, &test_fw_config->num_requests); out: return rc; } static ssize_t config_read_fw_idx_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { return test_dev_config_update_u8(buf, count, &test_fw_config->read_fw_idx); } The function test_dev_config_update_u8() is called from both the locked and the unlocked context, function config_num_requests_store() and config_read_fw_idx_store() which can both be called asynchronously as they are driver's methods, while test_dev_config_update_u8() and siblings change their argument pointed to by u8 *cfg or similar pointer. To avoid deadlock on test_fw_mutex, the lock is dropped before calling test_dev_config_update_u8() and re-acquired within test_dev_config_update_u8() itself, but alas this creates a race condition. Having two locks wouldn't assure a race-proof mutual exclusion. This situation is best avoided by the introduction of a new, unlocked function __test_dev_config_update_u8() which can be called from the locked context and reducing test_dev_config_update_u8() to: static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg) { int ret; mutex_lock(&test_fw_mutex); ret = __test_dev_config_update_u8(buf, size, cfg); mutex_unlock(&test_fw_mutex); return ret; } doing the locking and calling the unlocked primitive, which enables both locked and unlocked versions without duplication of code. Fixes: c92316bf8e948 ("test_firmware: add batched firmware tests") Cc: Luis R. Rodriguez <mcgrof(a)kernel.org> Cc: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> Cc: Russ Weight <russell.h.weight(a)intel.com> Cc: Takashi Iwai <tiwai(a)suse.de> Cc: Tianfei Zhang <tianfei.zhang(a)intel.com> Cc: Shuah Khan <shuah(a)kernel.org> Cc: Colin Ian King <colin.i.king(a)gmail.com> Cc: Randy Dunlap <rdunlap(a)infradead.org> Cc: linux-kselftest(a)vger.kernel.org Cc: stable(a)vger.kernel.org # v5.4, 4.19 Suggested-by: Dan Carpenter <error27(a)gmail.com> Link: https://lore.kernel.org/r/20230509084746.48259-1-mirsad.todorovac@alu.unizg… Signed-off-by: Mirsad Todorovac <mirsad.todorovac(a)alu.unizg.hr> [ This is the patch to fix the racing condition in locking for the 5.4, ] [ 4.19 and 4.4 stable branches. Not all the fixes from the upstream ] [ commit apply, but those which do are verbatim equal to those in the ] [ upstream commit. ] --- v2: bundled locking and ENOSPC patches together. tested on 5.4 and 4.19 stable. lib/test_firmware.c | 37 ++++++++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/lib/test_firmware.c b/lib/test_firmware.c index 38553944e967..92d7195d5b5b 100644 --- a/lib/test_firmware.c +++ b/lib/test_firmware.c @@ -301,16 +301,26 @@ static ssize_t config_test_show_str(char *dst, return len; } -static int test_dev_config_update_bool(const char *buf, size_t size, - bool *cfg) +static inline int __test_dev_config_update_bool(const char *buf, size_t size, + bool *cfg) { int ret; - mutex_lock(&test_fw_mutex); if (strtobool(buf, cfg) < 0) ret = -EINVAL; else ret = size; + + return ret; +} + +static int test_dev_config_update_bool(const char *buf, size_t size, + bool *cfg) +{ + int ret; + + mutex_lock(&test_fw_mutex); + ret = __test_dev_config_update_bool(buf, size, cfg); mutex_unlock(&test_fw_mutex); return ret; @@ -340,7 +350,7 @@ static ssize_t test_dev_config_show_int(char *buf, int cfg) return snprintf(buf, PAGE_SIZE, "%d\n", val); } -static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg) +static inline int __test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg) { int ret; long new; @@ -352,14 +362,23 @@ static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg) if (new > U8_MAX) return -EINVAL; - mutex_lock(&test_fw_mutex); *(u8 *)cfg = new; - mutex_unlock(&test_fw_mutex); /* Always return full write size even if we didn't consume all */ return size; } +static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg) +{ + int ret; + + mutex_lock(&test_fw_mutex); + ret = __test_dev_config_update_u8(buf, size, cfg); + mutex_unlock(&test_fw_mutex); + + return ret; +} + static ssize_t test_dev_config_show_u8(char *buf, u8 cfg) { u8 val; @@ -392,10 +411,10 @@ static ssize_t config_num_requests_store(struct device *dev, mutex_unlock(&test_fw_mutex); goto out; } - mutex_unlock(&test_fw_mutex); - rc = test_dev_config_update_u8(buf, count, - &test_fw_config->num_requests); + rc = __test_dev_config_update_u8(buf, count, + &test_fw_config->num_requests); + mutex_unlock(&test_fw_mutex); out: return rc; -- 2.39.3

1 year, 4 months

1
0
0 0

[RFC PATCH 0/3] memfd: cleanups for vm.memfd_noexec

by Aleksa Sarai

It seems that the most critical issue with vm.memfd_noexec=2 (the fact that passing MFD_EXEC would bypass it entirely[1]) has been fixed in Andrew's tree[2], but there are still some outstanding issues that need to be addressed: * The dmesg warnings are pr_warn_once, which on most systems means that they will be used up by systemd or some other boot process and userspace developers will never see it. The original patch posted to the ML used pr_warn_ratelimited but the merged patch had it changed (with a comment about it being "per review"), but given that the current warnings are useless, pr_warn_ratelimited makes far more sense. * vm.memfd_noexec=2 shouldn't reject old-style memfd_create(2) syscalls because it will make it far to difficult to ever migrate. Instead it should imply MFD_EXEC. * The racheting mechanism for vm.memfd_noexec doesn't make sense as a security mechanism because a CAP_SYS_ADMIN capable user can create executable binaries in a hidden tmpfs very easily, not to mention the many other things they can do. * The memfd selftests would not exit with a non-zero error code when certain tests that ran in a forked process (specifically the ones related to MFD_EXEC and MFD_NOEXEC_SEAL) failed. (This patchset is based on top of Jeff Xu's patches[2] fixing the MFD_EXEC bug in vm.memfd_noexec=2.) [1]: https://lore.kernel.org/all/ZJwcsU0vI-nzgOB_@codewreck.org/ [2]: https://lore.kernel.org/all/20230705063315.3680666-1-jeffxu@google.com/ Aleksa Sarai (3): memfd: cleanups for vm.memfd_noexec handling memfd: remove racheting feature from vm.memfd_noexec selftests: memfd: error out test process when child test fails include/linux/pid_namespace.h | 16 +++------ kernel/pid_sysctl.h | 7 ---- mm/memfd.c | 32 +++++++---------- tools/testing/selftests/memfd/memfd_test.c | 41 ++++++++++++++++++---- 4 files changed, 51 insertions(+), 45 deletions(-) -- 2.41.0

1 year, 4 months

3
17
0 0

2024

2023

2022

2021

2020

2019

2018

2017

Linux-kselftest-mirror August 2023