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

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

[Jeffin Philip]

On Mon, 17 Aug 2026 10:01:36 -0400, Alan Stern wrote:
>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.

Kconfig currently sets the limit from 2 to 256 with default as 2. However,
we use that only at build time. So, if we enter during runtime via configfs,
we set page content to 0\0 and get the null pointer dereference. So we will
need to add the check for num < 2 in fsg_opts_num_buffers_store() and return
EINVAL? Upper bound is 256 which is below what kstrtou8() can return anyway.

Thanks,
Jeffin.

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

[Jeffin Philip]

On Mon, 17 Aug 2026 21:33:19 +0530, Jeffin Philip wrote:
>Kconfig currently sets the limit from 2 to 256 with default as 2. However,
>we use that only at build time. So, if we enter during runtime via configfs,
>we set page content to 0\0 and get the null pointer dereference. So we will
>need to add the check for num < 2 in fsg_opts_num_buffers_store() and return
>EINVAL? Upper bound is 256 which is below what kstrtou8() can return anyway.

Correction on the last sentence: Upper bound is 256 which is _above_ what 
kstrtou8() can return anyway.

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 09:39:16PM +0530, Jeffin Philip wrote:
> On Mon, 17 Aug 2026 21:33:19 +0530, Jeffin Philip wrote:
> >Kconfig currently sets the limit from 2 to 256 with default as 2. However,
> >we use that only at build time. So, if we enter during runtime via configfs,
> >we set page content to 0\0 and get the null pointer dereference. So we will
> >need to add the check for num < 2 in fsg_opts_num_buffers_store() and return
> >EINVAL?

Or put the check in fsg_common_set_num_buffers(), which gets called on 
all the pathways.

Alan Stern

>  Upper bound is 256 which is below what kstrtou8() can return anyway.
> 
> Correction on the last sentence: Upper bound is 256 which is _above_ what 
> kstrtou8() can return anyway.