Re: [PATCH] HID: bpf: Fix signedness bug in hid_bpf_hw_request

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Integer overflow in hid_bpf_get_data on 32-bit platforms leads to out-of-bounds pointer return.
- [Critical] Missing bounds check on `size` in `dispatch_hid_bpf_device_event` leads to a kernel heap buffer overflow.
- [High] Dereference of ZERO_SIZE_PTR when processing a 0-length HID report in hid_bpf_hw_request.
--

commit 8a657f92f1d0d08c3cf3bdf8cdaf0b05c7deec72
Author: Guangshuo Li <lgs201920130244@gmail.com>
Subject: HID: bpf: Fix signedness bug in hid_bpf_hw_request

This patch clamps positive return values from hid_hw_raw_request() to the
size of the caller-supplied buffer. It preserves negative error codes while
preventing oversized successful returns from overflowing the buffer.

> diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
> index d0130658091b0..b13f911e5944a 100644
> --- a/drivers/hid/bpf/hid_bpf_dispatch.c
> +++ b/drivers/hid/bpf/hid_bpf_dispatch.c
> @@ -446,7 +446,7 @@ hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz,
>  					      (u64)(long)ctx,
>  					      true); /* prevent infinite recursions */
>  

[Severity: High]
This is a pre-existing issue, but does this code safely handle 0-length HID
reports?

In __hid_bpf_hw_check_params(), the size can be capped to 0. This causes the
subsequent kmemdup() to return ZERO_SIZE_PTR. The dma_data pointer is then
unconditionally dereferenced in hid_hw_raw_request() when accessing
dma_data[0], which could cause a crash.

> -	if (ret > size)
> +	if (ret > 0 && ret > size)
>  		ret = size;
>  	if (ret > 0)
>  		memcpy(buf, dma_data, ret);

[Severity: Critical]
This is a pre-existing issue, but can the bounds check in hid_bpf_get_data()
overflow on 32-bit platforms?

drivers/hid/bpf/hid_bpf_dispatch.c:hid_bpf_get_data() {
    [ ... ]
    if (rdwr_buf_size + offset > ctx->allocated_size)
        return NULL;

    return ctx_kern->data + offset;
}

The addition of rdwr_buf_size and offset could wrap around, bypassing the
bounds check and potentially returning an out-of-bounds pointer.

[Severity: Critical]
This is a pre-existing issue, but is there a missing bounds check on size in
dispatch_hid_bpf_device_event()?

drivers/hid/bpf/hid_bpf_dispatch.c:dispatch_hid_bpf_device_event() {
    [ ... ]
    memset(ctx_kern.data, 0, hdev->bpf.allocated_data);
    memcpy(ctx_kern.data, data, *size);
    [ ... ]
}

The memcpy() uses the provided size without validating it against
hdev->bpf.allocated_data. This could lead to a heap buffer overflow if a
larger size is provided, such as through a UHID_INPUT event.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260704160703.156298-1-lgs201920130244@gmail.com?part=1