Re: [PATCH] HID: uhid: convert to hid_safe_input_report()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] In `dispatch_hid_bpf_device_event()`, there is no bounds check before copying `*size` bytes into `hdev->bpf.device_data`, leading to an out-of-bounds write if the injected report size exceeds the BPF allocated memory.
--

commit c570c0c551ca1ea46eb86fb2144c1ad379f64069
Author: Carlos Llamas <cmllamas@google.com>

HID: uhid: convert to hid_safe_input_report()

This commit converts uhid to use hid_safe_input_report() and passes
UHID_DATA_MAX as the buffer size. This allows hid core to safely zero-pad
shorter reports instead of rejecting them, preventing regressions caused by
the earlier bogus memset() removal.

Closes: https://lore.kernel.org/all/ahsh0UtTX6e0ZeHa@google.com/

> diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
> index 524b53a3c87b3..37b60c3aaf66a 100644
> --- a/drivers/hid/uhid.c
> +++ b/drivers/hid/uhid.c
> @@ -595,8 +595,8 @@ static int uhid_dev_input(struct uhid_device *uhid, struct uhid_event *ev)
>  	if (!READ_ONCE(uhid->running))
>  		return -EINVAL;
>  
> -	hid_input_report(uhid->hid, HID_INPUT_REPORT, ev->u.input.data,
> -			 min_t(size_t, ev->u.input.size, UHID_DATA_MAX), 0);
> +	hid_safe_input_report(uhid->hid, HID_INPUT_REPORT, ev->u.input.data, UHID_DATA_MAX,
> +			      min_t(size_t, ev->u.input.size, UHID_DATA_MAX), 0);

[Severity: Critical]
This is a pre-existing issue, but does forwarding this report size without
constraining it to the device's max_report_len cause a buffer overflow in HID
BPF?

In uhid_dev_input(), we allow userspace to inject input reports up to
UHID_DATA_MAX (4096 bytes) regardless of the device's parsed max_report_len.

When a HID-BPF program is attached to the device, hdev->bpf.device_data is
allocated based on the device's max_report_len, which could be much smaller.

Later, in drivers/hid/bpf/hid_bpf_dispatch.c:dispatch_hid_bpf_device_event(),
it executes:

	memset(ctx_kern.data, 0, hdev->bpf.allocated_data);
	memcpy(ctx_kern.data, data, *size);

Since there is no bounds check on *size before the memcpy(), could a malicious
uhid client injecting a large report size cause a heap buffer overflow when
copying into the smaller BPF allocated memory?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260606181552.3095967-1-cmllamas@google.com?part=1