linux-kernel - Re: [PATCH v1 1/1] usb: hcd: Bump local buffer size in rh

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250117195213.651dc338@pumpkin>
Date: Fri, 17 Jan 2025 19:52:13 +0000
From: David Laight <david.laight.linux@...il.com>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 1/1] usb: hcd: Bump local buffer size in rh_string()

On Thu, 16 Jan 2025 18:05:43 +0200
Andy Shevchenko <andriy.shevchenko@...ux.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@...ux.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};
>