Re: [PATCH v2 4/4] HID: wiimote: fix uaf when hid events are handled during destroy

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Module cleanup commands (e.g., rumble off, LEDs off) are dropped during driver unload, leaving the controller hardware in an incorrect or stuck state.
--

--- Patch [4]: [PATCH v2 4/4] HID: wiimote: fix uaf when hid events are handled during destroy ---
commit 62bea240ecf4b4da704b1580ca426a1f9c83501a
Author: Rafael Passos <rafael@rcpassos.me>

HID: wiimote: fix uaf when hid events are handled during destroy

This commit adds a flag check to wiimote_queue() to drop events
if the driver is in the process of unloading. This mitigates a
use-after-free issue when handling events during destroy.

> diff --git a/drivers/hid/hid-wiimote-core.c b/drivers/hid/hid-wiimote-core.c
> index b549a7d58d512..c9638ec352e44 100644
> --- a/drivers/hid/hid-wiimote-core.c
> +++ b/drivers/hid/hid-wiimote-core.c
> @@ -92,6 +92,9 @@ static void wiimote_queue(struct wiimote_data *wdata, const __u8 *buffer,
>  	 */
>  
>  	spin_lock_irqsave(&wdata->queue.lock, flags);
> +	/* Do not schedule work if controller is exiting */
> +	if ((wdata->state.flags & WIIPROTO_FLAG_EXITING))
> +		goto out_unlock;

[Severity: Medium]
Will this aggressively drop hardware cleanup commands during driver unload?

Looking at wiimote_destroy(), the exiting flag is set before calling
wiimote_modules_unload():

drivers/hid/hid-wiimote-core.c:wiimote_destroy() {
    ...
	/* prevent init_worker from being scheduled again */
	scoped_guard(spinlock_irqsave, &wdata->state.lock)
		wdata->state.flags |= WIIPROTO_FLAG_EXITING;
    ...
	wiimote_modules_unload(wdata);
    ...
}

If a user unbinds the driver or disconnects the controller while rumble is
active, wiimote_modules_unload() invokes wiimod_rumble_remove() to send
a command to turn the rumble off.

Will this check in wiimote_queue() unexpectedly drop the rumble-off
packet, leaving the physical controller vibrating indefinitely?

>  
>  	memcpy(wdata->queue.outq[wdata->queue.head].data, buffer, count);
>  	wdata->queue.outq[wdata->queue.head].size = count;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715212501.3920033-1-rafael@rcpassos.me?part=4