Re: [PATCH v16 2/4] HID: nintendo: Add rumble support for Switch 2 controllers
flat view
From: sashiko-bot@kernel.org
Date: 2026-10-06 06:30:02
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] hid: nintendo: Use-after-free of ns2 in rumble_work due to missing sync cancel - [High] hid: nintendo: Workqueue list corruption due to rumble_work re-initialization -- commit a197b51ef6afbdd0338765f77b38c7488b6d41fe Author: Vicki Pfau [off-list ref] HID: nintendo: Add rumble support for Switch 2 controllers This commit adds basic rumble support for Switch 2 controllers, simulating standard force feedback using the HD Rumble actuators. It introduces a delayed work item to manage rumble state updates.
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c index c31ded267be76..c49ca95753206 100644 --- a/drivers/hid/hid-nintendo.c +++ b/drivers/hid/hid-nintendo.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -3374,6 +3550,26 @@ static int switch2_init_input(struct switch2_controller *ns2) return -EINVAL; } +#if IS_ENABLED(CONFIG_NINTENDO_FF) + ns2->rumble_buffer = devm_kzalloc(&input->dev, 64, GFP_KERNEL); + if (!ns2->rumble_buffer) { + input_free_device(input); + return -ENOMEM; + } + ret = devm_delayed_work_autocancel(&input->dev, &ns2->rumble_work, switch2_rumble_work);
[Severity: High] Can this lead to a use-after-free of the ns2 structure? The devm_delayed_work_autocancel() helper binds the work cancellation to the input device's devres group. If a userspace process holds the input device open when the physical device is unplugged, the devres execution is delayed until the file descriptor is closed. However, the driver's remove function frees the ns2 controller structure synchronously via kref_put() without waiting for the devres cleanup. If the delayed work is pending, it will execute and access the freed ns2 structure in switch2_rumble_work(). [Severity: High] Does this code corrupt the workqueue list if initialization is retried? In switch2_init_input(), the input device is registered and exposed to userspace. If a subsequent initialization step fails, such as the LED registration, the error path unregisters the device. If userspace opens the device and triggers a rumble effect before unregistration completes, ns2->rumble_work is queued. The state machine then retries switch2_init_controller(), which calls switch2_init_input() again. This allocates a new input device and calls devm_delayed_work_autocancel() on the exact same &ns2->rumble_work. This unconditionally calls INIT_DELAYED_WORK() on an already-queued work item, which overwrites its list pointers and corrupts the workqueue list.
quoted hunk ↗ jump to hunk
+ if (ret < 0) { + input_free_device(input); + return ret; + } + + input_set_capability(input, EV_FF, FF_RUMBLE); + ret = input_ff_create_memless(input, NULL, switch2_play_effect); + if (ret) { + input_free_device(input); + return ret; + } +#endif
-- Sashiko AI review · https://sashiko.dev/#/patchset/20261006061619.2005967-1-vi@endrift.com?part=2