All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH -next] kunit: fix uninitialized variables bug in attributes filtering
@ 2023-08-02 21:28 Rae Moar
  2023-08-03  8:15 ` David Gow
  0 siblings, 1 reply; 3+ messages in thread
From: Rae Moar @ 2023-08-02 21:28 UTC (permalink / raw)
  To: shuah, davidgow, brendan.higgins
  Cc: ruanjinjie, linux-kselftest, kunit-dev, linux-kernel, Rae Moar,
	kernel test robot, Dan Carpenter

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@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/r/202307270610.s0w4NKEn-lkp@intel.com/

Signed-off-by: Rae Moar <rmoar@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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH -next] kunit: fix uninitialized variables bug in attributes filtering
  2023-08-02 21:28 [PATCH -next] kunit: fix uninitialized variables bug in attributes filtering Rae Moar
@ 2023-08-03  8:15 ` David Gow
  2023-08-03 19:39   ` Rae Moar
  0 siblings, 1 reply; 3+ messages in thread
From: David Gow @ 2023-08-03  8:15 UTC (permalink / raw)
  To: Rae Moar
  Cc: shuah, brendan.higgins, ruanjinjie, linux-kselftest, kunit-dev,
	linux-kernel, kernel test robot, Dan Carpenter

[-- Attachment #1: Type: text/plain, Size: 8446 bytes --]

On Thu, 3 Aug 2023 at 05:28, Rae Moar <rmoar@google.com> wrote:
>
> 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@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/r/202307270610.s0w4NKEn-lkp@intel.com/
>
> Signed-off-by: Rae Moar <rmoar@google.com>
> ---

These fixes look good, especially the ones in attributes.c.

There's still a possibility of returning uninitialised or freed
pointers in executor.c. If we can keep 'filtered' valid at all times,
this should be easier to deal with, e.g.:

- Initialise 'filtered' to {NULL, NULL}, which is a valid "empty" value.
- Only ever set start and end at the same time, so don't set 'start'
immediately after allocation.
- Wait until the filtering is complete and successful (i.e., where
'end' is set now), and set 'start' there as well.
- Then return filtered will definitely either return the completely
filtered value, or a valid empty suite_set.

Otherwise, this looks good.

-- David

>
> 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;

Is 'filtered' properly initialised here?
filtered.start is already set to 'copy' by this point (so, having
freed 'copy', this would now be an invalid pointer).
filtered.end is uninitialised.

Can we instead initialise filtered to {NULL, NULL} at the start, and
only set start and end after the filtering has succeeded?

> +               }
>                 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
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4003 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH -next] kunit: fix uninitialized variables bug in attributes filtering
  2023-08-03  8:15 ` David Gow
@ 2023-08-03 19:39   ` Rae Moar
  0 siblings, 0 replies; 3+ messages in thread
From: Rae Moar @ 2023-08-03 19:39 UTC (permalink / raw)
  To: David Gow
  Cc: shuah, brendan.higgins, ruanjinjie, linux-kselftest, kunit-dev,
	linux-kernel, kernel test robot, Dan Carpenter

On Thu, Aug 3, 2023 at 4:15 AM David Gow <davidgow@google.com> wrote:
>
> On Thu, 3 Aug 2023 at 05:28, Rae Moar <rmoar@google.com> wrote:
> >
> > 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@intel.com>
> > Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> > Closes: https://lore.kernel.org/r/202307270610.s0w4NKEn-lkp@intel.com/
> >
> > Signed-off-by: Rae Moar <rmoar@google.com>
> > ---
>
> These fixes look good, especially the ones in attributes.c.
>
> There's still a possibility of returning uninitialised or freed
> pointers in executor.c. If we can keep 'filtered' valid at all times,
> this should be easier to deal with, e.g.:
>
> - Initialise 'filtered' to {NULL, NULL}, which is a valid "empty" value.
> - Only ever set start and end at the same time, so don't set 'start'
> immediately after allocation.
> - Wait until the filtering is complete and successful (i.e., where
> 'end' is set now), and set 'start' there as well.
> - Then return filtered will definitely either return the completely
> filtered value, or a valid empty suite_set.
>

Hi!

Great point. I will definitely change this. This is definitely a flaw
in the code. I have sent out a v2 of this patch with your suggested
changes above. Let me know what you think.

Thanks!
-Rae

> Otherwise, this looks good.
>
> -- David
>
> >
> > 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;
>
> Is 'filtered' properly initialised here?
> filtered.start is already set to 'copy' by this point (so, having
> freed 'copy', this would now be an invalid pointer).
> filtered.end is uninitialised.
>
> Can we instead initialise filtered to {NULL, NULL} at the start, and
> only set start and end after the filtering has succeeded?
>
> > +               }
> >                 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
> >

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-08-03 19:40 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-02 21:28 [PATCH -next] kunit: fix uninitialized variables bug in attributes filtering Rae Moar
2023-08-03  8:15 ` David Gow
2023-08-03 19:39   ` Rae Moar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.