Re: [PATCH 3/4] kunit: Fix possible memory leak in kunit_filter_suites()

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

 



On Thu, Aug 31, 2023 at 3:17 AM 'Jinjie Ruan' via KUnit Development
<kunit-dev@xxxxxxxxxxxxxxxx> wrote:
>
> 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.
>
> Fixes: 1c9fd080dffe ("kunit: fix uninitialized variables bug in attributes filtering")
> Signed-off-by: Jinjie Ruan <ruanjinjie@xxxxxxxxxx>

Hello!

Thanks for catching this! I just had two concerns listed below.

-Rae

> ---
>  lib/kunit/executor.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
> index 25ce8d6e5fe7..7654c09c1ab1 100644
> --- a/lib/kunit/executor.c
> +++ b/lib/kunit/executor.c
> @@ -176,10 +176,8 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set,
>         if (filters) {
>                 filter_count = kunit_get_filter_count(filters);
>                 parsed_filters = kcalloc(filter_count, sizeof(*parsed_filters), GFP_KERNEL);
> -               if (!parsed_filters) {
> -                       kfree(copy);
> -                       return filtered;
> -               }
> +               if (!parsed_filters)
> +                       goto err;

When this goes to the "err" label this will likely enter the if
(filter_count) statement and attempt to free parsed_filters. Since
parsed_filters is NULL in this case, it could be a cleaner practice to
set a new label, maybe "filter_err", to skip over that if statement.
Although, I realize this may require the order of the patches to
switch.

Additionally, the value of *err should definitely be set here before
"goto err". The kunit_run_all_tests() function checks this err value
in order to produce the correct error message and to prevent any
attempt to run tests. I would suggest: *err = -ENOMEM;

>                 for (j = 0; j < filter_count; j++)
>                         parsed_filters[j] = kunit_next_attr_filter(&filters, err);
>                 if (*err)
> --
> 2.34.1
>
> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@xxxxxxxxxxxxxxxx.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20230831071655.2907683-4-ruanjinjie%40huawei.com.




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

  Powered by Linux