[PATCH HID v2 00/11] HID: bpf: add a new hook to control hid-generic

[PATCH HID v2 00/11] HID: bpf: add a new hook to control hid-generic

[Benjamin Tissoires]

This is a slight change from the fundamentals of HID-BPF.
In theory, HID-BPF is abstract to the kernel itself, and makes
only changes at the HID level (through report descriptors or
events emitted to/from the device).

However, we have seen a few use cases where HID-BPF might interact with
the running kernel when the target device is already handled by a
specific device.

For example, the XP-Pen/Huion/UC-Logic tablets are handled by
hid-uclogic but this driver is also doing a report descriptor fixup
without checking if the device has already been fixed by HID-BPF.

In the same way, another recent example[0] was when a cheap foot pedal is
used and tricks iPhones and Windows machines by presenting itself as a
known Apple wireless keyboard. The problem is that this fake keyboard is
not presenting a compatible report descriptor and hid-core merges all
device nodes together making libinput ignore the keyboard part for
historical reasons.

Last, there has been a long standing request to allow to disable the
input part of a given gamepad while SDL or Steam opens the device
through hidraw.

This series aims at tackling both of these problems:
- first we had a new hook `hid_bpf_driver_probe` which allows the BPF
  program to decide if the curently probed driver should be used or not
- then this same hook can also change the ->driver_data of the struct
  hid_device_id argument, and we teach hid-generic to use that field as
  the connect mask.

Basically, it means that when we insert a BPF program to fix a device,
we can force hid-generic to handle the device, and thus preventing
any other kernel driver to tamper with our device. We can also
selectively decide to export the hidraw or input nodes when using
hid-generic.

In the SDL/Steam use case, this would means that the gaming application
will load one BPF program per input device it wants to open through
hidraw, that BPF program reassigns the input device to hid-generic and
disables hid-input, then it can open the new hidraw node.
Once that program terminates, the BPF program is removed (either
automatically because no-one has the fd of the links open, or manually
by SDL/Steam), and the normal driver rebinds to the HID device,
restoring full input functionality.

This branch is on top of the for-6.12/hidraw and for-6.12/constify-rdesc
branches of hid.git, mainly because those branch would conflict otherwise.

[0] https://gitlab.freedesktop.org/libinput/libinput/-/issues/1014

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
Changes in v2:
- Refactored the API to not use a new hook but hid_bpf_rdesc_fixup
  instead
- Some cleanups in hid-core.c probe() device to not kmemdup multiple
  time the report descriptor when it's not required
- I'm still not 100% sure the HID_QUIRK_IGNORE_HIDINPUT is that
  required, but I can not think of anything else at the moment to
  temporary disable any driver input device.
- Link to v1: https://lore.kernel.org/r/20240903-hid-bpf-hid-generic-v1-0-9511a565b2da@kernel.org

---
Benjamin Tissoires (11):
      HID: bpf: move HID-BPF report descriptor fixup earlier
      HID: core: save one kmemdup during .probe()
      HID: core: remove one more kmemdup on .probe()
      HID: bpf: allow write access to quirks field in struct hid_device
      selftests/hid: add dependency on hid_common.h
      selftests/hid: cleanup C tests by adding a common struct uhid_device
      selftests/hid: allow to parametrize bus/vid/pid/rdesc on the test device
      HID: add per device quirk to force bind to hid-generic
      selftests/hid: add test for assigning a given device to hid-generic
      HID: add quirk to prevent hid-input to be used
      selftests/hid: add test to disable hid-input

 drivers/hid/bpf/hid_bpf_dispatch.c                 |   8 +-
 drivers/hid/bpf/hid_bpf_struct_ops.c               |   1 +
 drivers/hid/hid-core.c                             |  72 ++++++--
 drivers/hid/hid-generic.c                          |   3 +
 include/linux/hid.h                                |  22 ++-
 include/linux/hid_bpf.h                            |   9 +-
 tools/testing/selftests/hid/Makefile               |   2 +-
 tools/testing/selftests/hid/hid_bpf.c              | 205 ++++++++++++++++-----
 tools/testing/selftests/hid/hid_common.h           | 112 +++++++----
 tools/testing/selftests/hid/hidraw.c               |  36 +---
 tools/testing/selftests/hid/progs/hid.c            |  13 ++
 .../testing/selftests/hid/progs/hid_bpf_helpers.h  |   7 +-
 12 files changed, 343 insertions(+), 147 deletions(-)
---
base-commit: e1370d5de7b755600df050979e19fbcd625fb4c6
change-id: 20240829-hid-bpf-hid-generic-61579f5b5945

Best regards,
-- 
Benjamin Tissoires <bentiss@kernel.org>

[PATCH HID v2 01/11] HID: bpf: move HID-BPF report descriptor fixup earlier

[Benjamin Tissoires]

Currently, hid_bpf_rdesc_fixup() is called once the match between the
HID device and the driver is done. This can be problematic in case
the driver selected by the kernel would change the report descriptor
after the fact.

To give a chance for hid_bpf_rdesc_fixup() to provide hints on to how
to select a dedicated driver or not, move the call to that BPF hook
earlier in the .probe() process, when we get the first match.

However, this means that we might get called more than once (typically
once for hid-generic, and once for hid-vendor-specific). So we store the
result of HID-BPF fixup in struct hid_device. If that new value is NULL,
we need to use ->dev_rdesc.

In order to not grow struct hid_device, some fields are re-ordered. This
was the output of pahole for the first 128 bytes:
struct hid_device {
	__u8 *                     dev_rdesc;            /*     0     8 */
	unsigned int               dev_rsize;            /*     8     4 */

	/* XXX 4 bytes hole, try to pack */

	__u8 *                     rdesc;                /*    16     8 */
	unsigned int               rsize;                /*    24     4 */

	/* XXX 4 bytes hole, try to pack */

	struct hid_collection *    collection;           /*    32     8 */
	unsigned int               collection_size;      /*    40     4 */
	unsigned int               maxcollection;        /*    44     4 */
	unsigned int               maxapplication;       /*    48     4 */
	__u16                      bus;                  /*    52     2 */
	__u16                      group;                /*    54     2 */
	__u32                      vendor;               /*    56     4 */
	__u32                      product;              /*    60     4 */
	/* --- cacheline 1 boundary (64 bytes) --- */
	__u32                      version;              /*    64     4 */
	enum hid_type              type;                 /*    68     4 */
	unsigned int               country;              /*    72     4 */

	/* XXX 4 bytes hole, try to pack */

	struct hid_report_enum     report_enum[3];       /*    80  6216 */

Basically, we got three holes of 4 bytes. We can reorder things a little
and makes those 3 holes a continuous 12 bytes hole, which can be replaced
by the new pointer and the new unsigned int we need.

In terms of code allocation, when not using HID-BPF, we are back to kernel
v6.2 in hid_open_report(). These multiple kmemdup() calls will be fixed
in a later commit.

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>

---

new in v2
---
 drivers/hid/bpf/hid_bpf_dispatch.c |  8 ++++++--
 drivers/hid/hid-core.c             | 17 +++++++++++++----
 include/linux/hid.h                | 18 ++++++++++--------
 include/linux/hid_bpf.h            |  9 ++-------
 4 files changed, 31 insertions(+), 21 deletions(-)

diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
index 8420c227e21b..58839c125fba 100644
--- a/drivers/hid/bpf/hid_bpf_dispatch.c
+++ b/drivers/hid/bpf/hid_bpf_dispatch.c
@@ -183,7 +183,7 @@ u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, const u8 *rdesc, unsigned
 
  ignore_bpf:
 	kfree(ctx_kern.data);
-	return kmemdup(rdesc, *size, GFP_KERNEL);
+	return NULL;
 }
 EXPORT_SYMBOL_GPL(call_hid_bpf_rdesc_fixup);
 
@@ -260,8 +260,12 @@ int hid_bpf_allocate_event_data(struct hid_device *hdev)
 
 int hid_bpf_reconnect(struct hid_device *hdev)
 {
-	if (!test_and_set_bit(ffs(HID_STAT_REPROBED), &hdev->status))
+	if (!test_and_set_bit(ffs(HID_STAT_REPROBED), &hdev->status)) {
+		kfree(hdev->bpf_rdesc);
+		hdev->bpf_rdesc = NULL;
+		hdev->bpf_rsize = 0;
 		return device_reprobe(&hdev->dev);
+	}
 
 	return 0;
 }
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 311069588321..a5f5415571cb 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -708,6 +708,7 @@ void hiddev_free(struct kref *ref)
 
 	hid_close_report(hid);
 	kfree(hid->dev_rdesc);
+	kfree(hid->bpf_rdesc);
 	kfree(hid);
 }
 
@@ -1221,13 +1222,12 @@ int hid_open_report(struct hid_device *device)
 	if (WARN_ON(device->status & HID_STAT_PARSED))
 		return -EBUSY;
 
-	start = device->dev_rdesc;
+	start = device->bpf_rdesc ? device->bpf_rdesc : device->dev_rdesc;
+	size = device->bpf_rsize;
 	if (WARN_ON(!start))
 		return -ENODEV;
-	size = device->dev_rsize;
 
-	/* call_hid_bpf_rdesc_fixup() ensures we work on a copy of rdesc */
-	buf = call_hid_bpf_rdesc_fixup(device, start, &size);
+	buf = kmemdup(start, size, GFP_KERNEL);
 	if (buf == NULL)
 		return -ENOMEM;
 
@@ -2659,6 +2659,12 @@ static int __hid_device_probe(struct hid_device *hdev, struct hid_driver *hdrv)
 	const struct hid_device_id *id;
 	int ret;
 
+	if (!hdev->bpf_rsize) {
+		hdev->bpf_rsize = hdev->dev_rsize;
+		hdev->bpf_rdesc = call_hid_bpf_rdesc_fixup(hdev, hdev->dev_rdesc,
+							   &hdev->bpf_rsize);
+	}
+
 	if (!hid_check_device_match(hdev, hdrv, &id))
 		return -ENODEV;
 
@@ -2916,8 +2922,11 @@ static void hid_remove_device(struct hid_device *hdev)
 		hdev->status &= ~HID_STAT_ADDED;
 	}
 	kfree(hdev->dev_rdesc);
+	kfree(hdev->bpf_rdesc);
 	hdev->dev_rdesc = NULL;
+	hdev->bpf_rdesc = NULL;
 	hdev->dev_rsize = 0;
+	hdev->bpf_rsize = 0;
 }
 
 /**
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 3a998fa3812d..d41fa18f1e03 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -599,15 +599,17 @@ enum hid_battery_status {
 struct hid_driver;
 struct hid_ll_driver;
 
-struct hid_device {							/* device report descriptor */
-	const __u8 *dev_rdesc;
-	unsigned dev_rsize;
-	const __u8 *rdesc;
-	unsigned rsize;
+struct hid_device {
+	const __u8 *dev_rdesc;						/* device report descriptor */
+	const __u8 *bpf_rdesc;						/* bpf modified report descriptor, if any */
+	const __u8 *rdesc;						/* currently used report descriptor */
+	unsigned int dev_rsize;
+	unsigned int bpf_rsize;
+	unsigned int rsize;
+	unsigned int collection_size;					/* Number of allocated hid_collections */
 	struct hid_collection *collection;				/* List of HID collections */
-	unsigned collection_size;					/* Number of allocated hid_collections */
-	unsigned maxcollection;						/* Number of parsed collections */
-	unsigned maxapplication;					/* Number of applications */
+	unsigned int maxcollection;						/* Number of parsed collections */
+	unsigned int maxapplication;					/* Number of applications */
 	__u16 bus;							/* BUS ID */
 	__u16 group;							/* Report group */
 	__u32 vendor;							/* Vendor ID */
diff --git a/include/linux/hid_bpf.h b/include/linux/hid_bpf.h
index 6a47223e6460..0290833544c8 100644
--- a/include/linux/hid_bpf.h
+++ b/include/linux/hid_bpf.h
@@ -228,13 +228,8 @@ static inline int hid_bpf_connect_device(struct hid_device *hdev) { return 0; }
 static inline void hid_bpf_disconnect_device(struct hid_device *hdev) {}
 static inline void hid_bpf_destroy_device(struct hid_device *hid) {}
 static inline int hid_bpf_device_init(struct hid_device *hid) { return 0; }
-/*
- * This specialized allocator has to be a macro for its allocations to be
- * accounted separately (to have a separate alloc_tag). The typecast is
- * intentional to enforce typesafety.
- */
-#define call_hid_bpf_rdesc_fixup(_hdev, _rdesc, _size)	\
-		((u8 *)kmemdup(_rdesc, *(_size), GFP_KERNEL))
+static inline u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc,
+					   unsigned int *size) { return NULL; }
 
 #endif /* CONFIG_HID_BPF */
 

-- 
2.46.0

[PATCH HID v2 02/11] HID: core: save one kmemdup during .probe()

[Benjamin Tissoires]

Turns out the first kmemdup is only required for the .report_fixup()
driver callback. There is no need to do two kmemdup() in a raw in case
.report_fixup() is not present.

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>

---

new in v2
---
 drivers/hid/hid-core.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index a5f5415571cb..172746a082f9 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1206,7 +1206,7 @@ int hid_open_report(struct hid_device *device)
 	struct hid_item item;
 	unsigned int size;
 	const __u8 *start;
-	__u8 *buf;
+	__u8 *buf = NULL;
 	const __u8 *end;
 	const __u8 *next;
 	int ret;
@@ -1227,14 +1227,18 @@ int hid_open_report(struct hid_device *device)
 	if (WARN_ON(!start))
 		return -ENODEV;
 
-	buf = kmemdup(start, size, GFP_KERNEL);
-	if (buf == NULL)
-		return -ENOMEM;
+	if (device->driver->report_fixup) {
+		/*
+		 * device->driver->report_fixup() needs to work
+		 * on a copy of our report descriptor so it can
+		 * change it.
+		 */
+		buf = kmemdup(start, size, GFP_KERNEL);
+		if (buf == NULL)
+			return -ENOMEM;
 
-	if (device->driver->report_fixup)
 		start = device->driver->report_fixup(device, buf, &size);
-	else
-		start = buf;
+	}
 
 	start = kmemdup(start, size, GFP_KERNEL);
 	kfree(buf);

-- 
2.46.0

[PATCH HID v2 03/11] HID: core: remove one more kmemdup on .probe()

[Benjamin Tissoires]

That last kmemdup while opening the report descriptor was required to
have a common kfree() on it.

Move that kmemdup in the only special case it's required (if there is a
.report_fixup()), and add a more elaborated check before freeing
hdev->rdesc, to avoid a double free.

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>

---

new in v2
---
 drivers/hid/hid-core.c | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 172746a082f9..2b70a12a2135 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -685,7 +685,14 @@ static void hid_close_report(struct hid_device *device)
 		INIT_LIST_HEAD(&report_enum->report_list);
 	}
 
-	kfree(device->rdesc);
+	/*
+	 * If the HID driver had a rdesc_fixup() callback, dev->rdesc
+	 * will be allocated by hid-core and needs to be freed.
+	 * Otherwise, it is either equal to dev_rdesc or bpf_rdesc, in
+	 * which cases it'll be freed later on device removal or destroy.
+	 */
+	if (device->rdesc != device->dev_rdesc && device->rdesc != device->bpf_rdesc)
+		kfree(device->rdesc);
 	device->rdesc = NULL;
 	device->rsize = 0;
 
@@ -1206,7 +1213,6 @@ int hid_open_report(struct hid_device *device)
 	struct hid_item item;
 	unsigned int size;
 	const __u8 *start;
-	__u8 *buf = NULL;
 	const __u8 *end;
 	const __u8 *next;
 	int ret;
@@ -1233,17 +1239,23 @@ int hid_open_report(struct hid_device *device)
 		 * on a copy of our report descriptor so it can
 		 * change it.
 		 */
-		buf = kmemdup(start, size, GFP_KERNEL);
+		__u8 *buf = kmemdup(start, size, GFP_KERNEL);
+
 		if (buf == NULL)
 			return -ENOMEM;
 
 		start = device->driver->report_fixup(device, buf, &size);
-	}
 
-	start = kmemdup(start, size, GFP_KERNEL);
-	kfree(buf);
-	if (start == NULL)
-		return -ENOMEM;
+		/*
+		 * The second kmemdup is required in case report_fixup() returns
+		 * a static read-only memory, but we have no idea if that memory
+		 * needs to be cleaned up or not at the end.
+		 */
+		start = kmemdup(start, size, GFP_KERNEL);
+		kfree(buf);
+		if (start == NULL)
+			return -ENOMEM;
+	}
 
 	device->rdesc = start;
 	device->rsize = size;

-- 
2.46.0

[PATCH HID v2 04/11] HID: bpf: allow write access to quirks field in struct hid_device

[Benjamin Tissoires]

This allows to give more control from BPF during report descriptor fixup.
We already reset the quirks before calling ->probe(), so now we reset
it once before calling hid_bpf_rdesc_fixup().

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>

---

new in v2
---
 drivers/hid/bpf/hid_bpf_struct_ops.c |  1 +
 drivers/hid/hid-core.c               | 10 ++++++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/bpf/hid_bpf_struct_ops.c b/drivers/hid/bpf/hid_bpf_struct_ops.c
index cd696c59ba0f..e4c25ade54a3 100644
--- a/drivers/hid/bpf/hid_bpf_struct_ops.c
+++ b/drivers/hid/bpf/hid_bpf_struct_ops.c
@@ -79,6 +79,7 @@ static int hid_bpf_ops_btf_struct_access(struct bpf_verifier_log *log,
 		WRITE_RANGE(hid_device, name, true),
 		WRITE_RANGE(hid_device, uniq, true),
 		WRITE_RANGE(hid_device, phys, true),
+		WRITE_RANGE(hid_device, quirks, false),
 	};
 #undef WRITE_RANGE
 	const struct btf_type *state = NULL;
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 2b70a12a2135..37e52759a931 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2676,9 +2676,17 @@ static int __hid_device_probe(struct hid_device *hdev, struct hid_driver *hdrv)
 	int ret;
 
 	if (!hdev->bpf_rsize) {
+		unsigned int quirks;
+
+		/* reset the quirks that has been previously set */
+		quirks = hid_lookup_quirk(hdev);
+		hdev->quirks = quirks;
 		hdev->bpf_rsize = hdev->dev_rsize;
 		hdev->bpf_rdesc = call_hid_bpf_rdesc_fixup(hdev, hdev->dev_rdesc,
 							   &hdev->bpf_rsize);
+		if (quirks ^ hdev->quirks)
+			hid_info(hdev, "HID-BPF toggled quirks on the device: %04x",
+				 quirks ^ hdev->quirks);
 	}
 
 	if (!hid_check_device_match(hdev, hdrv, &id))
@@ -2688,8 +2696,6 @@ static int __hid_device_probe(struct hid_device *hdev, struct hid_driver *hdrv)
 	if (!hdev->devres_group_id)
 		return -ENOMEM;
 
-	/* reset the quirks that has been previously set */
-	hdev->quirks = hid_lookup_quirk(hdev);
 	hdev->driver = hdrv;
 
 	if (hdrv->probe) {

-- 
2.46.0

[PATCH HID v2 05/11] selftests/hid: add dependency on hid_common.h

[Benjamin Tissoires]

Allows to recompile the C tests when that file changes

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>

---

no changes in v2
---
 tools/testing/selftests/hid/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/hid/Makefile b/tools/testing/selftests/hid/Makefile
index 72be55ac4bdf..9399fa3f2f9d 100644
--- a/tools/testing/selftests/hid/Makefile
+++ b/tools/testing/selftests/hid/Makefile
@@ -229,7 +229,7 @@ $(BPF_SKELS): %.skel.h: %.bpf.o $(BPFTOOL) | $(OUTPUT)
 	$(Q)$(BPFTOOL) gen object $(<:.o=.linked1.o) $<
 	$(Q)$(BPFTOOL) gen skeleton $(<:.o=.linked1.o) name $(notdir $(<:.bpf.o=)) > $@
 
-$(OUTPUT)/%.o: %.c $(BPF_SKELS)
+$(OUTPUT)/%.o: %.c $(BPF_SKELS) hid_common.h
 	$(call msg,CC,,$@)
 	$(Q)$(CC) $(CFLAGS) -c $(filter %.c,$^) $(LDLIBS) -o $@
 

-- 
2.46.0

[PATCH HID v2 06/11] selftests/hid: cleanup C tests by adding a common struct uhid_device

[Benjamin Tissoires]

Allows to have an abstract class uhid_device which handles all of the
uhid part without having to mess up with individual fds.

struct attach_prog_args is now never used in hid_bpf.c, so drop it as well

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>

---

no changes in v2
---
 tools/testing/selftests/hid/hid_bpf.c    | 77 +++++++++++---------------------
 tools/testing/selftests/hid/hid_common.h | 74 ++++++++++++++++++++----------
 tools/testing/selftests/hid/hidraw.c     | 36 +++++----------
 3 files changed, 87 insertions(+), 100 deletions(-)

diff --git a/tools/testing/selftests/hid/hid_bpf.c b/tools/testing/selftests/hid/hid_bpf.c
index d10cf6883683..17ccbf5ff4b5 100644
--- a/tools/testing/selftests/hid/hid_bpf.c
+++ b/tools/testing/selftests/hid/hid_bpf.c
@@ -4,13 +4,6 @@
 #include "hid_common.h"
 #include <bpf/bpf.h>
 
-struct attach_prog_args {
-	int prog_fd;
-	unsigned int hid;
-	int retval;
-	int insert_head;
-};
-
 struct hid_hw_request_syscall_args {
 	__u8 data[10];
 	unsigned int hid;
@@ -21,11 +14,8 @@ struct hid_hw_request_syscall_args {
 };
 
 FIXTURE(hid_bpf) {
-	int dev_id;
-	int uhid_fd;
+	struct uhid_device hid;
 	int hidraw_fd;
-	int hid_id;
-	pthread_t tid;
 	struct hid *skel;
 	struct bpf_link *hid_links[3]; /* max number of programs loaded in a single test */
 };
@@ -54,10 +44,10 @@ static void detach_bpf(FIXTURE_DATA(hid_bpf) * self)
 FIXTURE_TEARDOWN(hid_bpf) {
 	void *uhid_err;
 
-	uhid_destroy(_metadata, self->uhid_fd);
+	uhid_destroy(_metadata, &self->hid);
 
 	detach_bpf(self);
-	pthread_join(self->tid, &uhid_err);
+	pthread_join(self->hid.tid, &uhid_err);
 }
 #define TEARDOWN_LOG(fmt, ...) do { \
 	TH_LOG(fmt, ##__VA_ARGS__); \
@@ -66,23 +56,10 @@ FIXTURE_TEARDOWN(hid_bpf) {
 
 FIXTURE_SETUP(hid_bpf)
 {
-	time_t t;
 	int err;
 
-	/* initialize random number generator */
-	srand((unsigned int)time(&t));
-
-	self->dev_id = rand() % 1024;
-
-	self->uhid_fd = setup_uhid(_metadata, self->dev_id);
-
-	/* locate the uev, self, variant);ent file of the created device */
-	self->hid_id = get_hid_id(self->dev_id);
-	ASSERT_GT(self->hid_id, 0)
-		TEARDOWN_LOG("Could not locate uhid device id: %d", self->hid_id);
-
-	err = uhid_start_listener(_metadata, &self->tid, self->uhid_fd);
-	ASSERT_EQ(0, err) TEARDOWN_LOG("could not start udev listener: %d", err);
+	err = setup_uhid(_metadata, &self->hid);
+	ASSERT_OK(err);
 }
 
 struct test_program {
@@ -129,7 +106,7 @@ static void load_programs(const struct test_program programs[],
 		ops_hid_id = bpf_map__initial_value(map, NULL);
 		ASSERT_OK_PTR(ops_hid_id) TH_LOG("unable to retrieve struct_ops data");
 
-		*ops_hid_id = self->hid_id;
+		*ops_hid_id = self->hid.hid_id;
 	}
 
 	/* we disable the auto-attach feature of all maps because we
@@ -157,7 +134,7 @@ static void load_programs(const struct test_program programs[],
 
 	hid__attach(self->skel);
 
-	self->hidraw_fd = open_hidraw(self->dev_id);
+	self->hidraw_fd = open_hidraw(&self->hid);
 	ASSERT_GE(self->hidraw_fd, 0) TH_LOG("open_hidraw");
 }
 
@@ -192,7 +169,7 @@ TEST_F(hid_bpf, raw_event)
 	/* inject one event */
 	buf[0] = 1;
 	buf[1] = 42;
-	uhid_send_event(_metadata, self->uhid_fd, buf, 6);
+	uhid_send_event(_metadata, &self->hid, buf, 6);
 
 	/* check that hid_first_event() was executed */
 	ASSERT_EQ(self->skel->data->callback_check, 42) TH_LOG("callback_check1");
@@ -208,7 +185,7 @@ TEST_F(hid_bpf, raw_event)
 	memset(buf, 0, sizeof(buf));
 	buf[0] = 1;
 	buf[1] = 47;
-	uhid_send_event(_metadata, self->uhid_fd, buf, 6);
+	uhid_send_event(_metadata, &self->hid, buf, 6);
 
 	/* check that hid_first_event() was executed */
 	ASSERT_EQ(self->skel->data->callback_check, 47) TH_LOG("callback_check1");
@@ -239,7 +216,7 @@ TEST_F(hid_bpf, subprog_raw_event)
 	/* inject one event */
 	buf[0] = 1;
 	buf[1] = 42;
-	uhid_send_event(_metadata, self->uhid_fd, buf, 6);
+	uhid_send_event(_metadata, &self->hid, buf, 6);
 
 	/* read the data from hidraw */
 	memset(buf, 0, sizeof(buf));
@@ -252,7 +229,7 @@ TEST_F(hid_bpf, subprog_raw_event)
 	memset(buf, 0, sizeof(buf));
 	buf[0] = 1;
 	buf[1] = 47;
-	uhid_send_event(_metadata, self->uhid_fd, buf, 6);
+	uhid_send_event(_metadata, &self->hid, buf, 6);
 
 	/* read the data from hidraw */
 	memset(buf, 0, sizeof(buf));
@@ -303,7 +280,7 @@ TEST_F(hid_bpf, test_attach_detach)
 	/* inject one event */
 	buf[0] = 1;
 	buf[1] = 42;
-	uhid_send_event(_metadata, self->uhid_fd, buf, 6);
+	uhid_send_event(_metadata, &self->hid, buf, 6);
 
 	/* read the data from hidraw */
 	memset(buf, 0, sizeof(buf));
@@ -326,14 +303,14 @@ TEST_F(hid_bpf, test_attach_detach)
 	/* detach the program */
 	detach_bpf(self);
 
-	self->hidraw_fd = open_hidraw(self->dev_id);
+	self->hidraw_fd = open_hidraw(&self->hid);
 	ASSERT_GE(self->hidraw_fd, 0) TH_LOG("open_hidraw");
 
 	/* inject another event */
 	memset(buf, 0, sizeof(buf));
 	buf[0] = 1;
 	buf[1] = 47;
-	uhid_send_event(_metadata, self->uhid_fd, buf, 6);
+	uhid_send_event(_metadata, &self->hid, buf, 6);
 
 	/* read the data from hidraw */
 	memset(buf, 0, sizeof(buf));
@@ -352,7 +329,7 @@ TEST_F(hid_bpf, test_attach_detach)
 	memset(buf, 0, sizeof(buf));
 	buf[0] = 1;
 	buf[1] = 42;
-	uhid_send_event(_metadata, self->uhid_fd, buf, 6);
+	uhid_send_event(_metadata, &self->hid, buf, 6);
 
 	/* read the data from hidraw */
 	memset(buf, 0, sizeof(buf));
@@ -382,7 +359,7 @@ TEST_F(hid_bpf, test_hid_change_report)
 	/* inject one event */
 	buf[0] = 1;
 	buf[1] = 42;
-	uhid_send_event(_metadata, self->uhid_fd, buf, 6);
+	uhid_send_event(_metadata, &self->hid, buf, 6);
 
 	/* read the data from hidraw */
 	memset(buf, 0, sizeof(buf));
@@ -412,7 +389,7 @@ TEST_F(hid_bpf, test_hid_user_input_report_call)
 
 	LOAD_BPF;
 
-	args.hid = self->hid_id;
+	args.hid = self->hid.hid_id;
 	args.data[0] = 1; /* report ID */
 	args.data[1] = 2; /* report ID */
 	args.data[2] = 42; /* report ID */
@@ -458,7 +435,7 @@ TEST_F(hid_bpf, test_hid_user_output_report_call)
 
 	LOAD_BPF;
 
-	args.hid = self->hid_id;
+	args.hid = self->hid.hid_id;
 	args.data[0] = 1; /* report ID */
 	args.data[1] = 2; /* report ID */
 	args.data[2] = 42; /* report ID */
@@ -506,7 +483,7 @@ TEST_F(hid_bpf, test_hid_user_raw_request_call)
 
 	LOAD_BPF;
 
-	args.hid = self->hid_id;
+	args.hid = self->hid.hid_id;
 	args.data[0] = 1; /* report ID */
 
 	prog_fd = bpf_program__fd(self->skel->progs.hid_user_raw_request);
@@ -539,7 +516,7 @@ TEST_F(hid_bpf, test_hid_filter_raw_request_call)
 	/* inject one event */
 	buf[0] = 1;
 	buf[1] = 42;
-	uhid_send_event(_metadata, self->uhid_fd, buf, 6);
+	uhid_send_event(_metadata, &self->hid, buf, 6);
 
 	/* read the data from hidraw */
 	memset(buf, 0, sizeof(buf));
@@ -565,7 +542,7 @@ TEST_F(hid_bpf, test_hid_filter_raw_request_call)
 	/* detach the program */
 	detach_bpf(self);
 
-	self->hidraw_fd = open_hidraw(self->dev_id);
+	self->hidraw_fd = open_hidraw(&self->hid);
 	ASSERT_GE(self->hidraw_fd, 0) TH_LOG("open_hidraw");
 
 	err = ioctl(self->hidraw_fd, HIDIOCGFEATURE(sizeof(buf)), buf);
@@ -641,7 +618,7 @@ TEST_F(hid_bpf, test_hid_filter_output_report_call)
 	/* inject one event */
 	buf[0] = 1;
 	buf[1] = 42;
-	uhid_send_event(_metadata, self->uhid_fd, buf, 6);
+	uhid_send_event(_metadata, &self->hid, buf, 6);
 
 	/* read the data from hidraw */
 	memset(buf, 0, sizeof(buf));
@@ -667,7 +644,7 @@ TEST_F(hid_bpf, test_hid_filter_output_report_call)
 	/* detach the program */
 	detach_bpf(self);
 
-	self->hidraw_fd = open_hidraw(self->dev_id);
+	self->hidraw_fd = open_hidraw(&self->hid);
 	ASSERT_GE(self->hidraw_fd, 0) TH_LOG("open_hidraw");
 
 	err = write(self->hidraw_fd, buf, 3);
@@ -742,7 +719,7 @@ TEST_F(hid_bpf, test_multiply_events_wq)
 	/* inject one event */
 	buf[0] = 1;
 	buf[1] = 42;
-	uhid_send_event(_metadata, self->uhid_fd, buf, 6);
+	uhid_send_event(_metadata, &self->hid, buf, 6);
 
 	/* read the data from hidraw */
 	memset(buf, 0, sizeof(buf));
@@ -780,7 +757,7 @@ TEST_F(hid_bpf, test_multiply_events)
 	/* inject one event */
 	buf[0] = 1;
 	buf[1] = 42;
-	uhid_send_event(_metadata, self->uhid_fd, buf, 6);
+	uhid_send_event(_metadata, &self->hid, buf, 6);
 
 	/* read the data from hidraw */
 	memset(buf, 0, sizeof(buf));
@@ -816,7 +793,7 @@ TEST_F(hid_bpf, test_hid_infinite_loop_input_report_call)
 	buf[1] = 2;
 	buf[2] = 42;
 
-	uhid_send_event(_metadata, self->uhid_fd, buf, 6);
+	uhid_send_event(_metadata, &self->hid, buf, 6);
 
 	/* read the data from hidraw */
 	memset(buf, 0, sizeof(buf));
@@ -867,7 +844,7 @@ TEST_F(hid_bpf, test_hid_attach_flags)
 
 	/* inject one event */
 	buf[0] = 1;
-	uhid_send_event(_metadata, self->uhid_fd, buf, 6);
+	uhid_send_event(_metadata, &self->hid, buf, 6);
 
 	/* read the data from hidraw */
 	memset(buf, 0, sizeof(buf));
diff --git a/tools/testing/selftests/hid/hid_common.h b/tools/testing/selftests/hid/hid_common.h
index f151f151a1ed..a7d836a35bb1 100644
--- a/tools/testing/selftests/hid/hid_common.h
+++ b/tools/testing/selftests/hid/hid_common.h
@@ -19,6 +19,13 @@
 	__typeof__(b) _b = (b); \
 	_a < _b ? _a : _b; })
 
+struct uhid_device {
+	int dev_id;		/* uniq (random) number to identify the device */
+	int uhid_fd;
+	int hid_id;		/* HID device id in the system */
+	pthread_t tid;		/* thread for reading uhid events */
+};
+
 static unsigned char rdesc[] = {
 	0x06, 0x00, 0xff,	/* Usage Page (Vendor Defined Page 1) */
 	0x09, 0x21,		/* Usage (Vendor Usage 0x21) */
@@ -146,14 +153,14 @@ static int uhid_create(struct __test_metadata *_metadata, int fd, int rand_nb)
 	return uhid_write(_metadata, fd, &ev);
 }
 
-static void uhid_destroy(struct __test_metadata *_metadata, int fd)
+static void uhid_destroy(struct __test_metadata *_metadata, struct uhid_device *hid)
 {
 	struct uhid_event ev;
 
 	memset(&ev, 0, sizeof(ev));
 	ev.type = UHID_DESTROY;
 
-	uhid_write(_metadata, fd, &ev);
+	uhid_write(_metadata, hid->uhid_fd, &ev);
 }
 
 static int uhid_event(struct __test_metadata *_metadata, int fd)
@@ -281,7 +288,8 @@ static int uhid_start_listener(struct __test_metadata *_metadata, pthread_t *tid
 	return 0;
 }
 
-static int uhid_send_event(struct __test_metadata *_metadata, int fd, __u8 *buf, size_t size)
+static int uhid_send_event(struct __test_metadata *_metadata, struct uhid_device *hid,
+			   __u8 *buf, size_t size)
 {
 	struct uhid_event ev;
 
@@ -294,25 +302,7 @@ static int uhid_send_event(struct __test_metadata *_metadata, int fd, __u8 *buf,
 
 	memcpy(ev.u.input2.data, buf, size);
 
-	return uhid_write(_metadata, fd, &ev);
-}
-
-static int setup_uhid(struct __test_metadata *_metadata, int rand_nb)
-{
-	int fd;
-	const char *path = "/dev/uhid";
-	int ret;
-
-	fd = open(path, O_RDWR | O_CLOEXEC);
-	ASSERT_GE(fd, 0) TH_LOG("open uhid-cdev failed; %d", fd);
-
-	ret = uhid_create(_metadata, fd, rand_nb);
-	ASSERT_EQ(0, ret) {
-		TH_LOG("create uhid device failed: %d", ret);
-		close(fd);
-	}
-
-	return fd;
+	return uhid_write(_metadata, hid->uhid_fd, &ev);
 }
 
 static bool match_sysfs_device(int dev_id, const char *workdir, struct dirent *dir)
@@ -421,12 +411,12 @@ static int get_hidraw(int dev_id)
 	return found;
 }
 
-static int open_hidraw(int dev_id)
+static int open_hidraw(struct uhid_device *hid)
 {
 	int hidraw_number;
 	char hidraw_path[64] = { 0 };
 
-	hidraw_number = get_hidraw(dev_id);
+	hidraw_number = get_hidraw(hid->dev_id);
 	if (hidraw_number < 0)
 		return hidraw_number;
 
@@ -434,3 +424,39 @@ static int open_hidraw(int dev_id)
 	sprintf(hidraw_path, "/dev/hidraw%d", hidraw_number);
 	return open(hidraw_path, O_RDWR | O_NONBLOCK);
 }
+
+static int setup_uhid(struct __test_metadata *_metadata, struct uhid_device *hid)
+{
+	const char *path = "/dev/uhid";
+	time_t t;
+	int ret;
+
+	/* initialize random number generator */
+	srand((unsigned int)time(&t));
+
+	hid->dev_id = rand() % 1024;
+
+	hid->uhid_fd = open(path, O_RDWR | O_CLOEXEC);
+	ASSERT_GE(hid->uhid_fd, 0) TH_LOG("open uhid-cdev failed; %d", hid->uhid_fd);
+
+	ret = uhid_create(_metadata, hid->uhid_fd, hid->dev_id);
+	ASSERT_EQ(0, ret) {
+		TH_LOG("create uhid device failed: %d", ret);
+		close(hid->uhid_fd);
+		return ret;
+	}
+
+	/* locate the uevent file of the created device */
+	hid->hid_id = get_hid_id(hid->dev_id);
+	ASSERT_GT(hid->hid_id, 0)
+		TH_LOG("Could not locate uhid device id: %d", hid->hid_id);
+
+	ret = uhid_start_listener(_metadata, &hid->tid, hid->uhid_fd);
+	ASSERT_EQ(0, ret) {
+		TH_LOG("could not start udev listener: %d", ret);
+		close(hid->uhid_fd);
+		return ret;
+	}
+
+	return 0;
+}
diff --git a/tools/testing/selftests/hid/hidraw.c b/tools/testing/selftests/hid/hidraw.c
index f8b4f7ff292c..5934818b2036 100644
--- a/tools/testing/selftests/hid/hidraw.c
+++ b/tools/testing/selftests/hid/hidraw.c
@@ -9,11 +9,8 @@
 #endif /* HIDIOCREVOKE */
 
 FIXTURE(hidraw) {
-	int dev_id;
-	int uhid_fd;
+	struct uhid_device hid;
 	int hidraw_fd;
-	int hid_id;
-	pthread_t tid;
 };
 static void close_hidraw(FIXTURE_DATA(hidraw) * self)
 {
@@ -25,10 +22,10 @@ static void close_hidraw(FIXTURE_DATA(hidraw) * self)
 FIXTURE_TEARDOWN(hidraw) {
 	void *uhid_err;
 
-	uhid_destroy(_metadata, self->uhid_fd);
+	uhid_destroy(_metadata, &self->hid);
 
 	close_hidraw(self);
-	pthread_join(self->tid, &uhid_err);
+	pthread_join(self->hid.tid, &uhid_err);
 }
 #define TEARDOWN_LOG(fmt, ...) do { \
 	TH_LOG(fmt, ##__VA_ARGS__); \
@@ -37,25 +34,12 @@ FIXTURE_TEARDOWN(hidraw) {
 
 FIXTURE_SETUP(hidraw)
 {
-	time_t t;
 	int err;
 
-	/* initialize random number generator */
-	srand((unsigned int)time(&t));
+	err = setup_uhid(_metadata, &self->hid);
+	ASSERT_OK(err);
 
-	self->dev_id = rand() % 1024;
-
-	self->uhid_fd = setup_uhid(_metadata, self->dev_id);
-
-	/* locate the uev, self, variant);ent file of the created device */
-	self->hid_id = get_hid_id(self->dev_id);
-	ASSERT_GT(self->hid_id, 0)
-		TEARDOWN_LOG("Could not locate uhid device id: %d", self->hid_id);
-
-	err = uhid_start_listener(_metadata, &self->tid, self->uhid_fd);
-	ASSERT_EQ(0, err) TEARDOWN_LOG("could not start udev listener: %d", err);
-
-	self->hidraw_fd = open_hidraw(self->dev_id);
+	self->hidraw_fd = open_hidraw(&self->hid);
 	ASSERT_GE(self->hidraw_fd, 0) TH_LOG("open_hidraw");
 }
 
@@ -79,7 +63,7 @@ TEST_F(hidraw, raw_event)
 	/* inject one event */
 	buf[0] = 1;
 	buf[1] = 42;
-	uhid_send_event(_metadata, self->uhid_fd, buf, 6);
+	uhid_send_event(_metadata, &self->hid, buf, 6);
 
 	/* read the data from hidraw */
 	memset(buf, 0, sizeof(buf));
@@ -101,7 +85,7 @@ TEST_F(hidraw, raw_event_revoked)
 	/* inject one event */
 	buf[0] = 1;
 	buf[1] = 42;
-	uhid_send_event(_metadata, self->uhid_fd, buf, 6);
+	uhid_send_event(_metadata, &self->hid, buf, 6);
 
 	/* read the data from hidraw */
 	memset(buf, 0, sizeof(buf));
@@ -117,7 +101,7 @@ TEST_F(hidraw, raw_event_revoked)
 	/* inject one other event */
 	buf[0] = 1;
 	buf[1] = 43;
-	uhid_send_event(_metadata, self->uhid_fd, buf, 6);
+	uhid_send_event(_metadata, &self->hid, buf, 6);
 
 	/* read the data from hidraw */
 	memset(buf, 0, sizeof(buf));
@@ -161,7 +145,7 @@ TEST_F(hidraw, poll_revoked)
 	/* inject one event */
 	buf[0] = 1;
 	buf[1] = 42;
-	uhid_send_event(_metadata, self->uhid_fd, buf, 6);
+	uhid_send_event(_metadata, &self->hid, buf, 6);
 
 	while (true) {
 		ready = poll(pfds, 1, 5000);

-- 
2.46.0

[PATCH HID v2 07/11] selftests/hid: allow to parametrize bus/vid/pid/rdesc on the test device

[Benjamin Tissoires]

This will be useful to introduce variants in tests to test the
interactions between HID-BPF and some kernel modules.

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>

---

no changes in v2
---
 tools/testing/selftests/hid/hid_bpf.c    |  2 +-
 tools/testing/selftests/hid/hid_common.h | 46 ++++++++++++++++++++------------
 tools/testing/selftests/hid/hidraw.c     |  2 +-
 3 files changed, 31 insertions(+), 19 deletions(-)

diff --git a/tools/testing/selftests/hid/hid_bpf.c b/tools/testing/selftests/hid/hid_bpf.c
index 17ccbf5ff4b5..7eb15da62bdc 100644
--- a/tools/testing/selftests/hid/hid_bpf.c
+++ b/tools/testing/selftests/hid/hid_bpf.c
@@ -58,7 +58,7 @@ FIXTURE_SETUP(hid_bpf)
 {
 	int err;
 
-	err = setup_uhid(_metadata, &self->hid);
+	err = setup_uhid(_metadata, &self->hid, BUS_USB, 0x0001, 0x0a36, rdesc, sizeof(rdesc));
 	ASSERT_OK(err);
 }
 
diff --git a/tools/testing/selftests/hid/hid_common.h b/tools/testing/selftests/hid/hid_common.h
index a7d836a35bb1..f77f69c6657d 100644
--- a/tools/testing/selftests/hid/hid_common.h
+++ b/tools/testing/selftests/hid/hid_common.h
@@ -23,6 +23,9 @@ struct uhid_device {
 	int dev_id;		/* uniq (random) number to identify the device */
 	int uhid_fd;
 	int hid_id;		/* HID device id in the system */
+	__u16 bus;
+	__u32 vid;
+	__u32 pid;
 	pthread_t tid;		/* thread for reading uhid events */
 };
 
@@ -129,7 +132,9 @@ static int uhid_write(struct __test_metadata *_metadata, int fd, const struct uh
 	}
 }
 
-static int uhid_create(struct __test_metadata *_metadata, int fd, int rand_nb)
+static int uhid_create(struct __test_metadata *_metadata, int fd, int rand_nb,
+		       __u16 bus, __u32 vid, __u32 pid, __u8 *rdesc,
+		       size_t rdesc_size)
 {
 	struct uhid_event ev;
 	char buf[25];
@@ -140,10 +145,10 @@ static int uhid_create(struct __test_metadata *_metadata, int fd, int rand_nb)
 	ev.type = UHID_CREATE;
 	strcpy((char *)ev.u.create.name, buf);
 	ev.u.create.rd_data = rdesc;
-	ev.u.create.rd_size = sizeof(rdesc);
-	ev.u.create.bus = BUS_USB;
-	ev.u.create.vendor = 0x0001;
-	ev.u.create.product = 0x0a37;
+	ev.u.create.rd_size = rdesc_size;
+	ev.u.create.bus = bus;
+	ev.u.create.vendor = vid;
+	ev.u.create.product = pid;
 	ev.u.create.version = 0;
 	ev.u.create.country = 0;
 
@@ -305,15 +310,17 @@ static int uhid_send_event(struct __test_metadata *_metadata, struct uhid_device
 	return uhid_write(_metadata, hid->uhid_fd, &ev);
 }
 
-static bool match_sysfs_device(int dev_id, const char *workdir, struct dirent *dir)
+static bool match_sysfs_device(struct uhid_device *hid, const char *workdir, struct dirent *dir)
 {
-	const char *target = "0003:0001:0A37.*";
+	char target[20] = "";
 	char phys[512];
 	char uevent[1024];
 	char temp[512];
 	int fd, nread;
 	bool found = false;
 
+	snprintf(target, sizeof(target), "%04X:%04X:%04X.*", hid->bus, hid->vid, hid->pid);
+
 	if (fnmatch(target, dir->d_name, 0))
 		return false;
 
@@ -324,7 +331,7 @@ static bool match_sysfs_device(int dev_id, const char *workdir, struct dirent *d
 	if (fd < 0)
 		return false;
 
-	sprintf(phys, "PHYS=%d", dev_id);
+	sprintf(phys, "PHYS=%d", hid->dev_id);
 
 	nread = read(fd, temp, ARRAY_SIZE(temp));
 	if (nread > 0 && (strstr(temp, phys)) != NULL)
@@ -335,7 +342,7 @@ static bool match_sysfs_device(int dev_id, const char *workdir, struct dirent *d
 	return found;
 }
 
-static int get_hid_id(int dev_id)
+static int get_hid_id(struct uhid_device *hid)
 {
 	const char *workdir = "/sys/devices/virtual/misc/uhid";
 	const char *str_id;
@@ -350,10 +357,10 @@ static int get_hid_id(int dev_id)
 		d = opendir(workdir);
 		if (d) {
 			while ((dir = readdir(d)) != NULL) {
-				if (!match_sysfs_device(dev_id, workdir, dir))
+				if (!match_sysfs_device(hid, workdir, dir))
 					continue;
 
-				str_id = dir->d_name + sizeof("0003:0001:0A37.");
+				str_id = dir->d_name + sizeof("0000:0000:0000.");
 				found = (int)strtol(str_id, NULL, 16);
 
 				break;
@@ -367,7 +374,7 @@ static int get_hid_id(int dev_id)
 	return found;
 }
 
-static int get_hidraw(int dev_id)
+static int get_hidraw(struct uhid_device *hid)
 {
 	const char *workdir = "/sys/devices/virtual/misc/uhid";
 	char sysfs[1024];
@@ -384,7 +391,7 @@ static int get_hidraw(int dev_id)
 			continue;
 
 		while ((dir = readdir(d)) != NULL) {
-			if (!match_sysfs_device(dev_id, workdir, dir))
+			if (!match_sysfs_device(hid, workdir, dir))
 				continue;
 
 			sprintf(sysfs, "%s/%s/hidraw", workdir, dir->d_name);
@@ -416,7 +423,7 @@ static int open_hidraw(struct uhid_device *hid)
 	int hidraw_number;
 	char hidraw_path[64] = { 0 };
 
-	hidraw_number = get_hidraw(hid->dev_id);
+	hidraw_number = get_hidraw(hid);
 	if (hidraw_number < 0)
 		return hidraw_number;
 
@@ -425,7 +432,8 @@ static int open_hidraw(struct uhid_device *hid)
 	return open(hidraw_path, O_RDWR | O_NONBLOCK);
 }
 
-static int setup_uhid(struct __test_metadata *_metadata, struct uhid_device *hid)
+static int setup_uhid(struct __test_metadata *_metadata, struct uhid_device *hid,
+		      __u16 bus, __u32 vid, __u32 pid, const __u8 *rdesc, size_t rdesc_size)
 {
 	const char *path = "/dev/uhid";
 	time_t t;
@@ -435,11 +443,15 @@ static int setup_uhid(struct __test_metadata *_metadata, struct uhid_device *hid
 	srand((unsigned int)time(&t));
 
 	hid->dev_id = rand() % 1024;
+	hid->bus = bus;
+	hid->vid = vid;
+	hid->pid = pid;
 
 	hid->uhid_fd = open(path, O_RDWR | O_CLOEXEC);
 	ASSERT_GE(hid->uhid_fd, 0) TH_LOG("open uhid-cdev failed; %d", hid->uhid_fd);
 
-	ret = uhid_create(_metadata, hid->uhid_fd, hid->dev_id);
+	ret = uhid_create(_metadata, hid->uhid_fd, hid->dev_id, bus, vid, pid,
+			  (__u8 *)rdesc, rdesc_size);
 	ASSERT_EQ(0, ret) {
 		TH_LOG("create uhid device failed: %d", ret);
 		close(hid->uhid_fd);
@@ -447,7 +459,7 @@ static int setup_uhid(struct __test_metadata *_metadata, struct uhid_device *hid
 	}
 
 	/* locate the uevent file of the created device */
-	hid->hid_id = get_hid_id(hid->dev_id);
+	hid->hid_id = get_hid_id(hid);
 	ASSERT_GT(hid->hid_id, 0)
 		TH_LOG("Could not locate uhid device id: %d", hid->hid_id);
 
diff --git a/tools/testing/selftests/hid/hidraw.c b/tools/testing/selftests/hid/hidraw.c
index 5934818b2036..821db37ba4bb 100644
--- a/tools/testing/selftests/hid/hidraw.c
+++ b/tools/testing/selftests/hid/hidraw.c
@@ -36,7 +36,7 @@ FIXTURE_SETUP(hidraw)
 {
 	int err;
 
-	err = setup_uhid(_metadata, &self->hid);
+	err = setup_uhid(_metadata, &self->hid, BUS_USB, 0x0001, 0x0a37, rdesc, sizeof(rdesc));
 	ASSERT_OK(err);
 
 	self->hidraw_fd = open_hidraw(&self->hid);

-- 
2.46.0

[PATCH HID v2 08/11] HID: add per device quirk to force bind to hid-generic

[Benjamin Tissoires]

We already have the possibility to force not binding to hid-generic and
rely on a dedicated driver, but we couldn't do the other way around.

This is useful for BPF programs where we are fixing the report descriptor
and the events, but want to avoid a specialized driver to come after BPF
which would unwind everything that is done there.

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>

---

changes in v2:
- rely on hdev->quirks for that instead of a new struct for hid_driver
---
 drivers/hid/hid-core.c    | 5 +++--
 drivers/hid/hid-generic.c | 3 +++
 include/linux/hid.h       | 2 ++
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 37e52759a931..bf63e2819baf 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2665,9 +2665,10 @@ static bool hid_check_device_match(struct hid_device *hdev,
 	/*
 	 * hid-generic implements .match(), so we must be dealing with a
 	 * different HID driver here, and can simply check if
-	 * hid_ignore_special_drivers is set or not.
+	 * hid_ignore_special_drivers or HID_QUIRK_IGNORE_SPECIAL_DRIVER
+	 * are set or not.
 	 */
-	return !hid_ignore_special_drivers;
+	return !hid_ignore_special_drivers && !(hdev->quirks & HID_QUIRK_IGNORE_SPECIAL_DRIVER);
 }
 
 static int __hid_device_probe(struct hid_device *hdev, struct hid_driver *hdrv)
diff --git a/drivers/hid/hid-generic.c b/drivers/hid/hid-generic.c
index f9db991d3c5a..88882c1bfffe 100644
--- a/drivers/hid/hid-generic.c
+++ b/drivers/hid/hid-generic.c
@@ -40,6 +40,9 @@ static bool hid_generic_match(struct hid_device *hdev,
 	if (ignore_special_driver)
 		return true;
 
+	if (hdev->quirks & HID_QUIRK_IGNORE_SPECIAL_DRIVER)
+		return true;
+
 	if (hdev->quirks & HID_QUIRK_HAVE_SPECIAL_DRIVER)
 		return false;
 
diff --git a/include/linux/hid.h b/include/linux/hid.h
index d41fa18f1e03..b3a9586363c9 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -359,6 +359,7 @@ struct hid_item {
  * | @HID_QUIRK_NO_OUTPUT_REPORTS_ON_INTR_EP:
  * | @HID_QUIRK_HAVE_SPECIAL_DRIVER:
  * | @HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE:
+ * | @HID_QUIRK_IGNORE_SPECIAL_DRIVER
  * | @HID_QUIRK_FULLSPEED_INTERVAL:
  * | @HID_QUIRK_NO_INIT_REPORTS:
  * | @HID_QUIRK_NO_IGNORE:
@@ -384,6 +385,7 @@ struct hid_item {
 #define HID_QUIRK_HAVE_SPECIAL_DRIVER		BIT(19)
 #define HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE	BIT(20)
 #define HID_QUIRK_NOINVERT			BIT(21)
+#define HID_QUIRK_IGNORE_SPECIAL_DRIVER		BIT(22)
 #define HID_QUIRK_FULLSPEED_INTERVAL		BIT(28)
 #define HID_QUIRK_NO_INIT_REPORTS		BIT(29)
 #define HID_QUIRK_NO_IGNORE			BIT(30)

-- 
2.46.0

[PATCH HID v2 09/11] selftests/hid: add test for assigning a given device to hid-generic

[Benjamin Tissoires]

We use a well known VID/PID on a driver that doesn't need to talk to
the device, ensures we created the device against the target driver,
then load our program and ensure we have unbound to this driver and use
hid-generic instead.

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>

---

changes in v2:
- amended for the new API
---
 tools/testing/selftests/hid/hid_bpf.c              | 80 +++++++++++++++++++++-
 tools/testing/selftests/hid/progs/hid.c            | 12 ++++
 .../testing/selftests/hid/progs/hid_bpf_helpers.h  |  6 +-
 3 files changed, 96 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/hid/hid_bpf.c b/tools/testing/selftests/hid/hid_bpf.c
index 7eb15da62bdc..edc061b38528 100644
--- a/tools/testing/selftests/hid/hid_bpf.c
+++ b/tools/testing/selftests/hid/hid_bpf.c
@@ -54,11 +54,41 @@ FIXTURE_TEARDOWN(hid_bpf) {
 	hid_bpf_teardown(_metadata, self, variant); \
 } while (0)
 
+struct specific_device {
+	const char test_name[64];
+	__u16 bus;
+	__u32 vid;
+	__u32 pid;
+};
+
 FIXTURE_SETUP(hid_bpf)
 {
+	const struct specific_device *match = NULL;
 	int err;
 
-	err = setup_uhid(_metadata, &self->hid, BUS_USB, 0x0001, 0x0a36, rdesc, sizeof(rdesc));
+	const struct specific_device devices[] = {
+	{
+		.test_name = "test_hid_driver_probe",
+		.bus = BUS_BLUETOOTH,
+		.vid = 0x05ac,  /* USB_VENDOR_ID_APPLE */
+		.pid = 0x022c,  /* USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI */
+	}, {
+		.test_name = "*",
+		.bus = BUS_USB,
+		.vid = 0x0001,
+		.pid = 0x0a36,
+	}};
+
+	for (int i = 0; i < ARRAY_SIZE(devices); i++) {
+		match = &devices[i];
+		if (!strncmp(_metadata->name, devices[i].test_name, sizeof(devices[i].test_name)))
+			break;
+	}
+
+	ASSERT_OK_PTR(match);
+
+	err = setup_uhid(_metadata, &self->hid, match->bus, match->vid, match->pid,
+			 rdesc, sizeof(rdesc));
 	ASSERT_OK(err);
 }
 
@@ -855,6 +885,54 @@ TEST_F(hid_bpf, test_hid_attach_flags)
 	ASSERT_EQ(buf[3], 3);
 }
 
+static bool is_using_driver(struct __test_metadata *_metadata, struct uhid_device *hid,
+			    const char *driver)
+{
+	char driver_line[512];
+	char uevent[1024];
+	char temp[512];
+	int fd, nread;
+	bool found = false;
+
+	sprintf(uevent, "/sys/bus/hid/devices/%04X:%04X:%04X.%04X/uevent",
+		hid->bus, hid->vid, hid->pid, hid->hid_id);
+
+	fd = open(uevent, O_RDONLY | O_NONBLOCK);
+	if (fd < 0) {
+		TH_LOG("couldn't open '%s': %d, %d", uevent, fd, errno);
+		return false;
+	}
+
+	sprintf(driver_line, "DRIVER=%s", driver);
+
+	nread = read(fd, temp, ARRAY_SIZE(temp));
+	if (nread > 0 && (strstr(temp, driver_line)) != NULL)
+		found = true;
+
+	close(fd);
+
+	return found;
+}
+
+/*
+ * Attach hid_driver_probe to the given uhid device,
+ * check that the device is now using hid-generic.
+ */
+TEST_F(hid_bpf, test_hid_driver_probe)
+{
+	const struct test_program progs[] = {
+		{
+			.name = "hid_test_driver_probe",
+		},
+	};
+
+	ASSERT_TRUE(is_using_driver(_metadata, &self->hid, "apple"));
+
+	LOAD_PROGRAMS(progs);
+
+	ASSERT_TRUE(is_using_driver(_metadata, &self->hid, "hid-generic"));
+}
+
 /*
  * Attach hid_rdesc_fixup to the given uhid device,
  * retrieve and open the matching hidraw node,
diff --git a/tools/testing/selftests/hid/progs/hid.c b/tools/testing/selftests/hid/progs/hid.c
index 5ecc845ef792..9b22e9a0e658 100644
--- a/tools/testing/selftests/hid/progs/hid.c
+++ b/tools/testing/selftests/hid/progs/hid.c
@@ -598,3 +598,15 @@ SEC(".struct_ops.link")
 struct hid_bpf_ops test_infinite_loop_input_report = {
 	.hid_device_event = (void *)hid_test_infinite_loop_input_report,
 };
+
+SEC("?struct_ops.s/hid_rdesc_fixup")
+int BPF_PROG(hid_test_driver_probe, struct hid_bpf_ctx *hid_ctx)
+{
+	hid_ctx->hid->quirks |= HID_QUIRK_IGNORE_SPECIAL_DRIVER;
+	return 0;
+}
+
+SEC(".struct_ops.link")
+struct hid_bpf_ops test_driver_probe = {
+	.hid_rdesc_fixup = (void *)hid_test_driver_probe,
+};
diff --git a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
index e5db897586bb..1a645684a117 100644
--- a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
+++ b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
@@ -84,10 +84,14 @@ struct hid_bpf_ops {
 	struct hid_device *hdev;
 };
 
+#define BIT(n) (1U << n)
+
 #ifndef BPF_F_BEFORE
-#define BPF_F_BEFORE (1U << 3)
+#define BPF_F_BEFORE BIT(3)
 #endif
 
+#define HID_QUIRK_IGNORE_SPECIAL_DRIVER		BIT(22)
+
 /* following are kfuncs exported by HID for HID-BPF */
 extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx,
 			      unsigned int offset,

-- 
2.46.0

[PATCH HID v2 10/11] HID: add quirk to prevent hid-input to be used

[Benjamin Tissoires]

This is useful when you want to handle the device entirely from BPF
without exposing the automatic input nodes.

This is useful in case we want to have a third party program that directly
talks to the hidraw node and we don't want regular input events to be
emitted. This third party program can load a BPF program that instructs
hid-generic to rebind on the device with hidraw only and then open the
hidraw node itself.

When the application is closed, the BPF program is unloaded and the normal
driver takes back the control of the device.

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>

---

changes in v2:
- rely on hdev->quirks instead of a new struct
---
 drivers/hid/hid-core.c | 2 ++
 include/linux/hid.h    | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index bf63e2819baf..35cefd7167aa 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2217,6 +2217,8 @@ int hid_connect(struct hid_device *hdev, unsigned int connect_mask)
 		connect_mask |= (HID_CONNECT_HIDDEV_FORCE | HID_CONNECT_HIDDEV);
 	if (hdev->quirks & HID_QUIRK_HIDINPUT_FORCE)
 		connect_mask |= HID_CONNECT_HIDINPUT_FORCE;
+	if (hdev->quirks & HID_QUIRK_IGNORE_HIDINPUT)
+		connect_mask &= ~HID_CONNECT_HIDINPUT;
 	if (hdev->bus != BUS_USB)
 		connect_mask &= ~HID_CONNECT_HIDDEV;
 	if (hid_hiddev(hdev))
diff --git a/include/linux/hid.h b/include/linux/hid.h
index b3a9586363c9..b6bad03649f6 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -360,6 +360,7 @@ struct hid_item {
  * | @HID_QUIRK_HAVE_SPECIAL_DRIVER:
  * | @HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE:
  * | @HID_QUIRK_IGNORE_SPECIAL_DRIVER
+ * | @HID_QUIRK_IGNORE_HIDINPUT
  * | @HID_QUIRK_FULLSPEED_INTERVAL:
  * | @HID_QUIRK_NO_INIT_REPORTS:
  * | @HID_QUIRK_NO_IGNORE:
@@ -386,6 +387,7 @@ struct hid_item {
 #define HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE	BIT(20)
 #define HID_QUIRK_NOINVERT			BIT(21)
 #define HID_QUIRK_IGNORE_SPECIAL_DRIVER		BIT(22)
+#define HID_QUIRK_IGNORE_HIDINPUT		BIT(23)
 #define HID_QUIRK_FULLSPEED_INTERVAL		BIT(28)
 #define HID_QUIRK_NO_INIT_REPORTS		BIT(29)
 #define HID_QUIRK_NO_IGNORE			BIT(30)

-- 
2.46.0

[PATCH HID v2 11/11] selftests/hid: add test to disable hid-input

[Benjamin Tissoires]

Add a test for the newly enabled feature to control the connect_mask
of hid-generic.

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>

---

changes in v2:
- amended for the new API
---
 tools/testing/selftests/hid/hid_bpf.c              | 60 +++++++++++++++++++++-
 tools/testing/selftests/hid/progs/hid.c            |  1 +
 .../testing/selftests/hid/progs/hid_bpf_helpers.h  |  1 +
 3 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/hid/hid_bpf.c b/tools/testing/selftests/hid/hid_bpf.c
index edc061b38528..41cacc30ef8b 100644
--- a/tools/testing/selftests/hid/hid_bpf.c
+++ b/tools/testing/selftests/hid/hid_bpf.c
@@ -4,6 +4,38 @@
 #include "hid_common.h"
 #include <bpf/bpf.h>
 
+static const __u8 mouse_rdesc[] = {
+	0x05, 0x01,  /* .Usage Page (Generic Desktop)        0  */
+	0x09, 0x02,  /* .Usage (Mouse)                       2  */
+	0xa1, 0x01,  /* .Collection (Application)            4  */
+	0x09, 0x02,  /* ..Usage (Mouse)                      6  */
+	0xa1, 0x02,  /* ..Collection (Logical)               8  */
+	0x09, 0x01,  /* ...Usage (Pointer)                   10 */
+	0xa1, 0x00,  /* ...Collection (Physical)             12 */
+	0x05, 0x09,  /* ....Usage Page (Button)              14 */
+	0x19, 0x01,  /* ....Usage Minimum (1)                16 */
+	0x29, 0x03,  /* ....Usage Maximum (3)                18 */
+	0x15, 0x00,  /* ....Logical Minimum (0)              20 */
+	0x25, 0x01,  /* ....Logical Maximum (1)              22 */
+	0x75, 0x01,  /* ....Report Size (1)                  24 */
+	0x95, 0x03,  /* ....Report Count (3)                 26 */
+	0x81, 0x02,  /* ....Input (Data,Var,Abs)             28 */
+	0x75, 0x05,  /* ....Report Size (5)                  30 */
+	0x95, 0x01,  /* ....Report Count (1)                 32 */
+	0x81, 0x03,  /* ....Input (Cnst,Var,Abs)             34 */
+	0x05, 0x01,  /* ....Usage Page (Generic Desktop)     36 */
+	0x09, 0x30,  /* ....Usage (X)                        38 */
+	0x09, 0x31,  /* ....Usage (Y)                        40 */
+	0x15, 0x81,  /* ....Logical Minimum (-127)           42 */
+	0x25, 0x7f,  /* ....Logical Maximum (127)            44 */
+	0x75, 0x08,  /* ....Report Size (8)                  46 */
+	0x95, 0x02,  /* ....Report Count (2)                 48 */
+	0x81, 0x06,  /* ....Input (Data,Var,Rel)             50 */
+	0xc0,        /* ...End Collection                    52 */
+	0xc0,        /* ..End Collection                     53 */
+	0xc0,        /* .End Collection                      54 */
+};
+
 struct hid_hw_request_syscall_args {
 	__u8 data[10];
 	unsigned int hid;
@@ -59,6 +91,8 @@ struct specific_device {
 	__u16 bus;
 	__u32 vid;
 	__u32 pid;
+	const __u8 *rdesc;
+	const size_t rdesc_size;
 };
 
 FIXTURE_SETUP(hid_bpf)
@@ -72,11 +106,15 @@ FIXTURE_SETUP(hid_bpf)
 		.bus = BUS_BLUETOOTH,
 		.vid = 0x05ac,  /* USB_VENDOR_ID_APPLE */
 		.pid = 0x022c,  /* USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI */
+		.rdesc = mouse_rdesc,
+		.rdesc_size = sizeof(mouse_rdesc),
 	}, {
 		.test_name = "*",
 		.bus = BUS_USB,
 		.vid = 0x0001,
 		.pid = 0x0a36,
+		.rdesc = rdesc,
+		.rdesc_size = sizeof(rdesc),
 	}};
 
 	for (int i = 0; i < ARRAY_SIZE(devices); i++) {
@@ -88,7 +126,7 @@ FIXTURE_SETUP(hid_bpf)
 	ASSERT_OK_PTR(match);
 
 	err = setup_uhid(_metadata, &self->hid, match->bus, match->vid, match->pid,
-			 rdesc, sizeof(rdesc));
+			 match->rdesc, match->rdesc_size);
 	ASSERT_OK(err);
 }
 
@@ -914,6 +952,24 @@ static bool is_using_driver(struct __test_metadata *_metadata, struct uhid_devic
 	return found;
 }
 
+static bool has_hid_input(struct uhid_device *hid)
+{
+	char input[1024];
+	DIR *d;
+
+	sprintf(input, "/sys/bus/hid/devices/%04X:%04X:%04X.%04X/input",
+		hid->bus, hid->vid, hid->pid, hid->hid_id);
+
+	d = opendir(input);
+	if (d) {
+		closedir(d);
+
+		return true;
+	}
+
+	return false;
+}
+
 /*
  * Attach hid_driver_probe to the given uhid device,
  * check that the device is now using hid-generic.
@@ -927,10 +983,12 @@ TEST_F(hid_bpf, test_hid_driver_probe)
 	};
 
 	ASSERT_TRUE(is_using_driver(_metadata, &self->hid, "apple"));
+	ASSERT_TRUE(has_hid_input(&self->hid)) TH_LOG("input node not found");
 
 	LOAD_PROGRAMS(progs);
 
 	ASSERT_TRUE(is_using_driver(_metadata, &self->hid, "hid-generic"));
+	ASSERT_FALSE(has_hid_input(&self->hid)) TH_LOG("input node unexpectly found");
 }
 
 /*
diff --git a/tools/testing/selftests/hid/progs/hid.c b/tools/testing/selftests/hid/progs/hid.c
index 9b22e9a0e658..7e24bec8ef43 100644
--- a/tools/testing/selftests/hid/progs/hid.c
+++ b/tools/testing/selftests/hid/progs/hid.c
@@ -603,6 +603,7 @@ SEC("?struct_ops.s/hid_rdesc_fixup")
 int BPF_PROG(hid_test_driver_probe, struct hid_bpf_ctx *hid_ctx)
 {
 	hid_ctx->hid->quirks |= HID_QUIRK_IGNORE_SPECIAL_DRIVER;
+	hid_ctx->hid->quirks |= HID_QUIRK_IGNORE_HIDINPUT;
 	return 0;
 }
 
diff --git a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
index 1a645684a117..9c3cfd61d992 100644
--- a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
+++ b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
@@ -91,6 +91,7 @@ struct hid_bpf_ops {
 #endif
 
 #define HID_QUIRK_IGNORE_SPECIAL_DRIVER		BIT(22)
+#define HID_QUIRK_IGNORE_HIDINPUT		BIT(23)
 
 /* following are kfuncs exported by HID for HID-BPF */
 extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx,

-- 
2.46.0

Re: [PATCH HID v2 02/11] HID: core: save one kmemdup during .probe()

[Peter Hutterer]

On Tue, Sep 10, 2024 at 11:43:38PM +0900, Benjamin Tissoires wrote:
> Turns out the first kmemdup is only required for the .report_fixup()
> driver callback. There is no need to do two kmemdup() in a raw in case

typo: "in a row"?

Cheers,
  Peter

> .report_fixup() is not present.
> 
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> 
> ---
> 
> new in v2
> ---
>  drivers/hid/hid-core.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index a5f5415571cb..172746a082f9 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1206,7 +1206,7 @@ int hid_open_report(struct hid_device *device)
>  	struct hid_item item;
>  	unsigned int size;
>  	const __u8 *start;
> -	__u8 *buf;
> +	__u8 *buf = NULL;
>  	const __u8 *end;
>  	const __u8 *next;
>  	int ret;
> @@ -1227,14 +1227,18 @@ int hid_open_report(struct hid_device *device)
>  	if (WARN_ON(!start))
>  		return -ENODEV;
>  
> -	buf = kmemdup(start, size, GFP_KERNEL);
> -	if (buf == NULL)
> -		return -ENOMEM;
> +	if (device->driver->report_fixup) {
> +		/*
> +		 * device->driver->report_fixup() needs to work
> +		 * on a copy of our report descriptor so it can
> +		 * change it.
> +		 */
> +		buf = kmemdup(start, size, GFP_KERNEL);
> +		if (buf == NULL)
> +			return -ENOMEM;
>  
> -	if (device->driver->report_fixup)
>  		start = device->driver->report_fixup(device, buf, &size);
> -	else
> -		start = buf;
> +	}
>  
>  	start = kmemdup(start, size, GFP_KERNEL);
>  	kfree(buf);
> 
> -- 
> 2.46.0
> 

Re: [PATCH HID v2 08/11] HID: add per device quirk to force bind to hid-generic

[Peter Hutterer]

On Tue, Sep 10, 2024 at 11:43:44PM +0900, Benjamin Tissoires wrote:
> We already have the possibility to force not binding to hid-generic and
> rely on a dedicated driver, but we couldn't do the other way around.
> 
> This is useful for BPF programs where we are fixing the report descriptor
> and the events, but want to avoid a specialized driver to come after BPF
> which would unwind everything that is done there.
> 
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> 
> ---
> 
> changes in v2:
> - rely on hdev->quirks for that instead of a new struct for hid_driver

I like this one a lot more than the previous approach, series is 

Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>

although for 01 and 06 this should be taken with a grain of salt :)

Cheers,
  Peter

> ---
>  drivers/hid/hid-core.c    | 5 +++--
>  drivers/hid/hid-generic.c | 3 +++
>  include/linux/hid.h       | 2 ++
>  3 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 37e52759a931..bf63e2819baf 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -2665,9 +2665,10 @@ static bool hid_check_device_match(struct hid_device *hdev,
>  	/*
>  	 * hid-generic implements .match(), so we must be dealing with a
>  	 * different HID driver here, and can simply check if
> -	 * hid_ignore_special_drivers is set or not.
> +	 * hid_ignore_special_drivers or HID_QUIRK_IGNORE_SPECIAL_DRIVER
> +	 * are set or not.
>  	 */
> -	return !hid_ignore_special_drivers;
> +	return !hid_ignore_special_drivers && !(hdev->quirks & HID_QUIRK_IGNORE_SPECIAL_DRIVER);
>  }
>  
>  static int __hid_device_probe(struct hid_device *hdev, struct hid_driver *hdrv)
> diff --git a/drivers/hid/hid-generic.c b/drivers/hid/hid-generic.c
> index f9db991d3c5a..88882c1bfffe 100644
> --- a/drivers/hid/hid-generic.c
> +++ b/drivers/hid/hid-generic.c
> @@ -40,6 +40,9 @@ static bool hid_generic_match(struct hid_device *hdev,
>  	if (ignore_special_driver)
>  		return true;
>  
> +	if (hdev->quirks & HID_QUIRK_IGNORE_SPECIAL_DRIVER)
> +		return true;
> +
>  	if (hdev->quirks & HID_QUIRK_HAVE_SPECIAL_DRIVER)
>  		return false;
>  
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index d41fa18f1e03..b3a9586363c9 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -359,6 +359,7 @@ struct hid_item {
>   * | @HID_QUIRK_NO_OUTPUT_REPORTS_ON_INTR_EP:
>   * | @HID_QUIRK_HAVE_SPECIAL_DRIVER:
>   * | @HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE:
> + * | @HID_QUIRK_IGNORE_SPECIAL_DRIVER
>   * | @HID_QUIRK_FULLSPEED_INTERVAL:
>   * | @HID_QUIRK_NO_INIT_REPORTS:
>   * | @HID_QUIRK_NO_IGNORE:
> @@ -384,6 +385,7 @@ struct hid_item {
>  #define HID_QUIRK_HAVE_SPECIAL_DRIVER		BIT(19)
>  #define HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE	BIT(20)
>  #define HID_QUIRK_NOINVERT			BIT(21)
> +#define HID_QUIRK_IGNORE_SPECIAL_DRIVER		BIT(22)
>  #define HID_QUIRK_FULLSPEED_INTERVAL		BIT(28)
>  #define HID_QUIRK_NO_INIT_REPORTS		BIT(29)
>  #define HID_QUIRK_NO_IGNORE			BIT(30)
> 
> -- 
> 2.46.0
> 

Re: (subset) [PATCH HID v2 00/11] HID: bpf: add a new hook to control hid-generic

[Benjamin Tissoires]

On Tue, 10 Sep 2024 23:43:36 +0900, Benjamin Tissoires wrote:
> This is a slight change from the fundamentals of HID-BPF.
> In theory, HID-BPF is abstract to the kernel itself, and makes
> only changes at the HID level (through report descriptors or
> events emitted to/from the device).
> 
> However, we have seen a few use cases where HID-BPF might interact with
> the running kernel when the target device is already handled by a
> specific device.
> 
> [...]

Applied to hid/hid.git (for-6.12/bpf), thanks!

[01/11] HID: bpf: move HID-BPF report descriptor fixup earlier
        https://git.kernel.org/hid/hid/c/f10a11b7b599
[02/11] HID: core: save one kmemdup during .probe()
        https://git.kernel.org/hid/hid/c/6941754dbbc7
[03/11] HID: core: remove one more kmemdup on .probe()
        https://git.kernel.org/hid/hid/c/4fe29f36d2a3
[04/11] HID: bpf: allow write access to quirks field in struct hid_device
        https://git.kernel.org/hid/hid/c/b722f588adc6
[05/11] selftests/hid: add dependency on hid_common.h
        https://git.kernel.org/hid/hid/c/3d816765e12e
[06/11] selftests/hid: cleanup C tests by adding a common struct uhid_device
        https://git.kernel.org/hid/hid/c/28023a0f99d1
[07/11] selftests/hid: allow to parametrize bus/vid/pid/rdesc on the test device
        https://git.kernel.org/hid/hid/c/10d3147f9bb1
[08/11] HID: add per device quirk to force bind to hid-generic
        https://git.kernel.org/hid/hid/c/d030f826ea47
[09/11] selftests/hid: add test for assigning a given device to hid-generic
        https://git.kernel.org/hid/hid/c/10929078201f

Cheers,
-- 
Benjamin Tissoires <bentiss@kernel.org>

Re: (subset) [PATCH HID v2 00/11] HID: bpf: add a new hook to control hid-generic

[Benjamin Tissoires]

On Sep 13 2024, Benjamin Tissoires wrote:
> On Tue, 10 Sep 2024 23:43:36 +0900, Benjamin Tissoires wrote:
> > This is a slight change from the fundamentals of HID-BPF.
> > In theory, HID-BPF is abstract to the kernel itself, and makes
> > only changes at the HID level (through report descriptors or
> > events emitted to/from the device).
> > 
> > However, we have seen a few use cases where HID-BPF might interact with
> > the running kernel when the target device is already handled by a
> > specific device.
> > 
> > [...]
> 
> Applied to hid/hid.git (for-6.12/bpf), thanks!
> 
> [01/11] HID: bpf: move HID-BPF report descriptor fixup earlier
>         https://git.kernel.org/hid/hid/c/f10a11b7b599
> [02/11] HID: core: save one kmemdup during .probe()
>         https://git.kernel.org/hid/hid/c/6941754dbbc7
> [03/11] HID: core: remove one more kmemdup on .probe()
>         https://git.kernel.org/hid/hid/c/4fe29f36d2a3
> [04/11] HID: bpf: allow write access to quirks field in struct hid_device
>         https://git.kernel.org/hid/hid/c/b722f588adc6
> [05/11] selftests/hid: add dependency on hid_common.h
>         https://git.kernel.org/hid/hid/c/3d816765e12e
> [06/11] selftests/hid: cleanup C tests by adding a common struct uhid_device
>         https://git.kernel.org/hid/hid/c/28023a0f99d1
> [07/11] selftests/hid: allow to parametrize bus/vid/pid/rdesc on the test device
>         https://git.kernel.org/hid/hid/c/10d3147f9bb1
> [08/11] HID: add per device quirk to force bind to hid-generic
>         https://git.kernel.org/hid/hid/c/d030f826ea47
> [09/11] selftests/hid: add test for assigning a given device to hid-generic
>         https://git.kernel.org/hid/hid/c/10929078201f
> 

Just for completeness, I've dropped 10/11 and 11/11 when applying the
series because even if they are working it's unclear if the use case is
rock solid, like the first one is.

The patches are still on the LKML, so if anyone believes they required
it, we can alwasy pull them in later.

Cheers,
Benjamin

Re: (subset) [PATCH HID v2 00/11] HID: bpf: add a new hook to control hid-generic

[Benjamin Tissoires]

On Sep 13 2024, Benjamin Tissoires wrote:
> On Sep 13 2024, Benjamin Tissoires wrote:
> > On Tue, 10 Sep 2024 23:43:36 +0900, Benjamin Tissoires wrote:
> > > This is a slight change from the fundamentals of HID-BPF.
> > > In theory, HID-BPF is abstract to the kernel itself, and makes
> > > only changes at the HID level (through report descriptors or
> > > events emitted to/from the device).
> > > 
> > > However, we have seen a few use cases where HID-BPF might interact with
> > > the running kernel when the target device is already handled by a
> > > specific device.
> > > 
> > > [...]
> > 
> > Applied to hid/hid.git (for-6.12/bpf), thanks!
> > 
> > [01/11] HID: bpf: move HID-BPF report descriptor fixup earlier
> >         https://git.kernel.org/hid/hid/c/f10a11b7b599
> > [02/11] HID: core: save one kmemdup during .probe()
> >         https://git.kernel.org/hid/hid/c/6941754dbbc7
> > [03/11] HID: core: remove one more kmemdup on .probe()
> >         https://git.kernel.org/hid/hid/c/4fe29f36d2a3
> > [04/11] HID: bpf: allow write access to quirks field in struct hid_device
> >         https://git.kernel.org/hid/hid/c/b722f588adc6
> > [05/11] selftests/hid: add dependency on hid_common.h
> >         https://git.kernel.org/hid/hid/c/3d816765e12e
> > [06/11] selftests/hid: cleanup C tests by adding a common struct uhid_device
> >         https://git.kernel.org/hid/hid/c/28023a0f99d1
> > [07/11] selftests/hid: allow to parametrize bus/vid/pid/rdesc on the test device
> >         https://git.kernel.org/hid/hid/c/10d3147f9bb1
> > [08/11] HID: add per device quirk to force bind to hid-generic
> >         https://git.kernel.org/hid/hid/c/d030f826ea47
> > [09/11] selftests/hid: add test for assigning a given device to hid-generic
> >         https://git.kernel.org/hid/hid/c/10929078201f
> > 
> 
> Just for completeness, I've dropped 10/11 and 11/11 when applying the
> series because even if they are working it's unclear if the use case is
> rock solid, like the first one is.
> 
> The patches are still on the LKML, so if anyone believes they required
> it, we can alwasy pull them in later.

And I just dropped them from for-next, I had some KASAN issues in the
testsuite, meaning that something is off in the memory allocation/free.

Cheers,
Benjamin