On Jul 11 2025, Benjamin Tissoires wrote:
On Jul 10 2025, Benjamin Tissoires wrote:
Syzbot found a buffer underflow in __hid_request(). Add a related test case for it.
It's not perfect, but it allows to catch a corner case when a report descriptor is crafted so that it has a size of 0.
Signed-off-by: Benjamin Tissoires bentiss@kernel.org
tools/testing/selftests/hid/tests/test_mouse.py | 70 +++++++++++++++++++++++++ 1 file changed, 70 insertions(+)
diff --git a/tools/testing/selftests/hid/tests/test_mouse.py b/tools/testing/selftests/hid/tests/test_mouse.py index 66daf7e5975ca50f0b065080669d7f6123fb177f..eb4e15a0e53bd5f3c8e0ea02365ff9da7eead93d 100644 --- a/tools/testing/selftests/hid/tests/test_mouse.py +++ b/tools/testing/selftests/hid/tests/test_mouse.py @@ -439,6 +439,68 @@ class BadResolutionMultiplierMouse(ResolutionMultiplierMouse): return 32 # EPIPE +class BadReportDescriptorMouse(BaseMouse):
- """
- This "device" was one autogenerated by syzbot. There are a lot of issues in
- it, and the most problematic is that it declares features that have no
- size.
- This leads to report->size being set to 0 and can mess up with usbhid
- internals. Fortunately, uhid merely passes the incoming buffer, without
- touching it so a buffer of size 0 will be translated to [] without
- triggering a kernel oops.
- Because the report descriptor is wrong, no input are created, and we need
- to tweak a little bit the parameters to make it look correct.
- """
- # fmt: off
- report_descriptor = [
0x96, 0x01, 0x00, # Report Count (1) 0
0x06, 0x01, 0x00, # Usage Page (Generic Desktop) 3
# 0x03, 0x00, 0x00, 0x00, 0x00, # Ignored by the kernel somehow
0x2a, 0x90, 0xa0, # Usage Maximum (41104) 6
0x27, 0x00, 0x00, 0x00, 0x00, # Logical Maximum (0) 9
0xb3, 0x81, 0x3e, 0x25, 0x03, # Feature (Cnst,Arr,Abs,Vol) 14
0x1b, 0xdd, 0xe8, 0x40, 0x50, # Usage Minimum (1346431197) 19
0x3b, 0x5d, 0x8c, 0x3d, 0xda, # Designator Index 24
- ]
- # fmt: on
- def __init__(
self, rdesc=report_descriptor, name=None, input_info=(3, 0x045E, 0x07DA)
- ):
super().__init__(rdesc, name, input_info)
self.high_resolution_report_called = False
- def get_evdev(self, application=None):
assert self._input_nodes is None
return (
"Ok" # should be a list or None, but both would fail, so abusing the system
)
- def next_sync_events(self, application=None):
# there are no evdev nodes, so no events
return []
- def is_ready(self):
# we wait for the SET_REPORT command to come
return self.high_resolution_report_called
- def set_report(self, req, rnum, rtype, data):
if rtype != self.UHID_FEATURE_REPORT:
raise InvalidHIDCommunication(f"Unexpected report type: {rtype}")
if rnum != 0x0:
raise InvalidHIDCommunication(f"Unexpected report number: {rnum}")
if len(data) != 1:
raise InvalidHIDCommunication(f"Unexpected data: {data}, expected '[0]'")
For the record, while thinking more about this, I realized that I changed the API for uhid with the previous patches.
*But* after second thoughts, every request to a HID device made through hid_hw_request() would see that change, but every request made through hid_hw_raw_request() already has the new behaviour. So that means that the users are already facing situations where they might have or not the first byte being the null report ID when it is 0, so, maybe we are making things more straightforward in the end.
Looking into this more: - uhid is mainly used for BLE devices - uhid is also used for testing, but I don't see that change a big issue - for BLE devices, we can check which kernel module is calling hid_hw_request() - and in those modules, we can check which are using a Bluetooth device - and then we can check if the command is used with a report ID or not.
Doing all the checks above gives me confidence that the only time the report ID 0 is used is when using the resolution multiplier. I don't expect a lot of BLE device without report ID to expose a feature with a high resolution wheel.
But for a more extensive checking: https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/profiles/input/hog-l... in function set_report(), bluez does the same shift if the report ID is 0 and the given buffer has a size > 0.
So I think this patch will also fix a hypothetical BLE device without report ID with high resolution wheel.
Therefore, I'm going to merge this series as it is (and include those blobs in the commit description).
Cheers, Benjamin
self.high_resolution_report_called = True
return 0
class ResolutionMultiplierHWheelMouse(TwoWheelMouse): # fmt: off report_descriptor = [ @@ -975,3 +1037,11 @@ class TestMiMouse(TestWheelMouse): # assert below print out the real error pass assert remaining == []
+class TestBadReportDescriptorMouse(base.BaseTestCase.TestUhid):
- def create_device(self):
return BadReportDescriptorMouse()
- def assertName(self, uhdev):
pass
-- 2.49.0