Thread (15 messages) flat view 15 messages, 3 authors, 2d ago
WARM2d

Revision v3 of 2 in this series.

Revisions (2)
  1. v2 [diff vs current]
  2. v3 current

[PATCH v3 0/4] HID: wiimote: new LED behavior on connect, scoped guards, uaf

From: Rafael Passos <hidden>
Date: 2026-07-29 16:50:37

Hi,
This patchset contains one feature change, two cleanup patches + 1 uaf fix.

The feature is turning different LEDs for each of the first 4 wiimotes connected.
From id 5 forward, the LED will cycle back to 1, and so on.
This uses the ida struct, so its quite simple and lightweight.
The hid_info log message prints out the controller id.

While implementing this feature, I decided to cleanup the code using
scoped_guard for the many spinlocks in the driver. There are two places
where the original lock/unlock version fits best, and I left them
untouched.
I also used the __free scope cleanup in the wiimote and LED probe functions.
The wiimote_probe required a new state tracker bitmask. Trivial for the LED.
Lastly, I fixed a pre-existing uaf pointed out by sashiko in V1, using
the driver for the playstation controller as a reference.

It was really fun working with this driver.
I tested it with 4 Wii Motion Plus remotes (gen2).
Video recording of my tests (48s video).
https://rcpassos.me/video/wiimote-led-linux-driver

Thanks,
Rafael Passos

---
V1: https://lore.kernel.org/linux-input/20260710153456.2093889-1-rafael@rcpassos.me/ (local)
Changes from v1:
    (1/3):
    - fix ida_alloc_min error handling to consider negative values
    - remove fallback to 1 on ida_alloc_min failure
    - move player_leds static array to hid-wiimote-core.c
    - s/instance_id/player_id/g
    - store player_id on an u8
    (2/3):
    - add header include for cleanup.h
    - add identation to one-liner scoped_guards
    (3/3):
    - add scoped cleanup function to wiimote_probe, with a bitmask to track state
      Patch used for testing this:
      https://lore.kernel.org/linux-input/20260715213513.3929001-1-rafael@rcpassos.me/ (local)
    (4/4) *new patch* :
    - sashiko found a pre-existing uaf. Unlikely, but correct.
      implemented using the playstation driver as an inspiration

V2: https://lore.kernel.org/linux-input/20260710153456.2093889-1-rafael@rcpassos.me/ (local)
Changes from v2:
    (2/4):
    - join the last two locks into a single scoped_guard lock in wiimote_modules_load

Notes on Sashiko reviews for V2:
    - controller state on driver unload: it would be funny if the
      controller would stay vibrating as suggested. I forced this case
      dropping the connection from kernel in the dirty state, but the
      controller just shuts down.
    - mixed goto/scoped cleanup: there is scoped locking and goto, not
      scoped cleanup. I think this is fine.
    - integer/u8 truncation in player_id: would need 256+ controllers.
      Not realistic. Even Bluetooth would refuse this.


Rafael Passos (4):
  HID: wiimote: turn on the LEDs indicating the controller id
  HID: wiimote: replace spinlock pairs with scoped_guard
  HID: wiimote: use scoped cleanup in wiimote and led probes
  HID: wiimote: fix uaf when hid events are handled during destroy

 drivers/hid/hid-wiimote-core.c    | 339 ++++++++++++++++--------------
 drivers/hid/hid-wiimote-debug.c   |  50 ++---
 drivers/hid/hid-wiimote-modules.c |  24 +--
 drivers/hid/hid-wiimote.h         |   2 +
 4 files changed, 216 insertions(+), 199 deletions(-)

-- 
2.53.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