linux-kernel - Re: [PATCH] HID: validate report length and constants

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 for Android: free password hash cracker in your pocket

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

Message-ID: <dob7q77qxuv3rmr4kliqp5kic36updvh6qxj4ld2be353zi7ba@5qte5m5fsuwy>
Date: Thu, 4 Dec 2025 10:53:31 +0100
From: Benjamin Tissoires <bentiss@...nel.org>
To: Davide Beatrici <me@...idebeatrici.dev>
Cc: Terry Junge <linuxhid@...micgizmosystems.com>, 
	linux-kernel@...r.kernel.org, linux-input@...r.kernel.org, jikos@...nel.org, 
	benjamin.tissoires@...hat.com
Subject: Re: [PATCH] HID: validate report length and constants

On Dec 02 2025, Davide Beatrici wrote:
> > Can you supply the Device, Configuration, and Report Descriptors?
> 
> Sure.
> 
> Device Descriptor:
>   idVendor           0x373b Compx
>   idProduct          0x1107 ATK X1 SE Nearlink
>   bcdDevice          1.21
>   bcdUSB             2.00
>   bMaxPacketSize0    64
>   iManufacturer      1 Compx
>   iProduct           2 ATK X1 SE Nearlink
>   bNumConfigurations 1
> 
> Configuration Descriptor:
>   wTotalLength       0x0054
>   bNumInterfaces     3
>   bmAttributes       0xa0 (Bus Powered, Remote Wakeup)
>   MaxPower           494mA
> 
> Interface 0: HID Keyboard
>   HID Descriptor: wDescriptorLength 77
>   Endpoint IN 0x81, Interrupt, 64 bytes
> 
> Interface 1: HID (non‑boot)
>   HID Descriptor: wDescriptorLength 156
>   Endpoint IN 0x82, Interrupt, 64 bytes
> 
> Interface 2: HID Mouse
>   HID Descriptor: wDescriptorLength 87
>   Endpoint IN 0x83, Interrupt, 64 bytes
> 
> Report Descriptors:
> 
> Interface 2 (Mouse):
>   05 01 09 02 A1 01 09 01 A1 00 05 09 19 01 29 05
>   15 00 25 01 95 05 75 01 81 02 95 01 75 03 81 01
>   05 01 09 30 09 31 16 00 80 26 FF 7F 75 10 95 02
>   81 06 C0 A1 00 05 01 09 38 15 81 25 7F 75 08 95
>   01 81 06 C0 A1 00 05 0C 0A 38 02 95 01 75 08 15
>   81 25 7F 81 06 C0 C0
> 
> Interface 1 (HID composite):
>   05 0C 09 01 A1 01 85 05 15 00 26 14 05 19 00 2A
>   14 05 75 10 95 01 81 00 C0 05 01 09 80 A1 01 85
>   03 19 81 29 83 15 00 25 01 95 03 75 01 81 02 95
>   01 75 05 81 01 C0 05 01 09 06 A1 01 85 04 05 07
>   15 00 25 01 19 00 29 9F 95 A0 75 01 81 02 C0 06
>   02 FF 09 02 A1 01 85 13 15 00 26 FF 00 75 08 95
>   13 09 02 81 00 09 02 91 00 C0 06 02 FF 09 02 A1
>   01 85 08 15 00 26 FF 00 75 08 95 10 09 02 81 00
>   09 02 91 00 C0 06 04 FF 09 02 A1 01 85 06 09 02
>   15 00 26 FF 00 75 08 95 07 B1 02 C0
> 
> Interface 0 (Keyboard):
>   05 01 09 06 A1 01 05 08 19 01 29 03 15 00 25 01
>   75 01 95 03 91 02 95 05 91 01 05 07 19 E0 29 E7
>   15 00 25 01 75 01 95 08 81 02 75 08 95 01 81 01
>   05 07 19 00 2A FF 00 15 00 26 FF 00 75 08 95 05
>   81 00 05 FF 09 03 75 08 95 01 81 02 C0

Thanks for the logs.

So after analysing the wireshark capture and these, the problem is that
the device sends a USB report of length 1 on the keyboard interface when
we should actually get one of size 8.

However, the device also shows an output report of size 1, but it is not
supposed to send it as an input report. I wonder if the firmware bug is
not that it tries to give the host the current state of its output
report at plug (which is wrong but Windows must be papering over it).

Anyway couple of observations:
- the URB is of size 1, so the fact that the constant field is not 0
	means that we are just reading random memory at offset 1 in the
	provided data, so you might have a chance that it eventually becomes 0

- the fix should be focusing on the length of the provided report, not
	on the content. However, in hid_report_raw_event(), just before you
	inserted your call to your hid_validate_report(), there is already a
	check on the length of the report which memsets to 0 the rest of the
	buffer. This seems a little bit optimistic if the provided buffer from
	USB is exactly the size of the provided "size" argument.
	But then, why would you get random data in the const fields if there
	is a memset if the provided length is "1"?

So, can you add a printk before your call to hid_validate_report() to
show the provided "size" argument (csize), or just enable the hid_dbg()
trace output which should tell us if we enter that test and do the
memset (which I suppose we are not).

Cheers,
Benjamin