Skip to contentSkip to conversation

HID: bpf: fix __hid_bpf_hw_check_params report length computation

PATCH Applied Benjamin Tissoires 2 participants 9 messages Sep 4 – Sep 14

Subsystems (2)
Kselftest HID
lore
Share & tools
lore

Benjamin Tissoires

author maintainer
#0 lore raw

cover HID: bpf: fix __hid_bpf_hw_check_params report length computation

While playing with hid_bpf_hw_output_report() on a device without report IDs, I realized I wasn't able to send proper reports to the USB endpoint.

This is because the __hid_bpf_hw_check_params() check wasn't taking into account the report ID reserved space, and it was trimming down one element in the report.

Of course, things are not simple, because sending an input report is not taking this extra space into account, so we need to account for the type of request as well.

Cheers,
Benjamin

Signed-off-by Benjamin Tissoires <[email protected]>

---
Benjamin Tissoires (3):
selftests/hid: add define for commonly used buf size
HID: bpf: fix __hid_bpf_hw_check_params report length selftests/hid: add unnumbered variant to the hid_bpf tests

 drivers/hid/bpf/hid_bpf_dispatch.c       | 12 +++--
 tools/testing/selftests/hid/hid_bpf.c    | 91 ++++++++++++++++++++++----------
 tools/testing/selftests/hid/hid_common.h | 25 ++++++++-
 tools/testing/selftests/hid/progs/hid.c  |  2 +-
 4 files changed, 96 insertions(+), 34 deletions(-)

---
base-commit: a93f3bf4e1d60777b1659b812c9e818cfc53b449
change-id: 20260904-wip-bpf-check_report-f490de16b864

Best regards,

Signature · 1 line
Benjamin Tissoires <[email protected]>
author maintainer
#1 lore raw

PATCH 1/3 selftests/hid: add define for commonly used buf size

If we want to add another report descriptor without report IDs with a report size bigger than 10, we have multiple magic values to replace.

Put a #define once and for all, so we don't have dangling ones.

Signed-off-by Benjamin Tissoires <[email protected]>
---
 tools/testing/selftests/hid/hid_bpf.c    | 38 ++++++++++++++++----------------
 tools/testing/selftests/hid/hid_common.h |  3 ++-
 2 files changed, 21 insertions(+), 20 deletions(-)
tools/testing/selftests/hid/hid_bpf.c +19 -19
index b851339308c2..e865ee55b4f1 100644
@@ -5,7 +5,7 @@
 #include <bpf/bpf.h>
 
 struct hid_hw_request_syscall_args {
-	__u8 data[10];
+	__u8 data[MAX_BUF_SIZE];
 	unsigned int hid;
 	int retval;
 	size_t size;

@@ -168,7 +168,7 @@ TEST_F(hid_bpf, raw_event)
 	const struct test_program progs[] = {
 		{ .name = "hid_first_event" },
 	};
-	__u8 buf[10] = {0};
+	__u8 buf[MAX_BUF_SIZE] = {0};
 	int err;
 
 	LOAD_PROGRAMS(progs);

@@ -219,7 +219,7 @@ TEST_F(hid_bpf, subprog_raw_event)
 	const struct test_program progs[] = {
 		{ .name = "hid_subprog_first_event" },
 	};
-	__u8 buf[10] = {0};
+	__u8 buf[MAX_BUF_SIZE] = {0};
 	int err;
 
 	LOAD_PROGRAMS(progs);

@@ -277,7 +277,7 @@ TEST_F(hid_bpf, test_attach_detach)
 		{ .name = "hid_second_event" },
 	};
 	struct bpf_link *link;
-	__u8 buf[10] = {0};
+	__u8 buf[MAX_BUF_SIZE] = {0};
 	int err, link_fd;
 
 	LOAD_PROGRAMS(progs);

@@ -362,7 +362,7 @@ TEST_F(hid_bpf, test_hid_change_report)
 	const struct test_program progs[] = {
 		{ .name = "hid_change_report_id" },
 	};
-	__u8 buf[10] = {0};
+	__u8 buf[MAX_BUF_SIZE] = {0};
 	int err;
 
 	LOAD_PROGRAMS(progs);

@@ -389,13 +389,13 @@ TEST_F(hid_bpf, test_hid_user_input_report_call)
 {
 	struct hid_hw_request_syscall_args args = {
 		.retval = -1,
-		.size = 10,
+		.size = MAX_BUF_SIZE,
 	};
 	DECLARE_LIBBPF_OPTS(bpf_test_run_opts, tattrs,
 			    .ctx_in = &args,
 			    .ctx_size_in = sizeof(args),
 	);
-	__u8 buf[10] = {0};
+	__u8 buf[MAX_BUF_SIZE] = {0};
 	int err, prog_fd;
 
 	LOAD_BPF;

@@ -435,7 +435,7 @@ TEST_F(hid_bpf, test_hid_user_output_report_call)
 {
 	struct hid_hw_request_syscall_args args = {
 		.retval = -1,
-		.size = 10,
+		.size = MAX_BUF_SIZE,
 	};
 	DECLARE_LIBBPF_OPTS(bpf_test_run_opts, tattrs,
 			    .ctx_in = &args,

@@ -484,7 +484,7 @@ TEST_F(hid_bpf, test_hid_user_raw_request_call)
 		.retval = -1,
 		.type = HID_FEATURE_REPORT,
 		.request_type = HID_REQ_GET_REPORT,
-		.size = 10,
+		.size = MAX_BUF_SIZE,
 	};
 	DECLARE_LIBBPF_OPTS(bpf_test_run_opts, tattrs,
 			    .ctx_in = &args,

@@ -517,7 +517,7 @@ TEST_F(hid_bpf, test_hid_filter_raw_request_call)
 	const struct test_program progs[] = {
 		{ .name = "hid_test_filter_raw_request" },
 	};
-	__u8 buf[10] = {0};
+	__u8 buf[MAX_BUF_SIZE] = {0};
 	int err;
 
 	LOAD_PROGRAMS(progs);

@@ -570,7 +570,7 @@ TEST_F(hid_bpf, test_hid_change_raw_request_call)
 	const struct test_program progs[] = {
 		{ .name = "hid_test_hidraw_raw_request" },
 	};
-	__u8 buf[10] = {0};
+	__u8 buf[MAX_BUF_SIZE] = {0};
 	int err;
 
 	LOAD_PROGRAMS(progs);

@@ -596,7 +596,7 @@ TEST_F(hid_bpf, test_hid_infinite_loop_raw_request_call)
 	const struct test_program progs[] = {
 		{ .name = "hid_test_infinite_loop_raw_request" },
 	};
-	__u8 buf[10] = {0};
+	__u8 buf[MAX_BUF_SIZE] = {0};
 	int err;
 
 	LOAD_PROGRAMS(progs);

@@ -619,7 +619,7 @@ TEST_F(hid_bpf, test_hid_filter_output_report_call)
 	const struct test_program progs[] = {
 		{ .name = "hid_test_filter_output_report" },
 	};
-	__u8 buf[10] = {0};
+	__u8 buf[MAX_BUF_SIZE] = {0};
 	int err;
 
 	LOAD_PROGRAMS(progs);

@@ -672,7 +672,7 @@ TEST_F(hid_bpf, test_hid_change_output_report_call)
 	const struct test_program progs[] = {
 		{ .name = "hid_test_hidraw_output_report" },
 	};
-	__u8 buf[10] = {0};
+	__u8 buf[MAX_BUF_SIZE] = {0};
 	int err;
 
 	LOAD_PROGRAMS(progs);

@@ -696,7 +696,7 @@ TEST_F(hid_bpf, test_hid_infinite_loop_output_report_call)
 	const struct test_program progs[] = {
 		{ .name = "hid_test_infinite_loop_output_report" },
 	};
-	__u8 buf[10] = {0};
+	__u8 buf[MAX_BUF_SIZE] = {0};
 	int err;
 
 	LOAD_PROGRAMS(progs);

@@ -722,7 +722,7 @@ TEST_F(hid_bpf, test_multiply_events_wq)
 	const struct test_program progs[] = {
 		{ .name = "hid_test_multiply_events_wq" },
 	};
-	__u8 buf[10] = {0};
+	__u8 buf[MAX_BUF_SIZE] = {0};
 	int err;
 
 	LOAD_PROGRAMS(progs);

@@ -760,7 +760,7 @@ TEST_F(hid_bpf, test_multiply_events)
 	const struct test_program progs[] = {
 		{ .name = "hid_test_multiply_events" },
 	};
-	__u8 buf[10] = {0};
+	__u8 buf[MAX_BUF_SIZE] = {0};
 	int err;
 
 	LOAD_PROGRAMS(progs);

@@ -794,7 +794,7 @@ TEST_F(hid_bpf, test_hid_infinite_loop_input_report_call)
 	const struct test_program progs[] = {
 		{ .name = "hid_test_infinite_loop_input_report" },
 	};
-	__u8 buf[10] = {0};
+	__u8 buf[MAX_BUF_SIZE] = {0};
 	int err;
 
 	LOAD_PROGRAMS(progs);

@@ -848,7 +848,7 @@ TEST_F(hid_bpf, test_hid_attach_flags)
 			.insert_head = 0,
 		},
 	};
-	__u8 buf[10] = {0};
+	__u8 buf[MAX_BUF_SIZE] = {0};
 	int err;
 
 	LOAD_PROGRAMS(progs);
tools/testing/selftests/hid/hid_common.h +2 -1
index e3b267446fa0..4567336f131d 100644
@@ -13,6 +13,7 @@
 #include <linux/uhid.h>
 
 #define SHOW_UHID_DEBUG 0
+#define MAX_BUF_SIZE 10
 
 #define min(a, b) \
 	({ __typeof__(a) _a = (a); \

@@ -110,7 +111,7 @@ static pthread_cond_t uhid_started = PTHREAD_COND_INITIALIZER;
 
 static pthread_mutex_t uhid_output_mtx = PTHREAD_MUTEX_INITIALIZER;
 static pthread_cond_t uhid_output_cond = PTHREAD_COND_INITIALIZER;
-static unsigned char output_report[10];
+static unsigned char output_report[MAX_BUF_SIZE];
 
 /* no need to protect uhid_stopped, only one thread accesses it */
 static bool uhid_stopped;
Signature · 1 line
2.55.0
author maintainer
#2 lore raw

PATCH 2/3 HID: bpf: fix __hid_bpf_hw_check_params report length

In reply to Benjamin Tissoires

Turns out that USB, I2C and other transport drivers (except uhid which just passes the data) still need to have the report ID in the first byte.

Because they expect the first byte to be the report ID or 0, when the report ID is 0, they strip that first byte before forwarding to the device. This means that the transport layer forwards a buffer of size N-1 to the device, which gets rejected.

Fixes 5599f8019661 ("HID: bpf: export hid_hw_output_report as a BPF kfunc")
Signed-off-by Benjamin Tissoires <[email protected]>
---
 drivers/hid/bpf/hid_bpf_dispatch.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
drivers/hid/bpf/hid_bpf_dispatch.c +8 -4
index 536f6d01fd14..b1de1dd0f21d 100644
@@ -359,7 +359,7 @@ hid_bpf_release_context(struct hid_bpf_ctx *ctx)
 
 static int
 __hid_bpf_hw_check_params(struct hid_bpf_ctx *ctx, __u8 *buf, size_t *buf__sz,
-			  enum hid_report_type rtype)
+			  enum hid_report_type rtype, bool hw_request)
 {
 	struct hid_report_enum *report_enum;
 	struct hid_report *report;

@@ -388,6 +388,10 @@ __hid_bpf_hw_check_params(struct hid_bpf_ctx *ctx, __u8 *buf, size_t *buf__sz,
 
 	report_len = hid_report_len(report);
 
+	/* unnumbered reports need to have a report ID reserved in the first byte */
+	if (hw_request && report_enum->numbered == 0)
+		report_len += 1;
+
 	if (*buf__sz > report_len)
 		*buf__sz = report_len;
 

@@ -420,7 +424,7 @@ hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz,
 		return -EDEADLOCK;
 
 	/* check arguments */
-	ret = __hid_bpf_hw_check_params(ctx, buf, &size, rtype);
+	ret = __hid_bpf_hw_check_params(ctx, buf, &size, rtype, true);
 	if (ret)
 		return ret;
 

@@ -480,7 +484,7 @@ hid_bpf_hw_output_report(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz)
 		return -EDEADLOCK;
 
 	/* check arguments */
-	ret = __hid_bpf_hw_check_params(ctx, buf, &size, HID_OUTPUT_REPORT);
+	ret = __hid_bpf_hw_check_params(ctx, buf, &size, HID_OUTPUT_REPORT, true);
 	if (ret)
 		return ret;
 

@@ -506,7 +510,7 @@ __hid_bpf_input_report(struct hid_bpf_ctx *ctx, enum hid_report_type type, u8 *b
 		return -EDEADLOCK;
 
 	/* check arguments */
-	ret = __hid_bpf_hw_check_params(ctx, buf, &size, type);
+	ret = __hid_bpf_hw_check_params(ctx, buf, &size, type, false);
 	if (ret)
 		return ret;
Signature · 1 line
2.55.0
AI review
#3 lore raw

Re: [PATCH 2/3] HID: bpf: fix __hid_bpf_hw_check_params report length

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] The logic to reserve an extra byte for the report ID incorrectly checks report_enum->numbered == 0 instead of report->id == 0, leading to buffer truncation for unnumbered reports on devices that also declare numbered reports.

Pre-existing issues:
- [High] An integer sign mismatch in hid_bpf_hw_request causes negative error codes returned by the transport driver to be cast to size_t and evaluated as large positive values, incorrectly converting hardware errors into a 'success' return value. --

commit 2abe2f741986f5f3130a3dadda6bf82df9cbea21
Author: Benjamin Tissoires <[email protected]>

HID: bpf: fix __hid_bpf_hw_check_params report length

This patch modifies __hid_bpf_hw_check_params to reserve an extra byte for the report ID during hardware requests on unnumbered reports. This ensures transport drivers do not strip the first byte of the payload and reject the request due to insufficient buffer size.

Quoted 4 lines
drivers/hid/bpf/hid_bpf_dispatch.c +0 -0
index 536f6d01fd14c..b1de1dd0f21d0 100644

[ ... ]

drivers/hid/bpf/hid_bpf_dispatch.c __hid_bpf_hw_check_params(struct hid_bpf_ctx *ctx, __u8 *buf, size_t *buf__sz, · line 394

Quoted 8 lines
drivers/hid/bpf/hid_bpf_dispatch.c +4 -0
@@ -388,6 +388,10 @@ __hid_bpf_hw_check_params(struct hid_bpf_ctx *ctx, __u8 *buf, size_t *buf__sz,
 
 	report_len = hid_report_len(report);
 
+	/* unnumbered reports need to have a report ID reserved in the first byte */
+	if (hw_request && report_enum->numbered == 0)
+		report_len += 1;
+

[Severity: Medium]
Does this correctly handle devices that declare both unnumbered and numbered reports?

If a non-compliant HID device declares an unnumbered report where the report ID is 0, but also declares other numbered reports, report_enum->numbered will be set to 1.

When a BPF program attempts a hardware request for the unnumbered report on such a device, the report_enum->numbered == 0 check will evaluate to false and skip the +1 padding. The BPF-provided buffer size is then clamped, but the transport driver will still strip the first byte before transmission, leading to buffer truncation.

Would it be safer to check if report->id == 0 instead?

Quoted 3 lines
drivers/hid/bpf/hid_bpf_dispatch.c +0 -0
 	if (*buf__sz > report_len)
 		*buf__sz = report_len;

[ ... ]

drivers/hid/bpf/hid_bpf_dispatch.c hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz, · line 430

Quoted 9 lines
drivers/hid/bpf/hid_bpf_dispatch.c +1 -1
@@ -420,7 +424,7 @@ hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz,
 		return -EDEADLOCK;
 
 	/* check arguments */
-	ret = __hid_bpf_hw_check_params(ctx, buf, &size, rtype);
+	ret = __hid_bpf_hw_check_params(ctx, buf, &size, rtype, true);
 	if (ret)
 		return ret;

[Severity: High]
This is a pre-existing issue, but I noticed a potential integer sign mismatch in hid_bpf_hw_request involving the size variable passed here.

In hid_bpf_hw_request, size is defined as a size_t, while ret is an int. Later in the function, after hid_hw_raw_request returns its status into ret, the code checks:

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

If a transport driver encounters a hardware error and returns a negative error code such as -EPIPE, ret is implicitly promoted to size_t when compared against size. This causes the negative error code to evaluate as a large positive number, making the (ret > size) condition true.

This silently converts the hardware error into a success return value and copies the unmodified buffer back, which could leave BPF programs operating on uninitialized or stale data.

Can this be addressed by casting size to an ssize_t during the comparison, or by handling negative values in ret before the comparison?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
author maintainer
#4 lore raw

Re: [PATCH 2/3] HID: bpf: fix __hid_bpf_hw_check_params report length

Quoted 37 lines · On Sep 04 2026, [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] The logic to reserve an extra byte for the report ID incorrectly checks report_enum->numbered == 0 instead of report->id == 0, leading to buffer truncation for unnumbered reports on devices that also declare numbered reports.

Pre-existing issues:
- [High] An integer sign mismatch in hid_bpf_hw_request causes negative error codes returned by the transport driver to be cast to size_t and evaluated as large positive values, incorrectly converting hardware errors into a 'success' return value. --

commit 2abe2f741986f5f3130a3dadda6bf82df9cbea21
Author: Benjamin Tissoires <[email protected]>

HID: bpf: fix __hid_bpf_hw_check_params report length

This patch modifies __hid_bpf_hw_check_params to reserve an extra byte for the report ID during hardware requests on unnumbered reports. This ensures transport drivers do not strip the first byte of the payload and reject the request due to insufficient buffer size.

Quoted 4 lines
drivers/hid/bpf/hid_bpf_dispatch.c +0 -0
index 536f6d01fd14c..b1de1dd0f21d0 100644

[ ... ]

drivers/hid/bpf/hid_bpf_dispatch.c __hid_bpf_hw_check_params(struct hid_bpf_ctx *ctx, __u8 *buf, size_t *buf__sz, · line 394

Quoted 8 lines
drivers/hid/bpf/hid_bpf_dispatch.c +4 -0
@@ -388,6 +388,10 @@ __hid_bpf_hw_check_params(struct hid_bpf_ctx *ctx, __u8 *buf, size_t *buf__sz,
 
 	report_len = hid_report_len(report);
 
+	/* unnumbered reports need to have a report ID reserved in the first byte */
+	if (hw_request && report_enum->numbered == 0)
+		report_len += 1;
+

[Severity: Medium]
Does this correctly handle devices that declare both unnumbered and numbered reports?

Quoted 3 lines

If a non-compliant HID device declares an unnumbered report where the report ID is 0, but also declares other numbered reports, report_enum->numbered will be set to 1.

Sashiko is wrong here. If a HID device declares a device with a report ID of 0, the report will be tagged as numbered. The report *will* have space for the report ID, and so there won't be any issues.

Quoted 41 lines

When a BPF program attempts a hardware request for the unnumbered report on such a device, the report_enum->numbered == 0 check will evaluate to false and skip the +1 padding. The BPF-provided buffer size is then clamped, but the transport driver will still strip the first byte before transmission, leading to buffer truncation.

Would it be safer to check if report->id == 0 instead?

Quoted 3 lines
drivers/hid/bpf/hid_bpf_dispatch.c +0 -0
 	if (*buf__sz > report_len)
 		*buf__sz = report_len;

[ ... ]

drivers/hid/bpf/hid_bpf_dispatch.c hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz, · line 430

Quoted 9 lines
drivers/hid/bpf/hid_bpf_dispatch.c +1 -1
@@ -420,7 +424,7 @@ hid_bpf_hw_request(struct hid_bpf_ctx *ctx, __u8 *buf, size_t buf__sz,
 		return -EDEADLOCK;
 
 	/* check arguments */
-	ret = __hid_bpf_hw_check_params(ctx, buf, &size, rtype);
+	ret = __hid_bpf_hw_check_params(ctx, buf, &size, rtype, true);
 	if (ret)
 		return ret;

[Severity: High]
This is a pre-existing issue, but I noticed a potential integer sign mismatch in hid_bpf_hw_request involving the size variable passed here.

In hid_bpf_hw_request, size is defined as a size_t, while ret is an int. Later in the function, after hid_hw_raw_request returns its status into ret, the code checks:

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

If a transport driver encounters a hardware error and returns a negative error code such as -EPIPE, ret is implicitly promoted to size_t when compared against size. This causes the negative error code to evaluate as a large positive number, making the (ret > size) condition true.

Quoted 6 lines

This silently converts the hardware error into a success return value and copies the unmodified buffer back, which could leave BPF programs operating on uninitialized or stale data.

Can this be addressed by casting size to an ssize_t during the comparison, or by handling negative values in ret before the comparison?

Damn, this one seems correct.

Cheers,
Benjamin

Quoted 4 lines
Signature · 2 lines
-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
author maintainer
#5 lore raw

PATCH 3/3 selftests/hid: add unnumbered variant to the hid_bpf tests

In reply to Benjamin Tissoires

A bug appeared in hid_bpf_dispatch.c where it wasn't properly handling unnumbered reports. Add a device variant without report IDs so we can also test them.

Signed-off-by Benjamin Tissoires <[email protected]>
---
 tools/testing/selftests/hid/hid_bpf.c    | 53 ++++++++++++++++++++++++++------
 tools/testing/selftests/hid/hid_common.h | 24 ++++++++++++++-
 tools/testing/selftests/hid/progs/hid.c  |  2 +-
 3 files changed, 68 insertions(+), 11 deletions(-)
tools/testing/selftests/hid/hid_bpf.c +44 -9
index e865ee55b4f1..d1bd07caf991 100644
@@ -54,11 +54,27 @@ FIXTURE_TEARDOWN(hid_bpf) {
 	hid_bpf_teardown(_metadata, self, variant); \
 } while (0)
 
+FIXTURE_VARIANT(hid_bpf) {
+	__u8 *rdesc;
+	size_t rdesc_size;
+};
+
+FIXTURE_VARIANT_ADD(hid_bpf, numbered) {
+	.rdesc = rdesc,
+	.rdesc_size = sizeof(rdesc),
+};
+
+FIXTURE_VARIANT_ADD(hid_bpf, unnumbered) {
+	.rdesc = fido2_rdesc,
+	.rdesc_size = sizeof(fido2_rdesc),
+};
+
 FIXTURE_SETUP(hid_bpf)
 {
 	int err;
 
-	err = setup_uhid(_metadata, &self->hid, BUS_USB, 0x0001, 0x0a36, rdesc, sizeof(rdesc));
+	err = setup_uhid(_metadata, &self->hid, BUS_USB, 0x0001, 0x0a36,
+			 variant->rdesc, variant->rdesc_size);
 	ASSERT_OK(err);
 }
 

@@ -402,8 +418,11 @@ TEST_F(hid_bpf, test_hid_user_input_report_call)
 
 	args.hid = self->hid.hid_id;
 	args.data[0] = 1; /* report ID */
-	args.data[1] = 2; /* report ID */
-	args.data[2] = 42; /* report ID */
+	args.data[1] = 2;
+	args.data[2] = 42;
+
+	if (variant->rdesc == fido2_rdesc)
+		args.data[0] = 0;
 
 	prog_fd = bpf_program__fd(self->skel->progs.hid_user_input_report);
 

@@ -421,8 +440,13 @@ TEST_F(hid_bpf, test_hid_user_input_report_call)
 	/* read the data from hidraw */
 	memset(buf, 0, sizeof(buf));
 	err = read(self->hidraw_fd, buf, sizeof(buf));
-	ASSERT_EQ(err, 6) TH_LOG("read_hidraw");
-	ASSERT_EQ(buf[0], 1);
+	if (variant->rdesc == rdesc) {
+		ASSERT_EQ(err, 6) TH_LOG("read_hidraw");
+	} else {
+		ASSERT_EQ(err, 64)
+		TH_LOG("read_hidraw");
+	}
+	ASSERT_EQ(buf[0], args.data[0]);
 	ASSERT_EQ(buf[1], 2);
 	ASSERT_EQ(buf[2], 42);
 }

@@ -448,8 +472,11 @@ TEST_F(hid_bpf, test_hid_user_output_report_call)
 
 	args.hid = self->hid.hid_id;
 	args.data[0] = 1; /* report ID */
-	args.data[1] = 2; /* report ID */
-	args.data[2] = 42; /* report ID */
+	args.data[1] = 2;
+	args.data[2] = 42;
+
+	if (variant->rdesc == fido2_rdesc)
+		args.data[0] = 0;
 
 	prog_fd = bpf_program__fd(self->skel->progs.hid_user_output_report);
 

@@ -465,9 +492,14 @@ TEST_F(hid_bpf, test_hid_user_output_report_call)
 	ASSERT_OK(err) TH_LOG("error while calling bpf_prog_test_run_opts");
 	ASSERT_OK(cond_err) TH_LOG("error while calling waiting for the condition");
 
-	ASSERT_EQ(args.retval, 3);
+	if (variant->rdesc == rdesc) {
+		ASSERT_EQ(args.retval, 3);
+	} else if (variant->rdesc == fido2_rdesc) {
+		ASSERT_EQ(args.retval, 65)
+		TH_LOG("report size error, should have 64 + 1 extra byte for the report ID 0");
+	}
 
-	ASSERT_EQ(output_report[0], 1);
+	ASSERT_EQ(output_report[0], args.data[0]);
 	ASSERT_EQ(output_report[1], 2);
 	ASSERT_EQ(output_report[2], 42);
 

@@ -879,6 +911,9 @@ TEST_F(hid_bpf, test_rdesc_fixup)
 	};
 	int err, desc_size;
 
+	if (variant->rdesc != rdesc)
+		SKIP(return, "not compatible report descriptor");
+
 	LOAD_PROGRAMS(progs);
 
 	/* check that hid_rdesc_fixup() was executed */
tools/testing/selftests/hid/hid_common.h +23 -1
index 4567336f131d..b7890ba2878f 100644
@@ -13,7 +13,7 @@
 #include <linux/uhid.h>
 
 #define SHOW_UHID_DEBUG 0
-#define MAX_BUF_SIZE 10
+#define MAX_BUF_SIZE 128
 
 #define min(a, b) \
 	({ __typeof__(a) _a = (a); \

@@ -98,6 +98,28 @@ static unsigned char rdesc[] = {
 
 static __u8 feature_data[] = { 1, 2 };
 
+static __maybe_unused unsigned char fido2_rdesc[] = {
+	0x06, 0xd0, 0xf1,	/* Usage Page (FIDO Alliance) */
+	0x09, 0x01,		/* Usage (U2F Authenticator Device) */
+	0xa1, 0x01,		/* Collection (Application) */
+	0x09, 0x20,		/*  Usage (Input Report Data) */
+	0x15, 0x00,		/*  Logical Minimum (0) */
+	0x26, 0xff, 0x00,	/*  Logical Maximum (255) */
+	0x75, 0x08,		/*  Report Size (8) */
+	0x95, 0x40,		/*  Report Count (64) */
+	0x81, 0x02,		/*  Input (Data,Var,Abs) */
+	0x09, 0x21,		/*  Usage (Output Report Data) */
+	0x15, 0x00,		/*  Logical Minimum (0) */
+	0x26, 0xff, 0x00,	/*  Logical Maximum (255) */
+	0x75, 0x08,		/*  Report Size (8) */
+	0x95, 0x40,		/*  Report Count (64) */
+	0x91, 0x02,		/*  Output (Data,Var,Abs) */
+	0x06, 0x00, 0xff,	/*  Usage Page (Vendor Defined Page 1) */
+	0x09, 0x22,		/*  Usage (Vendor Usage 0x22) */
+	0xb1, 0x02,		/*  Feature (Data,Var,Abs) */
+	0xc0,			/* End Collection */
+};
+
 #define ASSERT_OK(data) ASSERT_FALSE(data)
 #define ASSERT_OK_PTR(ptr) ASSERT_NE(NULL, ptr)
tools/testing/selftests/hid/progs/hid.c +1 -1
index b21fbb13c926..605d8a5f9cb2 100644
@@ -98,7 +98,7 @@ struct hid_bpf_ops change_report_id = {
 
 struct hid_hw_request_syscall_args {
 	/* data needs to come at offset 0 so we can use it in calls */
-	__u8 data[10];
+	__u8 data[128];
 	unsigned int hid;
 	int retval;
 	size_t size;
Signature · 1 line
2.55.0
AI review
#6 lore raw

Re: [PATCH 3/3] selftests/hid: add unnumbered variant to the hid_bpf tests

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The newly added unnumbered test variant runs against unadapted tests, leading to test suite…

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added unnumbered test variant runs against unadapted tests, leading to test suite regressions and false passes. --

commit 25b4cfc0df88875655d4c7a83e9318718c0ae97f
Author: Benjamin Tissoires <[email protected]>

selftests/hid: add unnumbered variant to the hid_bpf tests

A bug appeared in hid_bpf_dispatch.c where it wasn't properly handling unnumbered reports. Add a device variant without report IDs so we can also test them.

Quoted 13 lines
tools/testing/selftests/hid/hid_bpf.c +5 -0
index e865ee55b4f1d..d1bd07caf9919 100644
@@ -54,11 +54,27 @@ FIXTURE_TEARDOWN(hid_bpf) {
 	hid_bpf_teardown(_metadata, self, variant); \
 } while (0)
 
+FIXTURE_VARIANT(hid_bpf) {
+	__u8 *rdesc;
+	size_t rdesc_size;
+};
+

tools/testing/selftests/hid/hid_bpf.c FIXTURE_TEARDOWN(hid_bpf) { · line 71

Quoted 10 lines
tools/testing/selftests/hid/hid_bpf.c +10 -0
+FIXTURE_VARIANT_ADD(hid_bpf, numbered) {
+	.rdesc = rdesc,
+	.rdesc_size = sizeof(rdesc),
+};
+
+FIXTURE_VARIANT_ADD(hid_bpf, unnumbered) {
+	.rdesc = fido2_rdesc,
+	.rdesc_size = sizeof(fido2_rdesc),
+};
+

[Severity: Medium]
Will unadapted tests such as raw_event() and test_hid_filter_raw_request_call() fail when run against this new unnumbered variant?

Since FIXTURE_VARIANT_ADD registers this variant for all tests in the hid_bpf fixture, tests that weren't adapted might now fail because they still hardcode assumptions about the 6-byte numbered report. For instance, raw_event() expects exactly 6 bytes:

	err = read(self->hidraw_fd, buf, sizeof(buf));
	ASSERT_EQ(err, 6) TH_LOG("read_hidraw");

test_hid_user_raw_request_call() hardcodes a report ID of 1:

	args.hid = self->hid.hid_id;
	args.data[0] = 1; /* report ID */

And test_hid_filter_raw_request_call() injects a 6-byte event:

	buf[0] = 1;
	buf[1] = 42;
	uhid_send_event(_metadata, &self->hid, buf, 6);

Do these other tests need similar if/else logic for variant->rdesc, or should they explicitly call SKIP() for the unnumbered variant like test_rdesc_fixup() does?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
author maintainer
#7 lore raw

Re: [PATCH 3/3] selftests/hid: add unnumbered variant to the hid_bpf tests

Quoted 33 lines · On Sep 04 2026, [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added unnumbered test variant runs against unadapted tests, leading to test suite regressions and false passes. --

commit 25b4cfc0df88875655d4c7a83e9318718c0ae97f
Author: Benjamin Tissoires <[email protected]>

selftests/hid: add unnumbered variant to the hid_bpf tests

A bug appeared in hid_bpf_dispatch.c where it wasn't properly handling unnumbered reports. Add a device variant without report IDs so we can also test them.

Quoted 20 lines
tools/testing/selftests/hid/hid_bpf.c +12 -0
index e865ee55b4f1d..d1bd07caf9919 100644
@@ -54,11 +54,27 @@ FIXTURE_TEARDOWN(hid_bpf) {
 	hid_bpf_teardown(_metadata, self, variant); \
 } while (0)
 
+FIXTURE_VARIANT(hid_bpf) {
+	__u8 *rdesc;
+	size_t rdesc_size;
+};
+
+FIXTURE_VARIANT_ADD(hid_bpf, numbered) {
+	.rdesc = rdesc,
+	.rdesc_size = sizeof(rdesc),
+};
+
+FIXTURE_VARIANT_ADD(hid_bpf, unnumbered) {
+	.rdesc = fido2_rdesc,
Quoted 8 lines

tools/testing/selftests/hid/hid_bpf.c FIXTURE_TEARDOWN(hid_bpf) { · line 71

Quoted 3 lines
tools/testing/selftests/hid/hid_bpf.c +3 -0
+	.rdesc_size = sizeof(fido2_rdesc),
+};
+

[Severity: Medium]
Will unadapted tests such as raw_event() and test_hid_filter_raw_request_call() fail when run against this new unnumbered variant?

If they would have failed, I wouldn't have sent the series :)

Quoted 6 lines

Since FIXTURE_VARIANT_ADD registers this variant for all tests in the hid_bpf fixture, tests that weren't adapted might now fail because they still hardcode assumptions about the 6-byte numbered report. For instance, raw_event() expects exactly 6 bytes:

Quoted 7 lines
	err = read(self->hidraw_fd, buf, sizeof(buf));
	ASSERT_EQ(err, 6) TH_LOG("read_hidraw");

test_hid_user_raw_request_call() hardcodes a report ID of 1:

	args.hid = self->hid.hid_id;
	args.data[0] = 1; /* report ID */

report ID is a convention, but if there is no report ID, then it's just the first byte.

Quoted 10 lines

And test_hid_filter_raw_request_call() injects a 6-byte event:

	buf[0] = 1;
	buf[1] = 42;
	uhid_send_event(_metadata, &self->hid, buf, 6);

Do these other tests need similar if/else logic for variant->rdesc, or should they explicitly call SKIP() for the unnumbered variant like test_rdesc_fixup() does?

It works, because uhid is lazy enough to not check against the report descriptor and just forwards what goes in. So we are just checking the kernel is not tampering with the data.

Cheers,
Benjamin

Quoted 4 lines
Signature · 2 lines
-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
author maintainer Applied
#8 lore raw
In reply to Benjamin Tissoires
Quoted 9 lines · Benjamin Tissoires

While playing with hid_bpf_hw_output_report() on a device without report IDs, I realized I wasn't able to send proper reports to the USB endpoint.

This is because the __hid_bpf_hw_check_params() check wasn't taking into account the report ID reserved space, and it was trimming down one element in the report.

[...]

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git (for-7.3/upstream-fixes), thanks!

[1/3] selftests/hid: add define for commonly used buf size

      https://git.kernel.org/hid/hid/c/9d1e523b92d8
[2/3] HID: bpf: fix __hid_bpf_hw_check_params report length
      https://git.kernel.org/hid/hid/c/c4afa4862b87
[3/3] selftests/hid: add unnumbered variant to the hid_bpf tests
      https://git.kernel.org/hid/hid/c/b6f69097c827

Cheers,

Signature · 1 line
Benjamin Tissoires <[email protected]>