mbox series

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

Message ID 20240910-hid-bpf-hid-generic-v2-0-083dfc189e97@kernel.org (mailing list archive)
Headers show
Series HID: bpf: add a new hook to control hid-generic | expand

Message

Benjamin Tissoires Sept. 10, 2024, 2:43 p.m. UTC
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,

Comments

Benjamin Tissoires Sept. 13, 2024, 1:38 p.m. UTC | #1
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 Sept. 13, 2024, 1:47 p.m. UTC | #2
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
Benjamin Tissoires Sept. 14, 2024, 5:26 a.m. UTC | #3
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