The return value of kzalloc_flex() is used without
ensuring that the allocation succeeded, and the
pointer is dereferenced unconditionally.

Guard the access to the allocated structure to
avoid a potential NULL pointer dereference if the
allocation fails.

Fixes: 980cd426a257 ("staging: rtl8723bs: replace rtw_zmalloc() with kzalloc()")
Signed-off-by: Shyam Sunder Reddy Padira <shyamsunderreddypadira@gmail.com>
---
changes in v3:
-Shortened commit hash in Fixes tag to 12 characters
-Removed blank line between Signed-off-by and Fixes tag

changes in v2:
-Fixed spelling mistakes(dereference, potential)
-Added Fixes tag
---
 drivers/staging/rtl8723bs/os_dep/osdep_service.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/osdep_service.c b/drivers/staging/rtl8723bs/os_dep/osdep_service.c
index 7959daeabc6f..4cfdf7c62344 100644
--- a/drivers/staging/rtl8723bs/os_dep/osdep_service.c
+++ b/drivers/staging/rtl8723bs/os_dep/osdep_service.c
@@ -194,7 +194,8 @@ struct rtw_cbuf *rtw_cbuf_alloc(u32 size)
 	struct rtw_cbuf *cbuf;
 
 	cbuf = kzalloc_flex(*cbuf, bufs, size);
-	cbuf->size = size;
+	if (cbuf)
+		cbuf->size = size;
 
 	return cbuf;
 }
-- 
2.43.0

On Tue, Apr 14, 2026 at 12:43:06PM +0530, Shyam Sunder Reddy Padira wrote:
> The return value of kzalloc_flex() is used without
> ensuring that the allocation succeeded, and the
> pointer is dereferenced unconditionally.
> 
> Guard the access to the allocated structure to
> avoid a potential NULL pointer dereference if the
> allocation fails.

You have a procedural issue here: please avoid sending a new patch version in
the same email thread. It makes things harder to follow. For example, I usually
mark the entire thread as read if I see some comments and don't want to go into
the details. It effectively means that I will never see the new version that
already was in the same thread!

...

> --- a/drivers/staging/rtl8723bs/os_dep/osdep_service.c
> +++ b/drivers/staging/rtl8723bs/os_dep/osdep_service.c
> @@ -194,7 +194,8 @@ struct rtw_cbuf *rtw_cbuf_alloc(u32 size)
>  	struct rtw_cbuf *cbuf;
>  
>  	cbuf = kzalloc_flex(*cbuf, bufs, size);
> -	cbuf->size = size;
> +	if (cbuf)
> +		cbuf->size = size;
>  
>  	return cbuf;

Now to the code. This is still buggy. The problem is that the size is not
validated and when it's 0, the same issue (dereference of invalid pointer)
will happen.

Dan, JFYI k*alloc*(0) returns not NULL and not valid pointer.

-- 
With Best Regards,
Andy Shevchenko

On Tue, Apr 14, 2026 at 10:46:13AM +0300, Andy Shevchenko wrote:
> On Tue, Apr 14, 2026 at 12:43:06PM +0530, Shyam Sunder Reddy Padira wrote:
> > The return value of kzalloc_flex() is used without
> > ensuring that the allocation succeeded, and the
> > pointer is dereferenced unconditionally.
> > 
> > Guard the access to the allocated structure to
> > avoid a potential NULL pointer dereference if the
> > allocation fails.
> 
> You have a procedural issue here: please avoid sending a new patch version in
> the same email thread. It makes things harder to follow. For example, I usually
> mark the entire thread as read if I see some comments and don't want to go into
> the details. It effectively means that I will never see the new version that
> already was in the same thread!
> 
> ...
> 
> > --- a/drivers/staging/rtl8723bs/os_dep/osdep_service.c
> > +++ b/drivers/staging/rtl8723bs/os_dep/osdep_service.c
> > @@ -194,7 +194,8 @@ struct rtw_cbuf *rtw_cbuf_alloc(u32 size)
> >  	struct rtw_cbuf *cbuf;
> >  
> >  	cbuf = kzalloc_flex(*cbuf, bufs, size);
> > -	cbuf->size = size;
> > +	if (cbuf)
> > +		cbuf->size = size;
> >  
> >  	return cbuf;
> 
> Now to the code. This is still buggy. The problem is that the size is not
> validated and when it's 0, the same issue (dereference of invalid pointer)
> will happen.
> 
> Dan, JFYI k*alloc*(0) returns not NULL and not valid pointer.

kzalloc_flex() basically can't return the ZERO_SIZE pointer.  You
would need to pass an empty struct, which is really rare.  It can't
that here for sure.

regards,
dan carpenter

On Tue, Apr 14, 2026 at 10:55:56AM +0300, Dan Carpenter wrote:
> On Tue, Apr 14, 2026 at 10:46:13AM +0300, Andy Shevchenko wrote:
> > On Tue, Apr 14, 2026 at 12:43:06PM +0530, Shyam Sunder Reddy Padira wrote:
> > > The return value of kzalloc_flex() is used without
> > > ensuring that the allocation succeeded, and the
> > > pointer is dereferenced unconditionally.
> > > 
> > > Guard the access to the allocated structure to
> > > avoid a potential NULL pointer dereference if the
> > > allocation fails.
> > 
> > You have a procedural issue here: please avoid sending a new patch version in
> > the same email thread. It makes things harder to follow. For example, I usually
> > mark the entire thread as read if I see some comments and don't want to go into
> > the details. It effectively means that I will never see the new version that
> > already was in the same thread!
> > 
> > ...
> > 
> > > --- a/drivers/staging/rtl8723bs/os_dep/osdep_service.c
> > > +++ b/drivers/staging/rtl8723bs/os_dep/osdep_service.c
> > > @@ -194,7 +194,8 @@ struct rtw_cbuf *rtw_cbuf_alloc(u32 size)
> > >  	struct rtw_cbuf *cbuf;
> > >  
> > >  	cbuf = kzalloc_flex(*cbuf, bufs, size);
> > > -	cbuf->size = size;
> > > +	if (cbuf)
> > > +		cbuf->size = size;
> > >  
> > >  	return cbuf;
> > 
> > Now to the code. This is still buggy. The problem is that the size is not
> > validated and when it's 0, the same issue (dereference of invalid pointer)
> > will happen.
> > 
> > Dan, JFYI k*alloc*(0) returns not NULL and not valid pointer.
> 
> kzalloc_flex() basically can't return the ZERO_SIZE pointer.

I meant ZERO_SIZE_PTR.

regards,
dan carpenter

On Tue, Apr 14, 2026 at 11:00:02AM +0300, Dan Carpenter wrote:
> On Tue, Apr 14, 2026 at 10:55:56AM +0300, Dan Carpenter wrote:
> > On Tue, Apr 14, 2026 at 10:46:13AM +0300, Andy Shevchenko wrote:
> > > On Tue, Apr 14, 2026 at 12:43:06PM +0530, Shyam Sunder Reddy Padira wrote:

...

> > > > --- a/drivers/staging/rtl8723bs/os_dep/osdep_service.c
> > > > +++ b/drivers/staging/rtl8723bs/os_dep/osdep_service.c
> > > > @@ -194,7 +194,8 @@ struct rtw_cbuf *rtw_cbuf_alloc(u32 size)
> > > >  	struct rtw_cbuf *cbuf;
> > > >  
> > > >  	cbuf = kzalloc_flex(*cbuf, bufs, size);
> > > > -	cbuf->size = size;
> > > > +	if (cbuf)
> > > > +		cbuf->size = size;
> > > >  
> > > >  	return cbuf;
> > > 
> > > Now to the code. This is still buggy. The problem is that the size is not
> > > validated and when it's 0, the same issue (dereference of invalid pointer)
> > > will happen.
> > > 
> > > Dan, JFYI k*alloc*(0) returns not NULL and not valid pointer.
> > 
> > kzalloc_flex() basically can't return the ZERO_SIZE pointer.
> 
> I meant ZERO_SIZE_PTR.

Ah, true, I missed the _flex vs. _objs part. Then the patch is fine as is.

-- 
With Best Regards,
Andy Shevchenko

Really _obj() can't be ZERO_SIZE_PTR either, or if they are then it's
very difficult to dereference them since they don't have any struct
members.

The main way that ZERO_SIZE_PTR bugs show up is with strings and NUL
terminators.

regards,
dan carpenter

On Tue, Apr 14, 2026 at 11:33:22AM +0300, Dan Carpenter wrote:
> Really _obj() can't be ZERO_SIZE_PTR either, or if they are then it's
> very difficult to dereference them since they don't have any struct
> members.
> 
> The main way that ZERO_SIZE_PTR bugs show up is with strings and NUL
> terminators.

Wouldn't it be the problem for

struct foo {
	u32 baz[];
};

?

-- 
With Best Regards,
Andy Shevchenko

On Tue, Apr 14, 2026 at 11:44:18AM +0300, Andy Shevchenko wrote:
> On Tue, Apr 14, 2026 at 11:33:22AM +0300, Dan Carpenter wrote:
> > Really _obj() can't be ZERO_SIZE_PTR either, or if they are then it's
> > very difficult to dereference them since they don't have any struct
> > members.
> > 
> > The main way that ZERO_SIZE_PTR bugs show up is with strings and NUL
> > terminators.
> 
> Wouldn't it be the problem for
> 
> struct foo {
> 	u32 baz[];
> };

There isn't really any difference between doing an out of bounds access
on an array with zero elements vs 10 elements.  That's the beauty of
the ZERO_SIZE_PTR is that it lets you treat zero as just another number
of elements.  But zero sizes can cause problems for code which does
things like "size - 1" but because of signedness that's ULONG_MAX.

regards,
dan carpenter

On Tue, Apr 14, 2026 at 11:44:22AM +0300, Andy Shevchenko wrote:
> On Tue, Apr 14, 2026 at 11:33:22AM +0300, Dan Carpenter wrote:
> > Really _obj() can't be ZERO_SIZE_PTR either, or if they are then it's
> > very difficult to dereference them since they don't have any struct
> > members.
> > 
> > The main way that ZERO_SIZE_PTR bugs show up is with strings and NUL
> > terminators.
> 
> Wouldn't it be the problem for
> 
> struct foo {
> 	u32 baz[];
> };
> 
> ?

In any case the __alloc_objs() is defined as

        const size_t __obj_size = size_mul(sizeof(TYPE), COUNT);        \
        (TYPE *)KMALLOC(__obj_size, GFP);                               \

Supplying COUNT as 0 will lead directly to ZERO_SIZE_PTR. No?

-- 
With Best Regards,
Andy Shevchenko

On Tue, Apr 14, 2026 at 12:43:06PM +0530, Shyam Sunder Reddy Padira wrote:
> The return value of kzalloc_flex() is used without
> ensuring that the allocation succeeded, and the
> pointer is dereferenced unconditionally.
> 
> Guard the access to the allocated structure to
> avoid a potential NULL pointer dereference if the
> allocation fails.
> 
> Fixes: 980cd426a257 ("staging: rtl8723bs: replace rtw_zmalloc() with kzalloc()")
> Signed-off-by: Shyam Sunder Reddy Padira <shyamsunderreddypadira@gmail.com>
> ---
> changes in v3:
> -Shortened commit hash in Fixes tag to 12 characters
> -Removed blank line between Signed-off-by and Fixes tag

Thanks!

Reviewed-by: Dan Carpenter <error27@gmail.com>

regards,
dan carpenter