2024-05-24 19:15:19

by Kuan-Wei Chiu

Subject: [PATCH] tools/lib/slab: Fix potential NULL pointer dereference in kmalloc()

In kmalloc(), add a check to ensure that the pointer 'ret' is not NULL
before attempting to memset it when the __GFP_ZERO flag is set. This
prevents a potential NULL pointer dereference.

Signed-off-by: Kuan-Wei Chiu <[email protected]>
---
tools/lib/slab.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/slab.c b/tools/lib/slab.c
index 959997fb0652..aeaf535b422a 100644
--- a/tools/lib/slab.c
+++ b/tools/lib/slab.c
@@ -22,7 +22,7 @@ void *kmalloc(size_t size, gfp_t gfp)
uatomic_inc(&kmalloc_nr_allocated);
if (kmalloc_verbose)
printf("Allocating %p from malloc\n", ret);
- if (gfp & __GFP_ZERO)
+ if (gfp & __GFP_ZERO && ret)
memset(ret, 0, size);
return ret;
}
--
2.34.1

2024-05-24 19:24:01

by Andrew Morton

Subject: Re: [PATCH] tools/lib/slab: Fix potential NULL pointer dereference in kmalloc()

On Sat, 25 May 2024 03:14:59 +0800 Kuan-Wei Chiu <[email protected]> wrote:

> In kmalloc(), add a check to ensure that the pointer 'ret' is not NULL
> before attempting to memset it when the __GFP_ZERO flag is set. This
> prevents a potential NULL pointer dereference.
>
> ...
>
> --- a/tools/lib/slab.c
> +++ b/tools/lib/slab.c
> @@ -22,7 +22,7 @@ void *kmalloc(size_t size, gfp_t gfp)
> uatomic_inc(&kmalloc_nr_allocated);
> if (kmalloc_verbose)
> printf("Allocating %p from malloc\n", ret);
> - if (gfp & __GFP_ZERO)
> + if (gfp & __GFP_ZERO && ret)
> memset(ret, 0, size);
> return ret;
> }

I suspect we have a lot of unchecked mallocs in our userspace code. If
there's an argument for fixing them all(?) then it would be best to do
this in a wholesale fashion rather than patch-at-a-time piecemeal.

2024-05-24 19:47:15

by Kuan-Wei Chiu

Subject: Re: [PATCH] tools/lib/slab: Fix potential NULL pointer dereference in kmalloc()

On Fri, May 24, 2024 at 12:23:50PM -0700, Andrew Morton wrote:
> On Sat, 25 May 2024 03:14:59 +0800 Kuan-Wei Chiu <[email protected]> wrote:
>
> > In kmalloc(), add a check to ensure that the pointer 'ret' is not NULL
> > before attempting to memset it when the __GFP_ZERO flag is set. This
> > prevents a potential NULL pointer dereference.
> >
> > ...
> >
> > --- a/tools/lib/slab.c
> > +++ b/tools/lib/slab.c
> > @@ -22,7 +22,7 @@ void *kmalloc(size_t size, gfp_t gfp)
> > uatomic_inc(&kmalloc_nr_allocated);
> > if (kmalloc_verbose)
> > printf("Allocating %p from malloc\n", ret);
> > - if (gfp & __GFP_ZERO)
> > + if (gfp & __GFP_ZERO && ret)
> > memset(ret, 0, size);
> > return ret;
> > }
>
> I suspect we have a lot of unchecked mallocs in our userspace code. If
> there's an argument for fixing them all(?) then it would be best to do
> this in a wholesale fashion rather than patch-at-a-time piecemeal.
>
It seems likely that I'm not the first to notice the unchecked mallocs
in our userspace code if they're indeed widespread. Are there specific
reasons or guidelines indicating when malloc checks are necessary, and
when they can be omitted?

Regards,
Kuan-Wei