Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

Revert "HID: bpf: allow write access to quirks field in struct hid_device"

This reverts commit 6fd47effe92b, and the related self-test update
commit e14e0eaeb040 ("selftests/hid: add test for assigning a given
device to hid-generic").

It results in things like the scroll wheel on Logitech mice not working
after a reboot due to the kernel being confused about the state of the
high-resolution mode.

Quoting Benjamin Tissoires:
"The idea of 6fd47effe92b was to be able to call hid_bpf_rdesc_fixup()
once per reprobe of the device.

However, because the bpf filter can now change the quirk value, the
call had to be moved before the driver gets bound (which was
previously ensuring the unicity of the call).

The net effect is that now, in the case hid-generic gets loaded first
and then the specific driver gets loaded once the disk is available,
the value of ->quirks is not reset, but kept to the value that was set
by hid-generic (HID_QUIRK_INPUT_PER_APP).

Once hid-logitech-hidpp kicks in, that quirk is now set, which creates
two inputs for the single mouse: one keyboard for fancy shortcuts, and
one mouse node.

However, hid-logitech-hidpp expects only one input node to be attached
(it stores it into hidpp->input), and when a wheel event is received,
because there is some processing with high-resolution wheel events,
the wheel event is injected into hidpp->input.

And of course, when HID_QUIRK_INPUT_PER_APP is set, hidpp->input gets
the keyboard node, which doesn't have wheel event type, and the events
are ignored"

Reported-and-bisected-by: Mike Galbraith <efault@gmx.de>
Link: https://lore.kernel.org/all/CAHk-=wiUkQM3uheit2cNM0Y0OOY5qqspJgC8LkmOkJ2p2LDxcw@mail.gmail.com/
Acked-by: Benjamin Tissoires <bentiss@kernel.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

+4 -106
-1
drivers/hid/bpf/hid_bpf_struct_ops.c
··· 79 79 WRITE_RANGE(hid_device, name, true), 80 80 WRITE_RANGE(hid_device, uniq, true), 81 81 WRITE_RANGE(hid_device, phys, true), 82 - WRITE_RANGE(hid_device, quirks, false), 83 82 }; 84 83 #undef WRITE_RANGE 85 84 const struct btf_type *state = NULL;
+2 -9
drivers/hid/hid-core.c
··· 2692 2692 int ret; 2693 2693 2694 2694 if (!hdev->bpf_rsize) { 2695 - unsigned int quirks; 2696 - 2697 - /* reset the quirks that has been previously set */ 2698 - quirks = hid_lookup_quirk(hdev); 2699 - hdev->quirks = quirks; 2700 - 2701 2695 /* in case a bpf program gets detached, we need to free the old one */ 2702 2696 hid_free_bpf_rdesc(hdev); 2703 2697 ··· 2701 2707 /* call_hid_bpf_rdesc_fixup will always return a valid pointer */ 2702 2708 hdev->bpf_rdesc = call_hid_bpf_rdesc_fixup(hdev, hdev->dev_rdesc, 2703 2709 &hdev->bpf_rsize); 2704 - if (quirks ^ hdev->quirks) 2705 - hid_info(hdev, "HID-BPF toggled quirks on the device: %04x", 2706 - quirks ^ hdev->quirks); 2707 2710 } 2708 2711 2709 2712 if (!hid_check_device_match(hdev, hdrv, &id)) ··· 2710 2719 if (!hdev->devres_group_id) 2711 2720 return -ENOMEM; 2712 2721 2722 + /* reset the quirks that has been previously set */ 2723 + hdev->quirks = hid_lookup_quirk(hdev); 2713 2724 hdev->driver = hdrv; 2714 2725 2715 2726 if (hdrv->probe) {
+1 -79
tools/testing/selftests/hid/hid_bpf.c
··· 54 54 hid_bpf_teardown(_metadata, self, variant); \ 55 55 } while (0) 56 56 57 - struct specific_device { 58 - const char test_name[64]; 59 - __u16 bus; 60 - __u32 vid; 61 - __u32 pid; 62 - }; 63 - 64 57 FIXTURE_SETUP(hid_bpf) 65 58 { 66 - const struct specific_device *match = NULL; 67 59 int err; 68 60 69 - const struct specific_device devices[] = { 70 - { 71 - .test_name = "test_hid_driver_probe", 72 - .bus = BUS_BLUETOOTH, 73 - .vid = 0x05ac, /* USB_VENDOR_ID_APPLE */ 74 - .pid = 0x022c, /* USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI */ 75 - }, { 76 - .test_name = "*", 77 - .bus = BUS_USB, 78 - .vid = 0x0001, 79 - .pid = 0x0a36, 80 - }}; 81 - 82 - for (int i = 0; i < ARRAY_SIZE(devices); i++) { 83 - match = &devices[i]; 84 - if (!strncmp(_metadata->name, devices[i].test_name, sizeof(devices[i].test_name))) 85 - break; 86 - } 87 - 88 - ASSERT_OK_PTR(match); 89 - 90 - err = setup_uhid(_metadata, &self->hid, match->bus, match->vid, match->pid, 91 - rdesc, sizeof(rdesc)); 61 + err = setup_uhid(_metadata, &self->hid, BUS_USB, 0x0001, 0x0a36, rdesc, sizeof(rdesc)); 92 62 ASSERT_OK(err); 93 63 } 94 64 ··· 853 883 ASSERT_EQ(buf[1], 1); 854 884 ASSERT_EQ(buf[2], 2); 855 885 ASSERT_EQ(buf[3], 3); 856 - } 857 - 858 - static bool is_using_driver(struct __test_metadata *_metadata, struct uhid_device *hid, 859 - const char *driver) 860 - { 861 - char driver_line[512]; 862 - char uevent[1024]; 863 - char temp[512]; 864 - int fd, nread; 865 - bool found = false; 866 - 867 - sprintf(uevent, "/sys/bus/hid/devices/%04X:%04X:%04X.%04X/uevent", 868 - hid->bus, hid->vid, hid->pid, hid->hid_id); 869 - 870 - fd = open(uevent, O_RDONLY | O_NONBLOCK); 871 - if (fd < 0) { 872 - TH_LOG("couldn't open '%s': %d, %d", uevent, fd, errno); 873 - return false; 874 - } 875 - 876 - sprintf(driver_line, "DRIVER=%s", driver); 877 - 878 - nread = read(fd, temp, ARRAY_SIZE(temp)); 879 - if (nread > 0 && (strstr(temp, driver_line)) != NULL) 880 - found = true; 881 - 882 - close(fd); 883 - 884 - return found; 885 - } 886 - 887 - /* 888 - * Attach hid_driver_probe to the given uhid device, 889 - * check that the device is now using hid-generic. 890 - */ 891 - TEST_F(hid_bpf, test_hid_driver_probe) 892 - { 893 - const struct test_program progs[] = { 894 - { 895 - .name = "hid_test_driver_probe", 896 - }, 897 - }; 898 - 899 - ASSERT_TRUE(is_using_driver(_metadata, &self->hid, "apple")); 900 - 901 - LOAD_PROGRAMS(progs); 902 - 903 - ASSERT_TRUE(is_using_driver(_metadata, &self->hid, "hid-generic")); 904 886 } 905 887 906 888 /*
-12
tools/testing/selftests/hid/progs/hid.c
··· 598 598 struct hid_bpf_ops test_infinite_loop_input_report = { 599 599 .hid_device_event = (void *)hid_test_infinite_loop_input_report, 600 600 }; 601 - 602 - SEC("?struct_ops.s/hid_rdesc_fixup") 603 - int BPF_PROG(hid_test_driver_probe, struct hid_bpf_ctx *hid_ctx) 604 - { 605 - hid_ctx->hid->quirks |= HID_QUIRK_IGNORE_SPECIAL_DRIVER; 606 - return 0; 607 - } 608 - 609 - SEC(".struct_ops.link") 610 - struct hid_bpf_ops test_driver_probe = { 611 - .hid_rdesc_fixup = (void *)hid_test_driver_probe, 612 - };
+1 -5
tools/testing/selftests/hid/progs/hid_bpf_helpers.h
··· 84 84 struct hid_device *hdev; 85 85 }; 86 86 87 - #define BIT(n) (1U << n) 88 - 89 87 #ifndef BPF_F_BEFORE 90 - #define BPF_F_BEFORE BIT(3) 88 + #define BPF_F_BEFORE (1U << 3) 91 89 #endif 92 - 93 - #define HID_QUIRK_IGNORE_SPECIAL_DRIVER BIT(22) 94 90 95 91 /* following are kfuncs exported by HID for HID-BPF */ 96 92 extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx,