Re: [PATCH v2] HID: bpf: add __counted_by_ptr attribute to device_data

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] hid-bpf: kernel panic in FORTIFY memcpy due to lockless allocation race
--

commit 146c59142d1d9271032a5ce18e55e70b22ecb2ec
Author: Bill Wendling <morbo@google.com>

HID: bpf: add __counted_by_ptr attribute to device_data

This commit adds the __counted_by_ptr attribute to the device_data field in
struct hid_bpf, pointing to allocated_data. It also swaps the assignment order
of size and data in __hid_bpf_allocate_data() to ensure the buffer size is set
before the pointer.

> diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
> index d46779b63660b..fb302d0b97970 100644
> --- a/drivers/hid/bpf/hid_bpf_dispatch.c
> +++ b/drivers/hid/bpf/hid_bpf_dispatch.c
> @@ -255,8 +255,8 @@ static int __hid_bpf_allocate_data(struct hid_device *hdev, u8 **data, u32 *size
>  	if (!alloc_data)
>  		return -ENOMEM;
>  
> -	*data = alloc_data;
>  	*size = alloc_size;
> +	*data = alloc_data;

[Severity: High]
Does this lockless assignment guarantee that the reader will observe
allocated_data being set before device_data?

Without memory barriers like smp_store_release(), the CPU can reorder these
stores. Concurrently, an incoming HID report triggering
dispatch_hid_bpf_device_event() can read these fields locklessly without
smp_load_acquire().

If the reader observes device_data != NULL but allocated_data == 0, can it
lead to a fatal FORTIFY bounds check panic here:

drivers/hid/bpf/hid_bpf_dispatch.c:dispatch_hid_bpf_device_event() {
    ...
    memcpy(ctx_kern.data, data, *size);
    ...
}

Because the __counted_by_ptr annotation statically ties the bounds of
device_data to allocated_data, if allocated_data is 0, ctx_kern.data would
inherit a 0 bound. Calling memcpy() to write into it would then trigger a
kernel panic.

>  
>  	return 0;
>  }
> diff --git a/include/linux/hid_bpf.h b/include/linux/hid_bpf.h
> index 19fffa4574a47..f45fb9cccece8 100644
> --- a/include/linux/hid_bpf.h
> +++ b/include/linux/hid_bpf.h
> @@ -185,10 +185,12 @@ struct hid_bpf_ops {
>  
>  /* stored in each device */
>  struct hid_bpf {
> -	u8 *device_data;		/* allocated when a bpf program of type
> -					 * SEC(f.../hid_bpf_device_event) has been attached
> -					 * to this HID device
> -					 */
> +	/*
> +	 * allocated when a bpf program of type
> +	 * SEC(f.../hid_bpf_device_event) has been attached
> +	 * to this HID device
> +	 */
> +	u8 *device_data __counted_by_ptr(allocated_data);
>  	u32 allocated_data;
>  	bool destroyed;			/* prevents the assignment of any progs */
>

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