Thread (2 messages) 2 messages, 2 authors, 1d ago
WARM1d

[PATCH] HID: microsoft: cancel the rumble work on removal

From: Danish Khateeb <hidden>
Date: 2026-09-27 01:12:24
Also in: lkml
Subsystem: hid core layer, the rest · Maintainers: Jiri Kosina, Benjamin Tissoires, Linus Torvalds

Commit 1cfc77a64b71 ("HID: microsoft: move FF initialization to
.input_configured()") removed ms_remove_ff() along with ms_init_ff(),
and with it the only cancel_work_sync() of ff_worker. Nothing waits for
the rumble work any more before devres frees struct ms_data and the
report buffer that the work fills in.

Removing the device queues the work itself: hid_hw_stop() unregisters
the input device, and if an effect is playing, input_ff_flush() stops
it through ms_play_effect(). The work usually runs before remove()
returns, but nothing guarantees it.

Cancel the work again after hid_hw_stop(), when the input device is gone
and nothing can queue it any more. Initialize it in ms_probe(), so that
it can be cancelled for devices without force feedback too.

Fixes: 1cfc77a64b71 ("HID: microsoft: move FF initialization to .input_configured()")
Assisted-by: LLM
Signed-off-by: Danish Khateeb <redacted>
---

Notes:
    Tested in QEMU on a next-20260925 KASAN kernel with uhid devices bound
    to hid-microsoft: an Xbox Wireless Controller (BT 045e:0b13) destroyed
    while a rumble effect was playing and its evdev node was open (20
    cycles), a Natural Ergonomic 4000 (no force feedback, 5 cycles), then
    rmmod. Tracing shows the removal queuing ff_worker. There are no
    warnings with or without this patch: I could not reproduce the
    use-after-free, as the work always ran before remove() returned. A W=1
    build is clean.

 drivers/hid/hid-microsoft.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-microsoft.c b/drivers/hid/hid-microsoft.c
index a7d3493a6141..1b527954937d 100644
--- a/drivers/hid/hid-microsoft.c
+++ b/drivers/hid/hid-microsoft.c
@@ -335,7 +335,6 @@ static int ms_input_configured(struct hid_device *hdev, struct hid_input *hidinp
 		return 0;
 
 	ms->hdev = hdev;
-	INIT_WORK(&ms->ff_worker, ms_ff_worker);
 
 	ms->output_report_dmabuf = devm_kzalloc(&hdev->dev,
 						sizeof(struct xb1s_ff_report),
@@ -358,6 +357,7 @@ static int ms_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		return -ENOMEM;
 
 	ms->quirks = quirks;
+	INIT_WORK(&ms->ff_worker, ms_ff_worker);
 
 	hid_set_drvdata(hdev, ms);
 
@@ -384,7 +384,11 @@ static int ms_probe(struct hid_device *hdev, const struct hid_device_id *id)
 }
 static void ms_remove(struct hid_device *hdev)
 {
+	struct ms_data *ms = hid_get_drvdata(hdev);
+
 	hid_hw_stop(hdev);
+	/* Unregistering the input device stops rumble, which queues the work */
+	cancel_work_sync(&ms->ff_worker);
 }
 
 static const struct hid_device_id ms_devices[] = {
base-commit: 3f35b678a1d6b4c2dc773ad73623b1515cfc5bc5
-- 
2.55.0
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help