Skip to content

Commit 578e1b9

Browse files
Rongronggg9Benjamin Tissoires
authored andcommitted
HID: bpf: abort dispatch if device destroyed
The current HID bpf implementation assumes no output report/request will go through it after hid_bpf_destroy_device() has been called. This leads to a bug that unplugging certain types of HID devices causes a cleaned- up SRCU to be accessed. The bug was previously a hidden failure until a recent x86 percpu change [1] made it access not-present pages. The bug will be triggered if the conditions below are met: A) a device under the driver has some LEDs on B) hid_ll_driver->request() is uninplemented (e.g., logitech-djreceiver) If condition A is met, hidinput_led_worker() is always scheduled *after* hid_bpf_destroy_device(). hid_destroy_device ` hid_bpf_destroy_device ` cleanup_srcu_struct(&hdev->bpf.srcu) ` hid_remove_device ` ... ` led_classdev_unregister ` led_trigger_set(led_cdev, NULL) ` led_set_brightness(led_cdev, LED_OFF) ` ... ` input_inject_event ` input_event_dispose ` hidinput_input_event ` schedule_work(&hid->led_work) [hidinput_led_worker] This is fine when condition B is not met, where hidinput_led_worker() calls hid_ll_driver->request(). This is the case for most HID drivers, which implement it or use the generic one from usbhid. The driver itself or an underlying driver will then abort processing the request. Otherwise, hidinput_led_worker() tries hid_hw_output_report() and leads to the bug. hidinput_led_worker ` hid_hw_output_report ` dispatch_hid_bpf_output_report ` srcu_read_lock(&hdev->bpf.srcu) ` srcu_read_unlock(&hdev->bpf.srcu, idx) The bug has existed since the introduction [2] of dispatch_hid_bpf_output_report(). However, the same bug also exists in dispatch_hid_bpf_raw_requests(), and I've reproduced (no visible effect because of the lack of [1], but confirmed bpf.destroyed == 1) the bug against the commit (i.e., the Fixes:) introducing the function. This is because hidinput_led_worker() falls back to hid_hw_raw_request() when hid_ll_driver->output_report() is uninplemented (e.g., logitech- djreceiver). hidinput_led_worker ` hid_hw_output_report: -ENOSYS ` hid_hw_raw_request ` dispatch_hid_bpf_raw_requests ` srcu_read_lock(&hdev->bpf.srcu) ` srcu_read_unlock(&hdev->bpf.srcu, idx) Fix the issue by returning early in the two mentioned functions if hid_bpf has been marked as destroyed. Though dispatch_hid_bpf_device_event() handles input events, and there is no evidence that it may be called after the destruction, the same check, as a safety net, is also added to it to maintain the consistency among all dispatch functions. The impact of the bug on other architectures is unclear. Even if it acts as a hidden failure, this is still dangerous because it corrupts whatever is on the address calculated by SRCU. Thus, CC'ing the stable list. [1]: commit 9d7de2a ("x86/percpu/64: Use relative percpu offsets") [2]: commit 9286675 ("HID: bpf: add HID-BPF hooks for hid_hw_output_report") Closes: https://lore.kernel.org/all/20250506145548.GGaBoi9Jzp3aeJizTR@fat_crate.local/ Fixes: 8bd0488 ("HID: bpf: add HID-BPF hooks for hid_hw_raw_requests") Cc: stable@vger.kernel.org Signed-off-by: Rong Zhang <i@rong.moe> Tested-by: Petr Tesarik <petr@tesarici.cz> Link: https://patch.msgid.link/20250512152420.87441-1-i@rong.moe Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
1 parent fa9fdee commit 578e1b9

File tree

1 file changed

+9
-0
lines changed

1 file changed

+9
-0
lines changed

drivers/hid/bpf/hid_bpf_dispatch.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ dispatch_hid_bpf_device_event(struct hid_device *hdev, enum hid_report_type type
3838
struct hid_bpf_ops *e;
3939
int ret;
4040

41+
if (unlikely(hdev->bpf.destroyed))
42+
return ERR_PTR(-ENODEV);
43+
4144
if (type >= HID_REPORT_TYPES)
4245
return ERR_PTR(-EINVAL);
4346

@@ -93,6 +96,9 @@ int dispatch_hid_bpf_raw_requests(struct hid_device *hdev,
9396
struct hid_bpf_ops *e;
9497
int ret, idx;
9598

99+
if (unlikely(hdev->bpf.destroyed))
100+
return -ENODEV;
101+
96102
if (rtype >= HID_REPORT_TYPES)
97103
return -EINVAL;
98104

@@ -130,6 +136,9 @@ int dispatch_hid_bpf_output_report(struct hid_device *hdev,
130136
struct hid_bpf_ops *e;
131137
int ret, idx;
132138

139+
if (unlikely(hdev->bpf.destroyed))
140+
return -ENODEV;
141+
133142
idx = srcu_read_lock(&hdev->bpf.srcu);
134143
list_for_each_entry_srcu(e, &hdev->bpf.prog_list, list,
135144
srcu_read_lock_held(&hdev->bpf.srcu)) {

0 commit comments

Comments
 (0)