linux-kernel - Re: [PATCH HID v3 0/9] HID: bpf: add a new hook to control hid-generic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1fd8fd72-2439-4497-a18a-fdc06628eb1e@linuxfoundation.org>
Date: Wed, 2 Oct 2024 16:43:14 -0600
From: Shuah Khan <skhan@...uxfoundation.org>
To: Benjamin Tissoires <bentiss@...nel.org>, Jiri Kosina <jikos@...nel.org>,
 Peter Hutterer <peter.hutterer@...-t.net>, Vicki Pfau <vi@...rift.com>,
 Shuah Khan <shuah@...nel.org>
Cc: linux-input@...r.kernel.org, linux-kselftest@...r.kernel.org,
 linux-kernel@...r.kernel.org, bpf@...r.kernel.org,
 Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: [PATCH HID v3 0/9] HID: bpf: add a new hook to control
 hid-generic

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@...nel.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@...uxfoundation.org>

thanks,
-- Shuah