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

[David Laight <david.laight.linux@gmail.com>]

On Thu, 16 Jan 2025 18:05:43 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> GCC is not happy about the buffer size:
> 
> drivers/usb/core/hcd.c:441:48: error: ‘%s’ directive output may be truncated writing up to 64 bytes into a region of size between 35 and 99 [-Werror=format-truncation=]
>   441 |                 snprintf (buf, sizeof buf, "%s %s %s", init_utsname()->sysname,
>       |                                                ^~
>   442 |                         init_utsname()->release, hcd->driver->description);
>       |                         ~~~~~~~~~~~~~~~~~~~~~~~
> 
> Bump the size to get it enough for the possible strings.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/usb/core/hcd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 0b2490347b9f..a75cf1f6d741 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -415,7 +415,7 @@ ascii2desc(char const *s, u8 *buf, unsigned len)
>  static unsigned
>  rh_string(int id, struct usb_hcd const *hcd, u8 *data, unsigned len)
>  {
> -	char buf[100];
> +	char buf[160];

Pretty pointless - look at ascii2desc() just above.
(Converts to LE i6-bit chars with a leading type+length.)
It gets truncated to 126 characters.
Indeed the entire snprintf() is pretty pointless given what happens to the
data given that it is all strings.

Is the overall truncation even right?
The outer length is bounded to 254, but there may be fewer characters in the
buffer because the buffer length itself might be smaller.
Seems a recipe for disaster.

	David 


>  	char const *s;
>  	static char const langids[4] = {4, USB_DT_STRING, 0x09, 0x04};
>