[PATCH v2 0/4] HID: core: fix __hid_request when no report ID is used

[PATCH v2 0/4] HID: core: fix __hid_request when no report ID is used

[Benjamin Tissoires]

New version (unchanged for patches 1-3), with a test added so we can
detect this.

Followup of https://lore.kernel.org/linux-input/c75433e0-9b47-4072-bbe8-b1d14ea97b13@rowland.harvard.edu/

This initial series attempt at fixing the various bugs discovered by
Alan regarding __hid_request().

Syzbot managed to create a report descriptor which presents a feature
request of size 0 (still trying to extract it) and this exposed the fact
that __hid_request() was incorrectly handling the case when the report
ID is not used.

Send a first batch of fixes now so we get the feedback from syzbot ASAP.

Note: in the series, I also mentioned that the report of size 0 should
be stripped out of the HID device, but I'm not entirely sure this would
be a good idea in the end.

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
Changes in v2:
- added Tested-by from syzbot (https://lore.kernel.org/r/686e9113.050a0220.385921.0008.GAE@google.com)
- added a python test for it
- Link to v1: https://lore.kernel.org/r/20250709-report-size-null-v1-0-194912215cbc@kernel.org

---
Benjamin Tissoires (4):
      HID: core: ensure the allocated report buffer can contain the reserved report ID
      HID: core: ensure __hid_request reserves the report ID as the first byte
      HID: core: do not bypass hid_hw_raw_request
      selftests/hid: add a test case for the recent syzbot underflow

 drivers/hid/hid-core.c                          | 19 +++++--
 tools/testing/selftests/hid/tests/test_mouse.py | 70 +++++++++++++++++++++++++
 2 files changed, 84 insertions(+), 5 deletions(-)
---
base-commit: 1f988d0788f50d8464f957e793fab356e2937369
change-id: 20250709-report-size-null-37619ea20288

Best regards,
-- 
Benjamin Tissoires <bentiss@kernel.org>

[PATCH v2 1/4] HID: core: ensure the allocated report buffer can contain the reserved report ID

[Benjamin Tissoires]

When the report ID is not used, the low level transport drivers expect
the first byte to be 0. However, currently the allocated buffer not
account for that extra byte, meaning that instead of having 8 guaranteed
bytes for implement to be working, we only have 7.

Reported-by: Alan Stern <stern@rowland.harvard.edu>
Closes: https://lore.kernel.org/linux-input/c75433e0-9b47-4072-bbe8-b1d14ea97b13@rowland.harvard.edu/
Cc: stable@vger.kernel.org
Suggested-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
 drivers/hid/hid-core.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index b348d0464314ca331da073128f0ec4e0a6a91ed1..1a231dd9e4bc83202f2cbcd8b3a21e8c82b9deec 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1883,9 +1883,12 @@ u8 *hid_alloc_report_buf(struct hid_report *report, gfp_t flags)
 	/*
 	 * 7 extra bytes are necessary to achieve proper functionality
 	 * of implement() working on 8 byte chunks
+	 * 1 extra byte for the report ID if it is null (not used) so
+	 * we can reserve that extra byte in the first position of the buffer
+	 * when sending it to .raw_request()
 	 */
 
-	u32 len = hid_report_len(report) + 7;
+	u32 len = hid_report_len(report) + 7 + (report->id == 0);
 
 	return kzalloc(len, flags);
 }

-- 
2.49.0

[PATCH v2 2/4] HID: core: ensure __hid_request reserves the report ID as the first byte

[Benjamin Tissoires]

The low level transport driver expects the first byte to be the report
ID, even when the report ID is not use (in which case they just shift
the buffer).

However, __hid_request() whas not offsetting the buffer it used by one
in this case, meaning that the raw_request() callback emitted by the
transport driver would be stripped of the first byte.

Reported-by: Alan Stern <stern@rowland.harvard.edu>
Closes: https://lore.kernel.org/linux-input/c75433e0-9b47-4072-bbe8-b1d14ea97b13@rowland.harvard.edu/
Reported-by: syzbot+8258d5439c49d4c35f43@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=8258d5439c49d4c35f43
Tested-by: syzbot+8258d5439c49d4c35f43@syzkaller.appspotmail.com
Fixes: 4fa5a7f76cc7 ("HID: core: implement generic .request()")
Cc: stable@vger.kernel.org
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
 drivers/hid/hid-core.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 1a231dd9e4bc83202f2cbcd8b3a21e8c82b9deec..320887c365f7a36f7376556ffd19f99e52b7d732 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1976,7 +1976,7 @@ static struct hid_report *hid_get_report(struct hid_report_enum *report_enum,
 int __hid_request(struct hid_device *hid, struct hid_report *report,
 		enum hid_class_request reqtype)
 {
-	char *buf;
+	char *buf, *data_buf;
 	int ret;
 	u32 len;
 
@@ -1984,10 +1984,17 @@ int __hid_request(struct hid_device *hid, struct hid_report *report,
 	if (!buf)
 		return -ENOMEM;
 
+	data_buf = buf;
 	len = hid_report_len(report);
 
+	if (report->id == 0) {
+		/* reserve the first byte for the report ID */
+		data_buf++;
+		len++;
+	}
+
 	if (reqtype == HID_REQ_SET_REPORT)
-		hid_output_report(report, buf);
+		hid_output_report(report, data_buf);
 
 	ret = hid->ll_driver->raw_request(hid, report->id, buf, len,
 					  report->type, reqtype);

-- 
2.49.0

[PATCH v2 3/4] HID: core: do not bypass hid_hw_raw_request

[Benjamin Tissoires]

hid_hw_raw_request() is actually useful to ensure the provided buffer
and length are valid. Directly calling in the low level transport driver
function bypassed those checks and allowed invalid paramto be used.

Reported-by: Alan Stern <stern@rowland.harvard.edu>
Closes: https://lore.kernel.org/linux-input/c75433e0-9b47-4072-bbe8-b1d14ea97b13@rowland.harvard.edu/
Cc: stable@vger.kernel.org
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
 drivers/hid/hid-core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 320887c365f7a36f7376556ffd19f99e52b7d732..b31b8a2fd540bd5ed66599020824726e69d10d75 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1996,8 +1996,7 @@ int __hid_request(struct hid_device *hid, struct hid_report *report,
 	if (reqtype == HID_REQ_SET_REPORT)
 		hid_output_report(report, data_buf);
 
-	ret = hid->ll_driver->raw_request(hid, report->id, buf, len,
-					  report->type, reqtype);
+	ret = hid_hw_raw_request(hid, report->id, buf, len, report->type, reqtype);
 	if (ret < 0) {
 		dbg_hid("unable to complete request: %d\n", ret);
 		goto out;

-- 
2.49.0

[PATCH v2 4/4] selftests/hid: add a test case for the recent syzbot underflow

[Benjamin Tissoires]

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]'")
+
+        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

Re: [PATCH v2 4/4] selftests/hid: add a test case for the recent syzbot underflow

[Benjamin Tissoires]

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.

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
> 

Re: [PATCH v2 4/4] selftests/hid: add a test case for the recent syzbot underflow

[Benjamin Tissoires]

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-lib.c#n879
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
> > 

Re: [PATCH v2 0/4] HID: core: fix __hid_request when no report ID is used

[Benjamin Tissoires]

On Thu, 10 Jul 2025 16:01:32 +0200, Benjamin Tissoires wrote:
> New version (unchanged for patches 1-3), with a test added so we can
> detect this.
> 
> Followup of https://lore.kernel.org/linux-input/c75433e0-9b47-4072-bbe8-b1d14ea97b13@rowland.harvard.edu/
> 
> This initial series attempt at fixing the various bugs discovered by
> Alan regarding __hid_request().
> 
> [...]

Applied to hid/hid.git (for-6.16/upstream-fixes), thanks!

[1/4] HID: core: ensure the allocated report buffer can contain the reserved report ID
      https://git.kernel.org/hid/hid/c/4f15ee98304b
[2/4] HID: core: ensure __hid_request reserves the report ID as the first byte
      https://git.kernel.org/hid/hid/c/0d0777ccaa2d
[3/4] HID: core: do not bypass hid_hw_raw_request
      https://git.kernel.org/hid/hid/c/c2ca42f190b6
[4/4] selftests/hid: add a test case for the recent syzbot underflow
      https://git.kernel.org/hid/hid/c/3a1d22bd8538

Cheers,
-- 
Benjamin Tissoires <bentiss@kernel.org>