* [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