diff mbox series

[v2] list: test: potential dereference of null pointer

Message ID 20230922113923.3621959-1-make_ruc2021@163.com
State New
Headers show
Series [v2] list: test: potential dereference of null pointer | expand

Commit Message

Ma Ke Sept. 22, 2023, 11:39 a.m. UTC
To avoid the failure of alloc, we could check the return value of
kmalloc() and kzalloc().

Signed-off-by: Ma Ke <make_ruc2021@163.com>
---
 lib/list-test.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Daniel Latypov Sept. 22, 2023, 11:39 p.m. UTC | #1
On Fri, Sep 22, 2023 at 4:40 AM Ma Ke <make_ruc2021@163.com> wrote:
>
> To avoid the failure of alloc, we could check the return value of
> kmalloc() and kzalloc().

Thanks, that's a good point, some suggestions below.
And also a question for David Gow whenever he sees this.

>
> Signed-off-by: Ma Ke <make_ruc2021@163.com>
> ---
>  lib/list-test.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/lib/list-test.c b/lib/list-test.c
> index 0cc27de9cec8..70e898976dbf 100644
> --- a/lib/list-test.c
> +++ b/lib/list-test.c
> @@ -27,9 +27,18 @@ static void list_test_list_init(struct kunit *test)
>         INIT_LIST_HEAD(&list2);
>
>         list4 = kzalloc(sizeof(*list4), GFP_KERNEL | __GFP_NOFAIL);
> +       if (!list4) {
> +               KUNIT_FAIL(test, "Initialising list4 failed.\n");
> +               return;
> +       }

Note: we can replace this with a one-liner
  KUNIT_ASSERT_NOT_NULL(test, list4);

>         INIT_LIST_HEAD(list4);
>
>         list5 = kmalloc(sizeof(*list5), GFP_KERNEL | __GFP_NOFAIL);
> +       if (!list5) {
> +               kfree(list4);

We can also replace this check with the one-liner
  KUNIT_ASSERT_NOT_NULL(test, list5);

But we'd need to migrate the kzalloc() call to use kunit_kzalloc():
  kunit_kzalloc(test, sizeof(*list4), GFP_KERNEL | __GFP_NOFAIL);
that way we don't have to manually free list4.

I'm not sure why the original version didn't use the kunit helpers to
begin with.
Perhaps David would remember.

A quick lazy ^F over the original patch didn't find anything afaict,
https://lore.kernel.org/linux-kselftest/a127aeaa-e5ba-2d8d-0894-936e05637508@kernel.org/T/#u


> +               KUNIT_FAIL(test, "Initialising list5 failed.\n");
> +               return;
> +       }

Daniel
diff mbox series

Patch

diff --git a/lib/list-test.c b/lib/list-test.c
index 0cc27de9cec8..70e898976dbf 100644
--- a/lib/list-test.c
+++ b/lib/list-test.c
@@ -27,9 +27,18 @@  static void list_test_list_init(struct kunit *test)
 	INIT_LIST_HEAD(&list2);
 
 	list4 = kzalloc(sizeof(*list4), GFP_KERNEL | __GFP_NOFAIL);
+	if (!list4) {
+		KUNIT_FAIL(test, "Initialising list4 failed.\n");
+		return;
+	}
 	INIT_LIST_HEAD(list4);
 
 	list5 = kmalloc(sizeof(*list5), GFP_KERNEL | __GFP_NOFAIL);
+	if (!list5) {
+		kfree(list4);
+		KUNIT_FAIL(test, "Initialising list5 failed.\n");
+		return;
+	}
 	memset(list5, 0xFF, sizeof(*list5));
 	INIT_LIST_HEAD(list5);