BPF List
 help / color / mirror / Atom feed
* [PATCH] HID: bpf: add __counted_by_ptr attribute to device_data
@ 2026-09-23  6:31 Bill Wendling
  2026-09-23  6:46 ` sashiko-bot
  2026-09-23  6:56 ` [PATCH v2] " Bill Wendling
  0 siblings, 2 replies; 5+ messages in thread
From: Bill Wendling @ 2026-09-23  6:31 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires
  Cc: Kees Cook, Gustavo A. R. Silva, linux-input, linux-kernel,
	linux-hardening, bpf, Bill Wendling, codemender-patching+linux

The 'struct hid_bpf' contains a 'device_data' pointer field (of type
'u8 *') and an 'allocated_data' field (of type 'u32') that specifies the
size in bytes of the allocated memory for 'device_data'. Since
'device_data' is a pointer to 'u8' (elements of size 1 byte),
'allocated_data' represents the exact count of elements allocated for
'device_data'.

Annotate the 'device_data' field of 'struct hid_bpf' with the
'__counted_by_ptr' attribute, pointing to 'allocated_data'. This enables
bounds-checking sanitizers (like KASAN and UBSAN) to detect
out-of-bounds accesses to 'device_data'.

Because the count 'allocated_data' is always set before any access and
accurately tracks the allocated buffer size at all times, adding
'__counted_by_ptr' will not cause runtime panics or false-positive
bounds checks.

Cc: codemender-patching+linux@google.com
Assisted-by: LLM
Signed-off-by: Bill Wendling <morbo@google.com>
---
 include/linux/hid_bpf.h | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/include/linux/hid_bpf.h b/include/linux/hid_bpf.h
index 19fffa4574a4..f45fb9cccece 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 */
 
-- 
2.55.0.1082.g2b9226bbc0-goog


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] HID: bpf: add __counted_by_ptr attribute to device_data
  2026-09-23  6:31 [PATCH] HID: bpf: add __counted_by_ptr attribute to device_data Bill Wendling
@ 2026-09-23  6:46 ` sashiko-bot
  2026-09-23  6:56   ` Bill Wendling
  2026-09-23  6:56 ` [PATCH v2] " Bill Wendling
  1 sibling, 1 reply; 5+ messages in thread
From: sashiko-bot @ 2026-09-23  6:46 UTC (permalink / raw)
  To: Bill Wendling; +Cc: dmitry.torokhov, linux-input, bpf

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] hid-bpf: false-positive bounds check panic via data race in __hid_bpf_allocate_data()
--

commit 4a962db9c72042e7d956ca752f9439fb64fe5e37
Author: Bill Wendling <morbo@google.com>

HID: bpf: add __counted_by_ptr attribute to device_data

The 'struct hid_bpf' contains a 'device_data' pointer field and an
'allocated_data' field that specifies the size in bytes. This commit
adds the '__counted_by_ptr' attribute to 'device_data' to enable bounds
checking sanitizers.

> 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);

[Severity: High]
Does this newly added attribute introduce a false-positive bounds check panic?

The commit message mentions that allocated_data is always set before any
access, but looking at __hid_bpf_allocate_data() in 
drivers/hid/bpf/hid_bpf_dispatch.c, it appears the pointer is assigned before 
the count without memory barriers:

	*data = alloc_data;
	*size = alloc_size;

Concurrently, if an incoming HID report triggers 
dispatch_hid_bpf_device_event() (which holds driver_input_lock but not 
prog_list_lock), could it observe a non-NULL device_data pointer while the 
allocated_data count still evaluates to 0?

If so, would the new __counted_by_ptr() attribute cause sanitizers to evaluate
the buffer size as 0 while device_data is active, triggering a false-positive
kernel panic when memcpy(ctx_kern.data, data, *size) is called?

>  	u32 allocated_data;
>  	bool destroyed;			/* prevents the assignment of any progs */
>

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2] HID: bpf: add __counted_by_ptr attribute to device_data
  2026-09-23  6:31 [PATCH] HID: bpf: add __counted_by_ptr attribute to device_data Bill Wendling
  2026-09-23  6:46 ` sashiko-bot
@ 2026-09-23  6:56 ` Bill Wendling
  2026-09-23  7:12   ` sashiko-bot
  1 sibling, 1 reply; 5+ messages in thread
From: Bill Wendling @ 2026-09-23  6:56 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires
  Cc: Kees Cook, Gustavo A. R. Silva, linux-input, linux-kernel,
	linux-hardening, bpf, Bill Wendling, codemender-patching+linux

The 'struct hid_bpf' contains a 'device_data' pointer field (of type
'u8 *') and an 'allocated_data' field (of type 'u32') that specifies the
size in bytes of the allocated memory for 'device_data'. Since
'device_data' is a pointer to 'u8' (elements of size 1 byte),
'allocated_data' represents the exact count of elements allocated for
'device_data'.

Annotate the 'device_data' field of 'struct hid_bpf' with the
'__counted_by_ptr' attribute, pointing to 'allocated_data'. This enables
bounds-checking sanitizers (like KASAN and UBSAN) to detect
out-of-bounds accesses to 'device_data'.

Because the count 'allocated_data' is always set before any access and
accurately tracks the allocated buffer size at all times, adding
'__counted_by_ptr' will not cause runtime panics or false-positive
bounds checks.

Cc: codemender-patching+linux@google.com
Assisted-by: LLM
Signed-off-by: Bill Wendling <morbo@google.com>
---
v2: Reorder the assignment of the buffer and the count field. It won't
    generate an exception during execution, but it's a good coding
    habit that also satisfies LLMs' paranoia.
---
 drivers/hid/bpf/hid_bpf_dispatch.c |  2 +-
 include/linux/hid_bpf.h            | 10 ++++++----
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
index d46779b63660..fb302d0b9797 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;
 
 	return 0;
 }
diff --git a/include/linux/hid_bpf.h b/include/linux/hid_bpf.h
index 19fffa4574a4..f45fb9cccece 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 */
 
-- 
2.55.0.1082.g2b9226bbc0-goog


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] HID: bpf: add __counted_by_ptr attribute to device_data
  2026-09-23  6:46 ` sashiko-bot
@ 2026-09-23  6:56   ` Bill Wendling
  0 siblings, 0 replies; 5+ messages in thread
From: Bill Wendling @ 2026-09-23  6:56 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: dmitry.torokhov, linux-input, bpf

On Tue, Sep 22, 2026 at 11:46 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] hid-bpf: false-positive bounds check panic via data race in __hid_bpf_allocate_data()
> --
>
> commit 4a962db9c72042e7d956ca752f9439fb64fe5e37
> Author: Bill Wendling <morbo@google.com>
>
> HID: bpf: add __counted_by_ptr attribute to device_data
>
> The 'struct hid_bpf' contains a 'device_data' pointer field and an
> 'allocated_data' field that specifies the size in bytes. This commit
> adds the '__counted_by_ptr' attribute to 'device_data' to enable bounds
> checking sanitizers.
>
> > 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);
>
> [Severity: High]
> Does this newly added attribute introduce a false-positive bounds check panic?
>
> The commit message mentions that allocated_data is always set before any
> access, but looking at __hid_bpf_allocate_data() in
> drivers/hid/bpf/hid_bpf_dispatch.c, it appears the pointer is assigned before
> the count without memory barriers:
>
>         *data = alloc_data;
>         *size = alloc_size;
>
> Concurrently, if an incoming HID report triggers
> dispatch_hid_bpf_device_event() (which holds driver_input_lock but not
> prog_list_lock), could it observe a non-NULL device_data pointer while the
> allocated_data count still evaluates to 0?
>
> If so, would the new __counted_by_ptr() attribute cause sanitizers to evaluate
> the buffer size as 0 while device_data is active, triggering a false-positive
> kernel panic when memcpy(ctx_kern.data, data, *size) is called?
>
This is a false-positive, though arguably correct way to look at how
initializations of a counted buffer with its count field should be
ordered. It's a false-positive, because assigning the buffer isn't an
access. I'll send a v2.

-bw

> >       u32 allocated_data;
> >       bool destroyed;                 /* prevents the assignment of any progs */
> >
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260923063114.2683575-1-morbo@google.com?part=1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] HID: bpf: add __counted_by_ptr attribute to device_data
  2026-09-23  6:56 ` [PATCH v2] " Bill Wendling
@ 2026-09-23  7:12   ` sashiko-bot
  0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-09-23  7:12 UTC (permalink / raw)
  To: Bill Wendling; +Cc: dmitry.torokhov, bpf, linux-input

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-09-23  7:12 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-23  6:31 [PATCH] HID: bpf: add __counted_by_ptr attribute to device_data Bill Wendling
2026-09-23  6:46 ` sashiko-bot
2026-09-23  6:56   ` Bill Wendling
2026-09-23  6:56 ` [PATCH v2] " Bill Wendling
2026-09-23  7:12   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox