mbox series

[HID,v3,0/9] HID: bpf: add a new hook to control hid-generic

Message ID 20241001-hid-bpf-hid-generic-v3-0-2ef1019468df@kernel.org (mailing list archive)
Headers show
Series HID: bpf: add a new hook to control hid-generic | expand

Message

Benjamin Tissoires Oct. 1, 2024, 2:30 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.

This series aims at tackling this problem:
- first, we promote hid_bpf_report_descriptor_fixup to be called before
  any driver is even matched for the device
- then we allow hdev->quirks to be written during report_fixup and add a
  new quirk to force hid-core to ignore any non hid-generic driver.

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.

This branch is on top of the for-6.12/upstream-fixes branch of hid.git.

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

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
Changes in v3:
- dropped the last 2 patches with hid-input control, as I'm not 100%
  sure of it
- changed the first patch to avoid a double free on cleanup of a device
  when a HID-BPF program was attached
- kept Peter's rev-by for all but patches 1 and 6
- Link to v2: https://lore.kernel.org/r/20240910-hid-bpf-hid-generic-v2-0-083dfc189e97@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 (9):
      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

 drivers/hid/bpf/hid_bpf_dispatch.c                 |   9 +-
 drivers/hid/bpf/hid_bpf_struct_ops.c               |   1 +
 drivers/hid/hid-core.c                             |  84 +++++++++---
 drivers/hid/hid-generic.c                          |   3 +
 include/linux/hid.h                                |  20 +--
 include/linux/hid_bpf.h                            |  11 +-
 tools/testing/selftests/hid/Makefile               |   2 +-
 tools/testing/selftests/hid/hid_bpf.c              | 151 ++++++++++++++-------
 tools/testing/selftests/hid/hid_common.h           | 112 ++++++++++-----
 tools/testing/selftests/hid/hidraw.c               |  36 ++---
 tools/testing/selftests/hid/progs/hid.c            |  12 ++
 .../testing/selftests/hid/progs/hid_bpf_helpers.h  |   6 +-
 12 files changed, 296 insertions(+), 151 deletions(-)
---
base-commit: acd5f76fd5292c91628e04da83e8b78c986cfa2b
change-id: 20240829-hid-bpf-hid-generic-61579f5b5945

Best regards,

Comments

Shuah Khan Oct. 2, 2024, 10:43 p.m. UTC | #1
On 10/1/24 08:30, 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.
> 
> 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.
> 
> This series aims at tackling this problem:
> - first, we promote hid_bpf_report_descriptor_fixup to be called before
>    any driver is even matched for the device
> - then we allow hdev->quirks to be written during report_fixup and add a
>    new quirk to force hid-core to ignore any non hid-generic driver.
> 
> 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.
> 
> This branch is on top of the for-6.12/upstream-fixes branch of hid.git.
> 
> [0] https://gitlab.freedesktop.org/libinput/libinput/-/issues/1014
> 
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> ---
> Changes in v3:
> - dropped the last 2 patches with hid-input control, as I'm not 100%
>    sure of it
> - changed the first patch to avoid a double free on cleanup of a device
>    when a HID-BPF program was attached
> - kept Peter's rev-by for all but patches 1 and 6
> - Link to v2: https://lore.kernel.org/r/20240910-hid-bpf-hid-generic-v2-0-083dfc189e97@kernel.org
> 

>        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

> 
>   drivers/hid/bpf/hid_bpf_dispatch.c                 |   9 +-
>   drivers/hid/bpf/hid_bpf_struct_ops.c               |   1 +
>   drivers/hid/hid-core.c                             |  84 +++++++++---
>   drivers/hid/hid-generic.c                          |   3 +
>   include/linux/hid.h                                |  20 +--
>   include/linux/hid_bpf.h                            |  11 +-
>   tools/testing/selftests/hid/Makefile               |   2 +-
>   tools/testing/selftests/hid/hid_bpf.c              | 151 ++++++++++++++-------
>   tools/testing/selftests/hid/hid_common.h           | 112 ++++++++++-----
>   tools/testing/selftests/hid/hidraw.c               |  36 ++---
>   tools/testing/selftests/hid/progs/hid.c            |  12 ++
>   .../testing/selftests/hid/progs/hid_bpf_helpers.h  |   6 +-
>   12 files changed, 296 insertions(+), 151 deletions(-)

I am assuming selftests go with the driver changes.
For selftests:

Acked-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah
Benjamin Tissoires Oct. 4, 2024, 2:12 p.m. UTC | #2
On Tue, 01 Oct 2024 16:30:04 +0200, 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.13/bpf), thanks!

[1/9] HID: bpf: move HID-BPF report descriptor fixup earlier
      https://git.kernel.org/hid/hid/c/8b7fd6a15f8c
[2/9] HID: core: save one kmemdup during .probe()
      https://git.kernel.org/hid/hid/c/52cd1906ef6b
[3/9] HID: core: remove one more kmemdup on .probe()
      https://git.kernel.org/hid/hid/c/7316fef4b993
[4/9] HID: bpf: allow write access to quirks field in struct hid_device
      https://git.kernel.org/hid/hid/c/6fd47effe92b
[5/9] selftests/hid: add dependency on hid_common.h
      https://git.kernel.org/hid/hid/c/0b838d768ccd
[6/9] selftests/hid: cleanup C tests by adding a common struct uhid_device
      https://git.kernel.org/hid/hid/c/4fb41dfde069
[7/9] selftests/hid: allow to parametrize bus/vid/pid/rdesc on the test device
      https://git.kernel.org/hid/hid/c/72c55473fc8c
[8/9] HID: add per device quirk to force bind to hid-generic
      https://git.kernel.org/hid/hid/c/645c224ac5f6
[9/9] selftests/hid: add test for assigning a given device to hid-generic
      https://git.kernel.org/hid/hid/c/e14e0eaeb040

Cheers,