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

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <o57fhjeipgeb37e2pr6z6qtv3uuon3jsbejj37hgazqxqtklis@c6mumvsempsa>
Date: Fri, 11 Jul 2025 12:52:25 +0200
From: Benjamin Tissoires <bentiss@...nel.org>
To: Jiri Kosina <jikos@...nel.org>, Alan Stern <stern@...land.harvard.edu>, 
	Shuah Khan <shuah@...nel.org>
Cc: linux-input@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-kselftest@...r.kernel.org
Subject: Re: [PATCH v2 4/4] selftests/hid: add a test case for the recent
 syzbot underflow

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@...nel.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
>