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