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

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

•           KUNIT_FAIL(test, "Initialising list5 failed.\n");
  

•           return;
  

•   }
  

Daniel