[PATCH] usb: gadget: f_mass_storage: fix null pointer dereference in fsg_common_set_num_buffers()

[PATCH] usb: gadget: f_mass_storage: fix null pointer dereference in fsg_common_set_num_buffers()

[Jeffin Philip]

In fsg_common_set_num_buffers(), n can be 0 as conversion to u8
using kstrtou8() in fsg_opts_num_buffers_store() can return
values from _0_ to 255. When passing 0 as "n" value to kzalloc_objs,
it can return a ZERO_SIZE_PTR, which passes the null check for
buffhds. This leads to a null pointer dereference later in bh->next
in the do while loop. Fix this by adding a check for n = 0 case and
returning -EINVAL if n is 0.

Reported-by: syzbot+791be35f1fbcc85d06d7@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=791be35f1fbcc85d06d7
Fixes: fe5a6c48fd95 ("usb: gadget: storage: get rid of fsg_num_buffers_validate()")
Cc: stable@vger.kernel.org
Signed-off-by: Jeffin Philip <jeffinphilip14@gmail.com>
---
 drivers/usb/gadget/function/f_mass_storage.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
index a50743caf083..640d3bcb7bf0 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -2747,6 +2747,9 @@ int fsg_common_set_num_buffers(struct fsg_common *common, unsigned int n)
 	struct fsg_buffhd *bh, *buffhds;
 	int i;
 
+	if (!n)
+		return -EINVAL;
+
 	buffhds = kzalloc_objs(*buffhds, n);
 	if (!buffhds)
 		return -ENOMEM;
-- 
2.55.0

Re: [PATCH] usb: gadget: f_mass_storage: fix null pointer dereference in fsg_common_set_num_buffers()

[Greg KH]

On Mon, Aug 17, 2026 at 04:38:39PM +0530, Jeffin Philip wrote:
> In fsg_common_set_num_buffers(), n can be 0 as conversion to u8
> using kstrtou8() in fsg_opts_num_buffers_store() can return
> values from _0_ to 255. When passing 0 as "n" value to kzalloc_objs,
> it can return a ZERO_SIZE_PTR, which passes the null check for
> buffhds.

But that is probably a bad idea, right?  Shouldn't we fix that error
first?

thanks,

greg k-h

Re: [PATCH] usb: gadget: f_mass_storage: fix null pointer dereference in fsg_common_set_num_buffers()

[Jeffin Philip]

On Mon, 17 Aug 2026 13:10:27 +0200, Greg KH wrote:
>But that is probably a bad idea, right?  Shouldn't we fix that error
>first?

Thanks for the review. Don't quite understand what I need to fix here
(respectfully), should we harden the buffhds null check to include 
ZERO_SIZE_PTRs too, something like this should suffice?:

diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
index 640d3bcb7bf0..98643d53cc7a 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -2747,11 +2747,11 @@ int fsg_common_set_num_buffers(struct fsg_common *common, unsigned int n)
 	struct fsg_buffhd *bh, *buffhds;
 	int i;
  
 	buffhds = kzalloc_objs(*buffhds, n);
-	if (!buffhds)
+	if (ZERO_OR_NULL_PTR(buffhds))
 		return -ENOMEM;
 
 	/* Data buffers cyclic list */

Thanks,
Jeffin.

Re: [PATCH] usb: gadget: f_mass_storage: fix null pointer dereference in fsg_common_set_num_buffers()

[Greg KH]

On Mon, Aug 17, 2026 at 06:32:08PM +0530, Jeffin Philip wrote:
> On Mon, 17 Aug 2026 13:10:27 +0200, Greg KH wrote:
> >But that is probably a bad idea, right?  Shouldn't we fix that error
> >first?
> 
> Thanks for the review. Don't quite understand what I need to fix here
> (respectfully), should we harden the buffhds null check to include 
> ZERO_SIZE_PTRs too, something like this should suffice?:
> 
> diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
> index 640d3bcb7bf0..98643d53cc7a 100644
> --- a/drivers/usb/gadget/function/f_mass_storage.c
> +++ b/drivers/usb/gadget/function/f_mass_storage.c
> @@ -2747,11 +2747,11 @@ int fsg_common_set_num_buffers(struct fsg_common *common, unsigned int n)
>  	struct fsg_buffhd *bh, *buffhds;
>  	int i;
>   
>  	buffhds = kzalloc_objs(*buffhds, n);
> -	if (!buffhds)
> +	if (ZERO_OR_NULL_PTR(buffhds))

That doesn't look right, think about what your previous commit said it
was doing and why I thought that was a bad idea.

Was it created by a LLM?

thanks,

greg k-h

Re: [PATCH] usb: gadget: f_mass_storage: fix null pointer dereference in fsg_common_set_num_buffers()

[Alan Stern]

On Mon, Aug 17, 2026 at 04:38:39PM +0530, Jeffin Philip wrote:
> In fsg_common_set_num_buffers(), n can be 0 as conversion to u8
> using kstrtou8() in fsg_opts_num_buffers_store() can return
> values from _0_ to 255. When passing 0 as "n" value to kzalloc_objs,
> it can return a ZERO_SIZE_PTR, which passes the null check for
> buffhds. This leads to a null pointer dereference later in bh->next
> in the do while loop. Fix this by adding a check for n = 0 case and
> returning -EINVAL if n is 0.
> 
> Reported-by: syzbot+791be35f1fbcc85d06d7@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=791be35f1fbcc85d06d7
> Fixes: fe5a6c48fd95 ("usb: gadget: storage: get rid of fsg_num_buffers_validate()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jeffin Philip <jeffinphilip14@gmail.com>

In fe5a6c48fd95 ("usb: gadget: storage: get rid of 
fsg_num_buffers_validate()"), the code that was changed originally 
required the number to lie between 2 and 32.  Even 1 was not acceptable.

Alan Stern

> ---
>  drivers/usb/gadget/function/f_mass_storage.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
> index a50743caf083..640d3bcb7bf0 100644
> --- a/drivers/usb/gadget/function/f_mass_storage.c
> +++ b/drivers/usb/gadget/function/f_mass_storage.c
> @@ -2747,6 +2747,9 @@ int fsg_common_set_num_buffers(struct fsg_common *common, unsigned int n)
>  	struct fsg_buffhd *bh, *buffhds;
>  	int i;
>  
> +	if (!n)
> +		return -EINVAL;
> +
>  	buffhds = kzalloc_objs(*buffhds, n);
>  	if (!buffhds)
>  		return -ENOMEM;
> -- 
> 2.55.0
> 
>