[PATCH] HID: usbhid: sanitize hid->uniq against non-printable bytes

Taylor Hewetson posted 1 patch 1 month, 4 weeks ago
[PATCH] HID: usbhid: sanitize hid->uniq against non-printable bytes
Posted by Taylor Hewetson 1 month, 4 weeks ago
Some USB HID devices (observed on ASUS ROG Azoth via its 2.4GHz
dongle, USB ID 0b05:1a85) report an iSerialNumber string whose
USB string descriptor declares a longer length than the actual
serial, leaving uninitialized firmware memory - including control
characters such as 0x18 - appended to the returned string.

These non-printable bytes propagate into hid->uniq, which in turn
populates /sys/class/input/inputN/uniq. Downstream userspace
components (systemd sd-device property_is_valid(), and by extension
mutter input enumeration on GNOME Wayland sessions) reject devices
with control characters in their uniq, rendering otherwise-
functional input devices unusable in graphical sessions despite
the kernel input layer correctly translating keypresses.

Truncate hid->uniq at the first byte outside the printable ASCII
range (0x20..0x7e) after the serial is read.

Signed-off-by: Taylor Hewetson <taylor@exponent.digital>
---
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1427,8 +1427,17 @@
 		snprintf(hid->phys + len, sizeof(hid->phys) - len,
 			 "%d", intf->altsetting[0].desc.bInterfaceNumber);
 
-	if (usb_string(dev, dev->descriptor.iSerialNumber, hid->uniq, 64) <= 0)
+	if (usb_string(dev, dev->descriptor.iSerialNumber, hid->uniq, 64) <= 0) {
 		hid->uniq[0] = 0;
+	} else {
+		size_t i;
+		for (i = 0; i < sizeof(hid->uniq) && hid->uniq[i]; i++) {
+			if (hid->uniq[i] < 0x20 || hid->uniq[i] > 0x7e) {
+				hid->uniq[i] = 0;
+				break;
+			}
+		}
+	}
 
 	usbhid = kzalloc(sizeof(*usbhid), GFP_KERNEL);
 	if (usbhid == NULL) {
Re: [PATCH] HID: usbhid: sanitize hid->uniq against non-printable bytes
Posted by Greg KH 1 month, 4 weeks ago
On Sat, Apr 18, 2026 at 02:58:23PM +1200, Taylor Hewetson wrote:
> Some USB HID devices (observed on ASUS ROG Azoth via its 2.4GHz
> dongle, USB ID 0b05:1a85) report an iSerialNumber string whose
> USB string descriptor declares a longer length than the actual
> serial, leaving uninitialized firmware memory - including control
> characters such as 0x18 - appended to the returned string.
> 
> These non-printable bytes propagate into hid->uniq, which in turn
> populates /sys/class/input/inputN/uniq. Downstream userspace
> components (systemd sd-device property_is_valid(), and by extension
> mutter input enumeration on GNOME Wayland sessions) reject devices
> with control characters in their uniq, rendering otherwise-
> functional input devices unusable in graphical sessions despite
> the kernel input layer correctly translating keypresses.
> 
> Truncate hid->uniq at the first byte outside the printable ASCII
> range (0x20..0x7e) after the serial is read.

Why aren't we doing this in the USB core instead of forcing all users of
this to do it instead?

thanks,

greg k-h
Re: [PATCH] HID: usbhid: sanitize hid->uniq against non-printable bytes
Posted by Eric Naim 1 month, 3 weeks ago
On 4/18/26 3:14 PM, Greg KH wrote:
> On Sat, Apr 18, 2026 at 02:58:23PM +1200, Taylor Hewetson wrote:
>> Some USB HID devices (observed on ASUS ROG Azoth via its 2.4GHz
>> dongle, USB ID 0b05:1a85) report an iSerialNumber string whose
>> USB string descriptor declares a longer length than the actual
>> serial, leaving uninitialized firmware memory - including control
>> characters such as 0x18 - appended to the returned string.
>>
>> These non-printable bytes propagate into hid->uniq, which in turn
>> populates /sys/class/input/inputN/uniq. Downstream userspace
>> components (systemd sd-device property_is_valid(), and by extension
>> mutter input enumeration on GNOME Wayland sessions) reject devices
>> with control characters in their uniq, rendering otherwise-
>> functional input devices unusable in graphical sessions despite
>> the kernel input layer correctly translating keypresses.
>>
>> Truncate hid->uniq at the first byte outside the printable ASCII
>> range (0x20..0x7e) after the serial is read.
> 
> Why aren't we doing this in the USB core instead of forcing all users of
> this to do it instead?

Should it be up to the kernel to do this as well? Currently this is only a
problem with systemd because they added this check, and it looks like they
have something in mind to fix it as well [1].

[1] https://github.com/systemd/systemd/issues/41339#issuecomment-4266429563

> 
> thanks,
> 
> greg k-h


-- 
Regards,
  Eric
Re: [PATCH] HID: usbhid: sanitize hid->uniq against non-printable bytes
Posted by Greg KH 1 month, 3 weeks ago
On Thu, Apr 23, 2026 at 05:55:00AM +0000, Eric Naim wrote:
> On 4/18/26 3:14 PM, Greg KH wrote:
> > On Sat, Apr 18, 2026 at 02:58:23PM +1200, Taylor Hewetson wrote:
> >> Some USB HID devices (observed on ASUS ROG Azoth via its 2.4GHz
> >> dongle, USB ID 0b05:1a85) report an iSerialNumber string whose
> >> USB string descriptor declares a longer length than the actual
> >> serial, leaving uninitialized firmware memory - including control
> >> characters such as 0x18 - appended to the returned string.
> >>
> >> These non-printable bytes propagate into hid->uniq, which in turn
> >> populates /sys/class/input/inputN/uniq. Downstream userspace
> >> components (systemd sd-device property_is_valid(), and by extension
> >> mutter input enumeration on GNOME Wayland sessions) reject devices
> >> with control characters in their uniq, rendering otherwise-
> >> functional input devices unusable in graphical sessions despite
> >> the kernel input layer correctly translating keypresses.
> >>
> >> Truncate hid->uniq at the first byte outside the printable ASCII
> >> range (0x20..0x7e) after the serial is read.
> > 
> > Why aren't we doing this in the USB core instead of forcing all users of
> > this to do it instead?
> 
> Should it be up to the kernel to do this as well? Currently this is only a
> problem with systemd because they added this check, and it looks like they
> have something in mind to fix it as well [1].
> 
> [1] https://github.com/systemd/systemd/issues/41339#issuecomment-4266429563

It's either up to the kernel, or every single userspace program that
reads the strings from a device.  Might as well do it in one place,
right?

thanks,

greg k-h
Re: [PATCH] HID: usbhid: sanitize hid->uniq against non-printable bytes
Posted by Oliver Neukum 1 month, 3 weeks ago

On 23.04.26 11:29, Greg KH wrote:
> On Thu, Apr 23, 2026 at 05:55:00AM +0000, Eric Naim wrote:
>> On 4/18/26 3:14 PM, Greg KH wrote:

>> [1] https://github.com/systemd/systemd/issues/41339#issuecomment-4266429563
> 
> It's either up to the kernel, or every single userspace program that
> reads the strings from a device.  Might as well do it in one place,
> right?

No, because that puts the assumption that user space is not interested
in what the device actually returns and uses these strings just
for printing.
Eg. you can no longer use this in a udev rule.

	Regards
		Oliver
Re: [PATCH] HID: usbhid: sanitize hid->uniq against non-printable bytes
Posted by Alan Stern 1 month, 3 weeks ago
On Thu, Apr 23, 2026 at 11:49:02AM +0200, Oliver Neukum wrote:
> 
> 
> On 23.04.26 11:29, Greg KH wrote:
> > On Thu, Apr 23, 2026 at 05:55:00AM +0000, Eric Naim wrote:
> > > On 4/18/26 3:14 PM, Greg KH wrote:
> 
> > > [1] https://github.com/systemd/systemd/issues/41339#issuecomment-4266429563
> > 
> > It's either up to the kernel, or every single userspace program that
> > reads the strings from a device.  Might as well do it in one place,
> > right?
> 
> No, because that puts the assumption that user space is not interested
> in what the device actually returns and uses these strings just
> for printing.
> Eg. you can no longer use this in a udev rule.

Also, the USB spec doesn't say anything about what characters are or 
aren't allowed in a string descriptor.  Anything, even a control 
character, is valid.  The kernel should not rule them and certainly 
shouldn't truncate a string descriptor.

Alan Stern
Re: [PATCH] HID: usbhid: sanitize hid->uniq against non-printable bytes
Posted by Eric Naim 1 month, 3 weeks ago
On 4/23/26 5:29 PM, Greg KH wrote:
> On Thu, Apr 23, 2026 at 05:55:00AM +0000, Eric Naim wrote:
>> On 4/18/26 3:14 PM, Greg KH wrote:
>>> On Sat, Apr 18, 2026 at 02:58:23PM +1200, Taylor Hewetson wrote:
>>>> Some USB HID devices (observed on ASUS ROG Azoth via its 2.4GHz
>>>> dongle, USB ID 0b05:1a85) report an iSerialNumber string whose
>>>> USB string descriptor declares a longer length than the actual
>>>> serial, leaving uninitialized firmware memory - including control
>>>> characters such as 0x18 - appended to the returned string.
>>>>
>>>> These non-printable bytes propagate into hid->uniq, which in turn
>>>> populates /sys/class/input/inputN/uniq. Downstream userspace
>>>> components (systemd sd-device property_is_valid(), and by extension
>>>> mutter input enumeration on GNOME Wayland sessions) reject devices
>>>> with control characters in their uniq, rendering otherwise-
>>>> functional input devices unusable in graphical sessions despite
>>>> the kernel input layer correctly translating keypresses.
>>>>
>>>> Truncate hid->uniq at the first byte outside the printable ASCII
>>>> range (0x20..0x7e) after the serial is read.
>>>
>>> Why aren't we doing this in the USB core instead of forcing all users of
>>> this to do it instead?
>>
>> Should it be up to the kernel to do this as well? Currently this is only a
>> problem with systemd because they added this check, and it looks like they
>> have something in mind to fix it as well [1].
>>
>> [1] https://github.com/systemd/systemd/issues/41339#issuecomment-4266429563
> 
> It's either up to the kernel, or every single userspace program that
> reads the strings from a device.  Might as well do it in one place,
> right?

That is a very good point. Thanks for the insight!

> 
> thanks,
> 
> greg k-h


-- 
Regards,
  Eric
USB: core: sanitize string descriptors against C0 control characters
Posted by Taylor Hewetson 1 month, 4 weeks ago
Some USB devices report string descriptors with a declared length
greater than the actual string, leaving uninitialized firmware memory
- often including C0 control characters such as 0x18 - appended to
the returned string. This has been observed on the ASUS ROG Azoth
2.4GHz dongle (USB ID 0b05:1a85), where the trailing bytes make their
way into hid->uniq and then /sys/class/input/inputN/uniq.

Downstream userspace components then reject the device. systemd's
sd-device property_is_valid() treats any string property containing
control characters as invalid and refuses to set ID_SERIAL_SHORT,
which in turn prevents the device from being tagged with seat. On
GNOME Wayland, mutter silently declines to open input devices that
are missing this tagging, leaving the keyboard visible and producing
keycodes at the kernel layer but dead to the user in a graphical
session.

Truncate the returned UTF-8 string at the first C0 control character
(0x00..0x1F) or DEL (0x7F). Printable Unicode beyond ASCII is left
intact, so legitimate non-ASCII serials (e.g. European manufacturers)
continue to work. Callers that previously received a string with
trailing garbage now receive the clean leading portion, which is
well-formed UTF-8 and safe for all downstream consumers.

Signed-off-by: Taylor Hewetson <taylor@exponent.digital>
---

Changes since v1:
 - Move the sanitization from drivers/hid/usbhid/hid-core.c to
   drivers/usb/core/message.c so that all usb_string() callers
   benefit, not just usbhid. (Greg KH)
 - Broaden the scope from "ASUS Azoth workaround" to "well-formed
   string guarantee for usb_string()"; update commit message
   accordingly.

v1: https://lore.kernel.org/all/20260418025823.21767-1-taylor@exponent.digital/

--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1052,6 +1052,25 @@
 			UTF16_LITTLE_ENDIAN, buf, size);
 	buf[err] = 0;
 
+	/*
+	 * Some devices report string descriptors with a declared length
+	 * greater than the actual serial, leaving uninitialized firmware
+	 * memory (often including C0 control characters) appended to the
+	 * returned string. Truncate at the first control character so
+	 * callers get a clean, well-formed string.
+	 */
+	{
+		int i;
+		for (i = 0; i < err; i++) {
+			unsigned char c = buf[i];
+			if (c < 0x20 || c == 0x7f) {
+				buf[i] = 0;
+				err = i;
+				break;
+			}
+		}
+	}
+
 	if (tbuf[1] != USB_DT_STRING)
 		dev_dbg(&dev->dev,
 			"wrong descriptor type %02x for string %d (\"%s\")\n",