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

[Benjamin Tissoires <bentiss@kernel.org>]

On Sep 04 2026, sashiko-bot@kernel.org 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 <bentiss@kernel.org>
> 
> 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.
> 
> > diff --git a/tools/testing/selftests/hid/hid_bpf.c b/tools/testing/selftests/hid/hid_bpf.c
> > index e865ee55b4f1d..d1bd07caf9919 100644
> > --- a/tools/testing/selftests/hid/hid_bpf.c
> > +++ b/tools/testing/selftests/hid/hid_bpf.c
> > @@ -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),
> > +};
> > +
> 
> [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 :)

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

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

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

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260904-wip-bpf-check_report-v1-0-efe9a2a1ed28@kernel.org?part=3
>