[PATCH] USB: core: Use `krealloc()` in `usb_cache_string()`

[PATCH] USB: core: Use `krealloc()` in `usb_cache_string()`

[Bence Csókás via B4 Relay]

From: Bence Csókás <bence98@sch.bme.hu>

Instead of "shrinking" the allocation by `kmalloc()`ing a new, smaller
buffer, utilize `krealloc()` to shrink the existing allocation. This saves
a `memcpy()`, as well as guards against `smallbuf` allocation failure.

Signed-off-by: Bence Csókás <bence98@sch.bme.hu>
---
Using `krealloc()` makes this code from 2005 more readable as well as
robust. Nested `if`s were also unrolled.
---
 drivers/usb/core/message.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index ea970ddf8879..dfe61d8b913b 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1005,7 +1005,7 @@ int usb_string(struct usb_device *dev, int index, char *buf, size_t size)
 }
 EXPORT_SYMBOL_GPL(usb_string);
 
-/* one UTF-8-encoded 16-bit character has at most three bytes */
+/* one 16-bit character, when UTF-8-encoded, has at most three bytes */
 #define MAX_USB_STRING_SIZE (127 * 3 + 1)
 
 /**
@@ -1026,17 +1026,17 @@ char *usb_cache_string(struct usb_device *udev, int index)
 		return NULL;
 
 	buf = kmalloc(MAX_USB_STRING_SIZE, GFP_NOIO);
-	if (buf) {
-		len = usb_string(udev, index, buf, MAX_USB_STRING_SIZE);
-		if (len > 0) {
-			smallbuf = kmalloc(++len, GFP_NOIO);
-			if (!smallbuf)
-				return buf;
-			memcpy(smallbuf, buf, len);
-		}
+	if (!buf)
+		return NULL;
+
+	len = usb_string(udev, index, buf, MAX_USB_STRING_SIZE);
+	if (len <= 0) {
 		kfree(buf);
+		return NULL;
 	}
-	return smallbuf;
+
+	smallbuf = krealloc(buf, len + 1, GFP_NOIO);
+	return smallbuf ? : buf;
 }
 EXPORT_SYMBOL_GPL(usb_cache_string);
 

---
base-commit: 80234b5ab240f52fa45d201e899e207b9265ef91
change-id: 20260311-usb-krealloc-4e413f239a9f

Best regards,
-- 
Bence Csókás <bence98@sch.bme.hu>

Re: [PATCH] USB: core: Use `krealloc()` in `usb_cache_string()`

[Greg Kroah-Hartman]

On Thu, Mar 12, 2026 at 12:06:35AM +0100, Bence Csókás via B4 Relay wrote:
> From: Bence Csókás <bence98@sch.bme.hu>
> 
> Instead of "shrinking" the allocation by `kmalloc()`ing a new, smaller
> buffer, utilize `krealloc()` to shrink the existing allocation. This saves
> a `memcpy()`, as well as guards against `smallbuf` allocation failure.
> 
> Signed-off-by: Bence Csókás <bence98@sch.bme.hu>
> ---
> Using `krealloc()` makes this code from 2005 more readable as well as
> robust. Nested `if`s were also unrolled.

How is it more "robust" now?

> ---
>  drivers/usb/core/message.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)

Same number of lines. Well, not quite, because I'm going to ask you to
remove the ?: stuff below...

> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
> index ea970ddf8879..dfe61d8b913b 100644
> --- a/drivers/usb/core/message.c
> +++ b/drivers/usb/core/message.c
> @@ -1005,7 +1005,7 @@ int usb_string(struct usb_device *dev, int index, char *buf, size_t size)
>  }
>  EXPORT_SYMBOL_GPL(usb_string);
>  
> -/* one UTF-8-encoded 16-bit character has at most three bytes */
> +/* one 16-bit character, when UTF-8-encoded, has at most three bytes */

Why change this?

>  #define MAX_USB_STRING_SIZE (127 * 3 + 1)
>  
>  /**
> @@ -1026,17 +1026,17 @@ char *usb_cache_string(struct usb_device *udev, int index)
>  		return NULL;
>  
>  	buf = kmalloc(MAX_USB_STRING_SIZE, GFP_NOIO);
> -	if (buf) {
> -		len = usb_string(udev, index, buf, MAX_USB_STRING_SIZE);
> -		if (len > 0) {
> -			smallbuf = kmalloc(++len, GFP_NOIO);
> -			if (!smallbuf)
> -				return buf;
> -			memcpy(smallbuf, buf, len);
> -		}
> +	if (!buf)
> +		return NULL;
> +
> +	len = usb_string(udev, index, buf, MAX_USB_STRING_SIZE);
> +	if (len <= 0) {
>  		kfree(buf);
> +		return NULL;
>  	}
> -	return smallbuf;
> +
> +	smallbuf = krealloc(buf, len + 1, GFP_NOIO);
> +	return smallbuf ? : buf;

I hate ? : except where it can only be used (i.e. in function
arguments), so please spell it out exactly what you are doing here.

Also, how was this tested?

thanks,

greg k-h

Re: [PATCH] USB: core: Use `krealloc()` in `usb_cache_string()`

[Bence Csókás]

Hi Greg,

On 3/12/26 06:02, Greg Kroah-Hartman wrote:
> On Thu, Mar 12, 2026 at 12:06:35AM +0100, Bence Csókás via B4 Relay wrote:
>> From: Bence Csókás <bence98@sch.bme.hu>
>>
>> Instead of "shrinking" the allocation by `kmalloc()`ing a new, smaller
>> buffer, utilize `krealloc()` to shrink the existing allocation. This saves
>> a `memcpy()`, as well as guards against `smallbuf` allocation failure.
>>
>> Signed-off-by: Bence Csókás <bence98@sch.bme.hu>
>> ---
>> Using `krealloc()` makes this code from 2005 more readable as well as
>> robust. Nested `if`s were also unrolled.
> 
> How is it more "robust" now?

My understanding was (at least from reading mm/slub.c, and also by 
analogue to libc `realloc()`), that krealloc-ing an allocation to be 
smaller (without changing alignment or NUMA requirements) just shrinks 
it in-place, instead of allocating a new, smaller buffer (which is what 
the code was doing before, essentially "by hand"). Under memory 
pressure, this smaller allocation might fail, even though by the end, 
more memory will have been freed than what was initially allocated.

>> ---
>>   drivers/usb/core/message.c | 20 ++++++++++----------
>>   1 file changed, 10 insertions(+), 10 deletions(-)
> 
> Same number of lines. Well, not quite, because I'm going to ask you to
> remove the ?: stuff below...
> 
>> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
>> index ea970ddf8879..dfe61d8b913b 100644
>> --- a/drivers/usb/core/message.c
>> +++ b/drivers/usb/core/message.c
>> @@ -1005,7 +1005,7 @@ int usb_string(struct usb_device *dev, int index, char *buf, size_t size)
>>   }
>>   EXPORT_SYMBOL_GPL(usb_string);
>>   
>> -/* one UTF-8-encoded 16-bit character has at most three bytes */
>> +/* one 16-bit character, when UTF-8-encoded, has at most three bytes */
> 
> Why change this?

Right. While I was mentally parsing `usb_cache_string()` I came across 
this comment and found it very confusingly written. How can "one [...] 
16-bit character" be anything else than two bytes (assuming 8-bit bytes; 
let's ignore historical architectures like the PDP-10)? The answer is 
that the UTF-8 *encoding* has <= 3 bytes, not the 16-bit UCS-2 character 
it encodes.

> 
>>   #define MAX_USB_STRING_SIZE (127 * 3 + 1)
>>   
>>   /**
>> @@ -1026,17 +1026,17 @@ char *usb_cache_string(struct usb_device *udev, int index)
>>   		return NULL;
>>   
>>   	buf = kmalloc(MAX_USB_STRING_SIZE, GFP_NOIO);
>> -	if (buf) {
>> -		len = usb_string(udev, index, buf, MAX_USB_STRING_SIZE);
>> -		if (len > 0) {
>> -			smallbuf = kmalloc(++len, GFP_NOIO);
>> -			if (!smallbuf)
>> -				return buf;
>> -			memcpy(smallbuf, buf, len);
>> -		}
>> +	if (!buf)
>> +		return NULL;
>> +
>> +	len = usb_string(udev, index, buf, MAX_USB_STRING_SIZE);
>> +	if (len <= 0) {
>>   		kfree(buf);
>> +		return NULL;
>>   	}
>> -	return smallbuf;
>> +
>> +	smallbuf = krealloc(buf, len + 1, GFP_NOIO);
>> +	return smallbuf ? : buf;
> 
> I hate ? : except where it can only be used (i.e. in function
> arguments), so please spell it out exactly what you are doing here.

Sure.

> Also, how was this tested?

I just compiled and booted it on my Arch box (with the original vendor 
config), an AthlonII X2 PC. I'm now typing this mail on a USB keyboard 
and mouse under Plasma, running this kernel :) I also plugged in a 
pendrive, mounted it, `ls`'d the mount, unmounted, unplugged, and did 
this 2 more times.

I realize I should probably put this info under the dashes. I'll prepare 
a v2.

> thanks,
> 
> greg k-h
> 

Bence

Re: [PATCH] USB: core: Use `krealloc()` in `usb_cache_string()`

[Greg Kroah-Hartman]

On Sun, Mar 15, 2026 at 10:40:48AM +0100, Bence Csókás wrote:
> > Also, how was this tested?
> 
> I just compiled and booted it on my Arch box (with the original vendor
> config), an AthlonII X2 PC. I'm now typing this mail on a USB keyboard and
> mouse under Plasma, running this kernel :) I also plugged in a pendrive,
> mounted it, `ls`'d the mount, unmounted, unplugged, and did this 2 more
> times.
> 
> I realize I should probably put this info under the dashes. I'll prepare a
> v2.

Try looking at the USB strings in the device, as that's the path here
that is exercised.  Just using the device doesn't actually grab them
from the descriptor table.

And sorry for the extra review, I'm a bit picky about this function, it
was my first contribution to Linux way back in 1999 or so, and happen to
know too much about USB descriptor strings :)

thanks,

greg k-h

Re: [PATCH] USB: core: Use `krealloc()` in `usb_cache_string()`

[Bence Csókás]

Hi,

On 3/15/26 10:47, Greg Kroah-Hartman wrote:
> On Sun, Mar 15, 2026 at 10:40:48AM +0100, Bence Csókás wrote:
>>> Also, how was this tested?
>>
>> I just compiled and booted it on my Arch box (with the original vendor
>> config), an AthlonII X2 PC. I'm now typing this mail on a USB keyboard and
>> mouse under Plasma, running this kernel :) I also plugged in a pendrive,
>> mounted it, `ls`'d the mount, unmounted, unplugged, and did this 2 more
>> times.
>>
>> I realize I should probably put this info under the dashes. I'll prepare a
>> v2.
> 
> Try looking at the USB strings in the device, as that's the path here
> that is exercised.  Just using the device doesn't actually grab them
> from the descriptor table.

Sure.

   $ sudo lsusb -vvv | grep iProduct
   can't get debug descriptor: Resource temporarily unavailable
     iProduct                2 OHCI PCI host controller
     iProduct                2 HP USB Smart Card Keyboard
   can't get device qualifier: Resource temporarily unavailable
   can't get debug descriptor: Resource temporarily unavailable
     iProduct                2 USB OPTICAL MOUSE
   can't get debug descriptor: Resource temporarily unavailable
   can't get device qualifier: Resource temporarily unavailable
   can't get debug descriptor: Resource temporarily unavailable
     iProduct                2 EHCI Host Controller
     iProduct                2 EHCI Host Controller
   can't get device qualifier: Resource temporarily unavailable
   can't get debug descriptor: Resource temporarily unavailable
   can't get debug descriptor: Resource temporarily unavailable
     iProduct                2 Disk 2.0
     iProduct                2 EHCI Host Controller
   can't get device qualifier: Resource temporarily unavailable
   can't get debug descriptor: Resource temporarily unavailable
   can't get debug descriptor: Resource temporarily unavailable
     iProduct                2 OHCI PCI host controller
     iProduct                2 OHCI PCI host controller
   can't get debug descriptor: Resource temporarily unavailable
   can't get debug descriptor: Resource temporarily unavailable
     iProduct                2 OHCI PCI host controller

Are these -EAGAINs cause for concern?

> And sorry for the extra review, I'm a bit picky about this function, it
> was my first contribution to Linux way back in 1999 or so, and happen to
> know too much about USB descriptor strings :)

A good review is a thorough review.

> thanks,
> 
> greg k-h
> 

Bence

Re: [PATCH] USB: core: Use `krealloc()` in `usb_cache_string()`

[Bence Csókás]

On 3/15/26 10:59, Bence Csókás wrote:
> Hi,
> 
> On 3/15/26 10:47, Greg Kroah-Hartman wrote:
>> On Sun, Mar 15, 2026 at 10:40:48AM +0100, Bence Csókás wrote:
>>>> Also, how was this tested?
>>>
>>> I just compiled and booted it on my Arch box (with the original vendor
>>> config), an AthlonII X2 PC. I'm now typing this mail on a USB 
>>> keyboard and
>>> mouse under Plasma, running this kernel :) I also plugged in a pendrive,
>>> mounted it, `ls`'d the mount, unmounted, unplugged, and did this 2 more
>>> times.
>>>
>>> I realize I should probably put this info under the dashes. I'll 
>>> prepare a
>>> v2.
>>
>> Try looking at the USB strings in the device, as that's the path here
>> that is exercised.  Just using the device doesn't actually grab them
>> from the descriptor table.
> 
> Sure.
> 
>    $ sudo lsusb -vvv | grep iProduct
>    can't get debug descriptor: Resource temporarily unavailable
>      iProduct                2 OHCI PCI host controller
>      iProduct                2 HP USB Smart Card Keyboard
>    can't get device qualifier: Resource temporarily unavailable
>    can't get debug descriptor: Resource temporarily unavailable
>      iProduct                2 USB OPTICAL MOUSE
>    can't get debug descriptor: Resource temporarily unavailable
>    can't get device qualifier: Resource temporarily unavailable
>    can't get debug descriptor: Resource temporarily unavailable
>      iProduct                2 EHCI Host Controller
>      iProduct                2 EHCI Host Controller
>    can't get device qualifier: Resource temporarily unavailable
>    can't get debug descriptor: Resource temporarily unavailable
>    can't get debug descriptor: Resource temporarily unavailable
>      iProduct                2 Disk 2.0
>      iProduct                2 EHCI Host Controller
>    can't get device qualifier: Resource temporarily unavailable
>    can't get debug descriptor: Resource temporarily unavailable
>    can't get debug descriptor: Resource temporarily unavailable
>      iProduct                2 OHCI PCI host controller
>      iProduct                2 OHCI PCI host controller
>    can't get debug descriptor: Resource temporarily unavailable
>    can't get debug descriptor: Resource temporarily unavailable
>      iProduct                2 OHCI PCI host controller
> 
> Are these -EAGAINs cause for concern?

It happens with the vendor kernel (6.19.8-arch1-1) too, so it's probably 
just some weirdness in my HW...

>> And sorry for the extra review, I'm a bit picky about this function, it
>> was my first contribution to Linux way back in 1999 or so, and happen to
>> know too much about USB descriptor strings :)
> 
> A good review is a thorough review.
> 
>> thanks,
>>
>> greg k-h
>>
> 
> Bence

Re: [PATCH] USB: core: Use `krealloc()` in `usb_cache_string()`

[Greg Kroah-Hartman]

On Sun, Mar 15, 2026 at 11:34:52AM +0100, Bence Csókás wrote:
> 
> 
> On 3/15/26 10:59, Bence Csókás wrote:
> > Hi,
> > 
> > On 3/15/26 10:47, Greg Kroah-Hartman wrote:
> > > On Sun, Mar 15, 2026 at 10:40:48AM +0100, Bence Csókás wrote:
> > > > > Also, how was this tested?
> > > > 
> > > > I just compiled and booted it on my Arch box (with the original vendor
> > > > config), an AthlonII X2 PC. I'm now typing this mail on a USB
> > > > keyboard and
> > > > mouse under Plasma, running this kernel :) I also plugged in a pendrive,
> > > > mounted it, `ls`'d the mount, unmounted, unplugged, and did this 2 more
> > > > times.
> > > > 
> > > > I realize I should probably put this info under the dashes. I'll
> > > > prepare a
> > > > v2.
> > > 
> > > Try looking at the USB strings in the device, as that's the path here
> > > that is exercised.  Just using the device doesn't actually grab them
> > > from the descriptor table.
> > 
> > Sure.
> > 
> >    $ sudo lsusb -vvv | grep iProduct
> >    can't get debug descriptor: Resource temporarily unavailable
> >      iProduct                2 OHCI PCI host controller
> >      iProduct                2 HP USB Smart Card Keyboard
> >    can't get device qualifier: Resource temporarily unavailable
> >    can't get debug descriptor: Resource temporarily unavailable
> >      iProduct                2 USB OPTICAL MOUSE
> >    can't get debug descriptor: Resource temporarily unavailable
> >    can't get device qualifier: Resource temporarily unavailable
> >    can't get debug descriptor: Resource temporarily unavailable
> >      iProduct                2 EHCI Host Controller
> >      iProduct                2 EHCI Host Controller
> >    can't get device qualifier: Resource temporarily unavailable
> >    can't get debug descriptor: Resource temporarily unavailable
> >    can't get debug descriptor: Resource temporarily unavailable
> >      iProduct                2 Disk 2.0
> >      iProduct                2 EHCI Host Controller
> >    can't get device qualifier: Resource temporarily unavailable
> >    can't get debug descriptor: Resource temporarily unavailable
> >    can't get debug descriptor: Resource temporarily unavailable
> >      iProduct                2 OHCI PCI host controller
> >      iProduct                2 OHCI PCI host controller
> >    can't get debug descriptor: Resource temporarily unavailable
> >    can't get debug descriptor: Resource temporarily unavailable
> >      iProduct                2 OHCI PCI host controller
> > 
> > Are these -EAGAINs cause for concern?
> 
> It happens with the vendor kernel (6.19.8-arch1-1) too, so it's probably
> just some weirdness in my HW...

You can't get the debug descriptor when a driver is bound to the device.
Also look at the iManufacturer string, that should be coming from the
device (except for the root hub, that descriptor comes from the kernel.)

thanks,

greg k-h