| From 578e1b96fad7402ff7e9c7648c8f1ad0225147c8 Mon Sep 17 00:00:00 2001 |
| From: Rong Zhang <i@rong.moe> |
| Date: Mon, 12 May 2025 23:24:19 +0800 |
| Subject: HID: bpf: abort dispatch if device destroyed |
| |
| From: Rong Zhang <i@rong.moe> |
| |
| commit 578e1b96fad7402ff7e9c7648c8f1ad0225147c8 upstream. |
| |
| 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 9d7de2aa8b41 ("x86/percpu/64: Use relative percpu offsets") |
| [2]: commit 9286675a2aed ("HID: bpf: add HID-BPF hooks for |
| hid_hw_output_report") |
| |
| Closes: https://lore.kernel.org/all/20250506145548.GGaBoi9Jzp3aeJizTR@fat_crate.local/ |
| Fixes: 8bd0488b5ea5 ("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> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| drivers/hid/bpf/hid_bpf_dispatch.c | 9 +++++++++ |
| 1 file changed, 9 insertions(+) |
| |
| --- a/drivers/hid/bpf/hid_bpf_dispatch.c |
| +++ b/drivers/hid/bpf/hid_bpf_dispatch.c |
| @@ -38,6 +38,9 @@ dispatch_hid_bpf_device_event(struct hid |
| struct hid_bpf_ops *e; |
| int ret; |
| |
| + if (unlikely(hdev->bpf.destroyed)) |
| + return ERR_PTR(-ENODEV); |
| + |
| if (type >= HID_REPORT_TYPES) |
| return ERR_PTR(-EINVAL); |
| |
| @@ -93,6 +96,9 @@ int dispatch_hid_bpf_raw_requests(struct |
| struct hid_bpf_ops *e; |
| int ret, idx; |
| |
| + if (unlikely(hdev->bpf.destroyed)) |
| + return -ENODEV; |
| + |
| if (rtype >= HID_REPORT_TYPES) |
| return -EINVAL; |
| |
| @@ -130,6 +136,9 @@ int dispatch_hid_bpf_output_report(struc |
| struct hid_bpf_ops *e; |
| int ret, idx; |
| |
| + if (unlikely(hdev->bpf.destroyed)) |
| + return -ENODEV; |
| + |
| idx = srcu_read_lock(&hdev->bpf.srcu); |
| list_for_each_entry_srcu(e, &hdev->bpf.prog_list, list, |
| srcu_read_lock_held(&hdev->bpf.srcu)) { |