| Message ID | 20230922113923.3621959-1-make_ruc2021@163.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] list: test: potential dereference of null pointer | expand |
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 --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);
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(+)