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

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

[Guangshuo Li]

hid_bpf_hw_request() clamps the return value of hid_hw_raw_request() to
the size of the caller supplied buffer before copying data back to the
BPF buffer.

However, ret is signed while size is unsigned. If hid_hw_raw_request()
returns a negative error code, the comparison promotes ret to size_t.
This makes the negative value look like a very large positive value, so
the error is clamped to size. The following memcpy() then treats the
failed request as a successful transfer and copies stale data back to
the caller.

Only clamp positive return values. This preserves negative error codes
while still preventing oversized successful returns from overflowing the
caller supplied buffer.

Fixes: 2b658c1c442e ("HID: bpf: prevent buffer overflow in hid_hw_request")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/hid/bpf/hid_bpf_dispatch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
index d0130658091b..b13f911e5944 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 */
 
-	if (ret > size)
+	if (ret > 0 && ret > size)
 		ret = size;
 	if (ret > 0)
 		memcpy(buf, dma_data, ret);
-- 
2.43.0

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

[sashiko-bot]

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