[PATCH v1 1/1] usb: f_mass_storage: Bump local buffer size in fsg_common_create_luns()

[PATCH v1 1/1] usb: f_mass_storage: Bump local buffer size in fsg_common_create_luns()

[Andy Shevchenko]

GCC is not happy about the buffer size:

drivers/usb/gadget/function/f_mass_storage.c:2970:48: error: ‘%d’ directive output may be truncated writing between 1 and 9 bytes into a region of size 5 [-Werror=format-truncation=]

Bump the size to get it enough for all possible values.

Note, the existing comment is wrong as size 8 for the whole buffer doesn't
cover 100 mil numbers, hence drop it altogether.

Fixes: b27c08c953e9 ("usb: gadget: f_mass_storage: create lun creation helpers for use in fsg_common_init")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/usb/gadget/function/f_mass_storage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
index b7b06cb79ff5..3d1823b74cd3 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -2961,7 +2961,7 @@ EXPORT_SYMBOL_GPL(fsg_common_create_lun);
 
 int fsg_common_create_luns(struct fsg_common *common, struct fsg_config *cfg)
 {
-	char buf[8]; /* enough for 100000000 different numbers, decimal */
+	char buf[14];
 	int i, rc;
 
 	fsg_common_remove_luns(common);
-- 
2.50.1

Re: [PATCH v1 1/1] usb: f_mass_storage: Bump local buffer size in fsg_common_create_luns()

[Alan Stern]

On Mon, Aug 17, 2026 at 12:47:34PM +0200, Andy Shevchenko wrote:
> GCC is not happy about the buffer size:
> 
> drivers/usb/gadget/function/f_mass_storage.c:2970:48: error: ‘%d’ directive output may be truncated writing between 1 and 9 bytes into a region of size 5 [-Werror=format-truncation=]
> 
> Bump the size to get it enough for all possible values.
> 
> Note, the existing comment is wrong as size 8 for the whole buffer doesn't
> cover 100 mil numbers, hence drop it altogether.

It seems highly unlikely that anyone would ever want to create 100 
million LUNs.  Why not limit the number of LUNs to some more reasonable 
value, like 1000?

Alan Stern

> Fixes: b27c08c953e9 ("usb: gadget: f_mass_storage: create lun creation helpers for use in fsg_common_init")
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/usb/gadget/function/f_mass_storage.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
> index b7b06cb79ff5..3d1823b74cd3 100644
> --- a/drivers/usb/gadget/function/f_mass_storage.c
> +++ b/drivers/usb/gadget/function/f_mass_storage.c
> @@ -2961,7 +2961,7 @@ EXPORT_SYMBOL_GPL(fsg_common_create_lun);
>  
>  int fsg_common_create_luns(struct fsg_common *common, struct fsg_config *cfg)
>  {
> -	char buf[8]; /* enough for 100000000 different numbers, decimal */
> +	char buf[14];
>  	int i, rc;
>  
>  	fsg_common_remove_luns(common);
> -- 
> 2.50.1
> 
> 

Re: [PATCH v1 1/1] usb: f_mass_storage: Bump local buffer size in fsg_common_create_luns()

[Andy Shevchenko]

On Mon, Aug 17, 2026 at 09:47:45AM -0400, Alan Stern wrote:
> On Mon, Aug 17, 2026 at 12:47:34PM +0200, Andy Shevchenko wrote:
> > GCC is not happy about the buffer size:
> > 
> > drivers/usb/gadget/function/f_mass_storage.c:2970:48: error: ‘%d’ directive output may be truncated writing between 1 and 9 bytes into a region of size 5 [-Werror=format-truncation=]
> > 
> > Bump the size to get it enough for all possible values.
> > 
> > Note, the existing comment is wrong as size 8 for the whole buffer doesn't
> > cover 100 mil numbers, hence drop it altogether.
> 
> It seems highly unlikely that anyone would ever want to create 100 million
> LUNs.

Completely agree (but see below).

> Why not limit the number of LUNs to some more reasonable value, like 1000?

I chose the robust way as different versions of the compiler may or may not
that limit (yes, we had such a case in the past [1] and it required to replace
also specifier and variable type altogether. Given that, I'm not feeling to
rework that way. Up to you to implement, though. If you think this patch is
not good enough, consider this then as a bug report (with `make W=1` it breaks
the build, exactly what my case is).

Thanks for the review.

[1]: 239afba8b9f3 ("leds: pca955x: Avoid potential overflow when filling default_label (take 2)")

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] usb: f_mass_storage: Bump local buffer size in fsg_common_create_luns()

[Alan Stern]

On Mon, Aug 17, 2026 at 06:00:53PM +0300, Andy Shevchenko wrote:
> On Mon, Aug 17, 2026 at 09:47:45AM -0400, Alan Stern wrote:
> > On Mon, Aug 17, 2026 at 12:47:34PM +0200, Andy Shevchenko wrote:
> > > GCC is not happy about the buffer size:
> > > 
> > > drivers/usb/gadget/function/f_mass_storage.c:2970:48: error: ‘%d’ directive output may be truncated writing between 1 and 9 bytes into a region of size 5 [-Werror=format-truncation=]
> > > 
> > > Bump the size to get it enough for all possible values.
> > > 
> > > Note, the existing comment is wrong as size 8 for the whole buffer doesn't
> > > cover 100 mil numbers, hence drop it altogether.
> > 
> > It seems highly unlikely that anyone would ever want to create 100 million
> > LUNs.
> 
> Completely agree (but see below).
> 
> > Why not limit the number of LUNs to some more reasonable value, like 1000?
> 
> I chose the robust way as different versions of the compiler may or may not
> that limit (yes, we had such a case in the past [1] and it required to replace
> also specifier and variable type altogether. Given that, I'm not feeling to
> rework that way. Up to you to implement, though. If you think this patch is
> not good enough, consider this then as a bug report (with `make W=1` it breaks
> the build, exactly what my case is).

Okay, now I get it.  The compilers' limitations are a big part of the 
reason for this change.

And looking through the code, I see the only way that the existing 
drivers ever construct a struct fsg_config is by calling 
fsg_config_from_params(), which does indeed limit the number of LUNs to 
FSG_MAX_LUNS.

So yes, please update the patch description to mention that although 
cfg->nluns is limited to FSG_MAX_LUNS (16), the compiler doesn't realize 
this and complains about the buffer size.  Then you can add:

Acked-by: Alan Stern <stern@rowland.harvard.edu>

Do you think it's worth adding an explicit check in 
fsg_common_create_luns(), such as:

	if (cfg->nluns > FSG_MAX_LUNS)
		return -EINVAL;

to catch the case where someone tries to bypass 
fsg_config_from_params()?  That alone might well be enough to prevent 
the compiler from warning about the field length.

Alan Stern

Re: [PATCH v1 1/1] usb: f_mass_storage: Bump local buffer size in fsg_common_create_luns()

[Andy Shevchenko]

On Mon, Aug 17, 2026 at 11:31:51AM -0400, Alan Stern wrote:
> On Mon, Aug 17, 2026 at 06:00:53PM +0300, Andy Shevchenko wrote:
> > On Mon, Aug 17, 2026 at 09:47:45AM -0400, Alan Stern wrote:
> > > On Mon, Aug 17, 2026 at 12:47:34PM +0200, Andy Shevchenko wrote:
> > > > GCC is not happy about the buffer size:
> > > > 
> > > > drivers/usb/gadget/function/f_mass_storage.c:2970:48: error: ‘%d’ directive output may be truncated writing between 1 and 9 bytes into a region of size 5 [-Werror=format-truncation=]
> > > > 
> > > > Bump the size to get it enough for all possible values.
> > > > 
> > > > Note, the existing comment is wrong as size 8 for the whole buffer doesn't
> > > > cover 100 mil numbers, hence drop it altogether.
> > > 
> > > It seems highly unlikely that anyone would ever want to create 100 million
> > > LUNs.
> > 
> > Completely agree (but see below).
> > 
> > > Why not limit the number of LUNs to some more reasonable value, like 1000?
> > 
> > I chose the robust way as different versions of the compiler may or may not
> > that limit (yes, we had such a case in the past [1] and it required to replace
> > also specifier and variable type altogether. Given that, I'm not feeling to
> > rework that way. Up to you to implement, though. If you think this patch is
> > not good enough, consider this then as a bug report (with `make W=1` it breaks
> > the build, exactly what my case is).
> 
> Okay, now I get it.  The compilers' limitations are a big part of the 
> reason for this change.
> 
> And looking through the code, I see the only way that the existing 
> drivers ever construct a struct fsg_config is by calling 
> fsg_config_from_params(), which does indeed limit the number of LUNs to 
> FSG_MAX_LUNS.
> 
> So yes, please update the patch description to mention that although 
> cfg->nluns is limited to FSG_MAX_LUNS (16), the compiler doesn't realize 
> this and complains about the buffer size.  Then you can add:
> 
> Acked-by: Alan Stern <stern@rowland.harvard.edu>

Sure, will do!

> Do you think it's worth adding an explicit check in 
> fsg_common_create_luns(), such as:
> 
> 	if (cfg->nluns > FSG_MAX_LUNS)
> 		return -EINVAL;
> 
> to catch the case where someone tries to bypass 
> fsg_config_from_params()?  That alone might well be enough to prevent 
> the compiler from warning about the field length.

It does not fix the compiler from warning.
gcc (Debian 14.2.0-19) 14.2.0

-- 
With Best Regards,
Andy Shevchenko