Re: [PATCH] kunit: Check for kunit_parse_glob_filter() failure

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

 



On Fri, Sep 15, 2023 at 8:58 AM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
>
> Smatch complains that the missing error checks would lead to a crash:
>
>     lib/kunit/executor_test.c:40 parse_filter_test()
>     error: double free of 'filter.test_glob'
>
> We may as well do it right...
>
> Fixes: a127b154a8f2 ("kunit: tool: allow filtering test cases via glob")
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>

Hello!

We definitely should add in checks for these errors. I have a couple
comments below.

Thanks!
-Rae

> ---
>  lib/kunit/executor_test.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/lib/kunit/executor_test.c b/lib/kunit/executor_test.c
> index b4f6f96b2844..176c9c9dfcfc 100644
> --- a/lib/kunit/executor_test.c
> +++ b/lib/kunit/executor_test.c
> @@ -27,13 +27,15 @@ static void parse_filter_test(struct kunit *test)
>  {
>         struct kunit_glob_filter filter = {NULL, NULL};
>
> -       kunit_parse_glob_filter(&filter, "suite");
> +       if (!kunit_parse_glob_filter(&filter, "suite"))
> +               return;

First, this is returning every time this test is run because the
kunit_parse_glob_filter returns 0 when there is no error. So this
should instead be checking for a result of not equal to 0.

Secondly, this should fail the test if the parsing returns an error.
So instead of returning I would recommend using a KUNIT_ASSERT
statement to check the result of this method is equal to 0.

>         KUNIT_EXPECT_STREQ(test, filter.suite_glob, "suite");
>         KUNIT_EXPECT_FALSE(test, filter.test_glob);
>         kfree(filter.suite_glob);
>         kfree(filter.test_glob);
>
> -       kunit_parse_glob_filter(&filter, "suite.test");
> +       if (!kunit_parse_glob_filter(&filter, "suite.test"))
> +               return;

Similar to above I think this should be changed to a KUNIT_ASSERT
statement to ensure the result is equal to 0.

>         KUNIT_EXPECT_STREQ(test, filter.suite_glob, "suite");
>         KUNIT_EXPECT_STREQ(test, filter.test_glob, "test");
>         kfree(filter.suite_glob);
> --
> 2.39.2
>
> --
> 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/3ff9d019-75b6-45ee-9b03-9d6ec7c5364f%40moroto.mountain.




[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