| Message ID | 20230914114629.1517650-4-ruanjinjie@huawei.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | kunit: Fix some bugs in kunit | expand |
On Thu, Sep 14, 2023 at 7:47 AM 'Jinjie Ruan' via KUnit Development <kunit-dev@googlegroups.com> wrote: > > If the outer layer for loop is iterated more than once and it fails not > in the first iteration, the filtered_suite and filtered_suite->test_cases > allocated in the last kunit_filter_attr_tests() in last inner for loop > is leaked. > > So add a new free_filtered_suite err label and free the filtered_suite > and filtered_suite->test_cases so far. And change kmalloc_array of copy > to kcalloc to Clear the copy to make the kfree safe. > > Fixes: 5d31f71efcb6 ("kunit: add kunit.filter_glob cmdline option to filter suites") > Fixes: 529534e8cba3 ("kunit: Add ability to filter attributes") > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> Hello! This looks good to me. I just have one comment below. Reviewed-by: Rae Moar <rmoar@google.com> Thanks! -Rae > --- > lib/kunit/executor.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > > diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c > index 9358ed2df839..1236b3cd2fbb 100644 > --- a/lib/kunit/executor.c > +++ b/lib/kunit/executor.c > @@ -157,10 +157,11 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set, > struct kunit_suite_set filtered = {NULL, NULL}; > struct kunit_glob_filter parsed_glob; > struct kunit_attr_filter *parsed_filters = NULL; > + struct kunit_suite * const *suites; > > const size_t max = suite_set->end - suite_set->start; > > - copy = kmalloc_array(max, sizeof(*filtered.start), GFP_KERNEL); > + copy = kcalloc(max, sizeof(*filtered.start), GFP_KERNEL); > if (!copy) { /* won't be able to run anything, return an empty set */ > return filtered; > } > @@ -195,7 +196,7 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set, > parsed_glob.test_glob); > if (IS_ERR(filtered_suite)) { > *err = PTR_ERR(filtered_suite); > - goto free_parsed_filters; > + goto free_filtered_suite; > } > } > if (filter_count > 0 && parsed_filters != NULL) { > @@ -212,11 +213,11 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set, > filtered_suite = new_filtered_suite; > > if (*err) > - goto free_parsed_filters; > + goto free_filtered_suite; > > if (IS_ERR(filtered_suite)) { > *err = PTR_ERR(filtered_suite); > - goto free_parsed_filters; > + goto free_filtered_suite; > } > if (!filtered_suite) > break; > @@ -231,6 +232,14 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set, > filtered.start = copy_start; > filtered.end = copy; > > +free_filtered_suite: > + if (*err) { > + for (suites = copy_start; suites < copy; suites++) { > + kfree((*suites)->test_cases); > + kfree(*suites); > + } > + } > + As this is pretty similar code to kunit_free_suite_set, I wish you could use that method instead but I'm not actually sure it would be cleaner. > free_parsed_filters: > if (filter_count) > kfree(parsed_filters); > -- > 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@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20230914114629.1517650-4-ruanjinjie%40huawei.com.
diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index 9358ed2df839..1236b3cd2fbb 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -157,10 +157,11 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set, struct kunit_suite_set filtered = {NULL, NULL}; struct kunit_glob_filter parsed_glob; struct kunit_attr_filter *parsed_filters = NULL; + struct kunit_suite * const *suites; const size_t max = suite_set->end - suite_set->start; - copy = kmalloc_array(max, sizeof(*filtered.start), GFP_KERNEL); + copy = kcalloc(max, sizeof(*filtered.start), GFP_KERNEL); if (!copy) { /* won't be able to run anything, return an empty set */ return filtered; } @@ -195,7 +196,7 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set, parsed_glob.test_glob); if (IS_ERR(filtered_suite)) { *err = PTR_ERR(filtered_suite); - goto free_parsed_filters; + goto free_filtered_suite; } } if (filter_count > 0 && parsed_filters != NULL) { @@ -212,11 +213,11 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set, filtered_suite = new_filtered_suite; if (*err) - goto free_parsed_filters; + goto free_filtered_suite; if (IS_ERR(filtered_suite)) { *err = PTR_ERR(filtered_suite); - goto free_parsed_filters; + goto free_filtered_suite; } if (!filtered_suite) break; @@ -231,6 +232,14 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set, filtered.start = copy_start; filtered.end = copy; +free_filtered_suite: + if (*err) { + for (suites = copy_start; suites < copy; suites++) { + kfree((*suites)->test_cases); + kfree(*suites); + } + } + free_parsed_filters: if (filter_count) kfree(parsed_filters);
If the outer layer for loop is iterated more than once and it fails not in the first iteration, the filtered_suite and filtered_suite->test_cases allocated in the last kunit_filter_attr_tests() in last inner for loop is leaked. So add a new free_filtered_suite err label and free the filtered_suite and filtered_suite->test_cases so far. And change kmalloc_array of copy to kcalloc to Clear the copy to make the kfree safe. Fixes: 5d31f71efcb6 ("kunit: add kunit.filter_glob cmdline option to filter suites") Fixes: 529534e8cba3 ("kunit: Add ability to filter attributes") Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> --- lib/kunit/executor.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)