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

["Emil Tsalapatis" <emil@etsalapatis.com>]

On Sat Jul 4, 2026 at 12:07 PM EDT, Guangshuo Li wrote:
> 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;

I think this is making the code more difficult to write. Why not add a
label right before the cleanup at the end of the function, then
jump to it on error like so:

	if (ret < 0)
		goto done;

	if (ret > size)
		ret = size;

	if (ret > 0)
		memcpy(...);

done:
	kfree(dma_data);
	return ret;


>  	if (ret > 0)
>  		memcpy(buf, dma_data, ret);