Re: [PATCH 4/9] HID: core: Validate GET_REPORT report ID centrally

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] hid: bpf: HID-BPF cannot fix invalid report IDs due to centralized validation
- [High] hid: __hid_hw_raw_request() breaks uhid unnumbered GET_REPORT
- [Medium] hid: core: un-ratelimited mismatched report ID warning allows log flooding DoS
- [Medium] hid: uninitialized memory read in __hid_hw_raw_request() on timeout
--

commit d5fcafe8cdb215ca55eba80f821711e3f7748edf
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

HID: core: Validate GET_REPORT report ID centrally

This commit centralizes the check for whether a report retrieval response
returns the expected report ID into the core HID raw request path. It also
introduces a new quirk, HID_QUIRK_IGNORE_REPORT_ID_MISMATCH, to handle
hardware with buggy firmware.

> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 3a9bd56dbf85e..cd099a420b9b6 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -2565,8 +2565,17 @@ int __hid_hw_raw_request(struct hid_device *hdev,
>  	if (ret)
>  		return ret;
>  
> -	return hdev->ll_driver->raw_request(hdev, reportnum, buf, len,
> +	ret = hdev->ll_driver->raw_request(hdev, reportnum, buf, len,
>  					    rtype, reqtype);
> +	if (ret > 0 && reqtype == HID_REQ_GET_REPORT &&
> +	    !(hdev->quirks & HID_QUIRK_IGNORE_REPORT_ID_MISMATCH) &&
> +	    buf[0] != reportnum) {

[Severity: High]
Does this validation break uhid unnumbered GET_REPORT requests?

When __hid_hw_raw_request() in drivers/hid/hid-core.c is called for an
unnumbered report (reportnum == 0) on a uhid device, this check expects buf[0]
to contain the report ID (0).

However, the uhid driver copies the userspace response verbatim into the
buffer:

drivers/hid/uhid.c:uhid_hid_get_report() {
    ...
	req = &uhid->report_buf.u.get_report_reply;
	if (req->err) {
		ret = -EIO;
	} else {
		ret = min3(count, (size_t)req->size, (size_t)UHID_DATA_MAX);
		memcpy(buf, req->data, ret);
	}
    ...
}

Because uhid does not prepend a dummy 0x00 report ID like usbhid does, buf[0]
contains the first byte of the data payload. Unless the payload happens to
start with a 0, this check will fail and spuriously reject valid reports.

[Severity: Medium]
Could this read uninitialized memory?

If a userspace application issues a GET_REPORT request via hidraw, it passes
an uninitialized kmalloc buffer down to __hid_hw_raw_request():

drivers/hid/hidraw.c:hidraw_get_report() {
    ...
	buf = kmalloc(count, GFP_KERNEL);
    ...
}

If a transport driver like ishtp_raw_request() in
drivers/hid/intel-ish-hid/ishtp-hid.c times out waiting for the device, but
returns the length without populating the buffer:

drivers/hid/intel-ish-hid/ishtp-hid.c:ishtp_raw_request() {
    ...
	hid_hw_wait(hid);

	return len;
}

This validation check will evaluate the uninitialized memory and leak one
byte of kernel heap memory via the warning.

> +		hid_warn(hdev, "mismatched report ID %u (expected %u)\n",
> +			 buf[0], reportnum);

[Severity: Medium]
Could this warning cause a log flooding denial of service?

Any local user with read/write access to the hidraw node can synchronously
trigger this warning in a tight loop by issuing HIDIOCGFEATURE ioctls on a
device that returns a mismatched report ID.

Because hid_warn() expands to dev_warn() without rate limiting, this can be
exploited to flood the kernel log.

> +		return -EINVAL;

[Severity: High]
Does this prevent HID-BPF programs from fixing invalid report IDs?

When a BPF program calls hid_bpf_hw_request() to fetch a feature report from
the hardware, if the hardware returns an incorrect report ID in buf[0], this
check intercepts the response and immediately returns -EINVAL.

The BPF program receives -EINVAL instead of the raw data buffer, leaving it
unable to read the buggy buffer or apply a fixup, forcing the use of hardcoded
kernel quirks instead.

> +	}
> +
> +	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-hid-core-report-validation-v1-0-e0dd5cb60b2e@gmail.com?part=4