[PATCH 0/2] HID: winwing: two teardown fixes

["René Onier" <f3nr1l@me.com>]

Two teardown bugs in hid-winwing, both present in mainline. The series is
based on hid.git for-next, which already carries the related fix
a1a5ad37e50c ("HID: winwing: fix use-after-free in force feedback
teardown"); it does not depend on it.

Patch 1 fixes a use-after-free of the rumble work item. winwing_remove()
cancels the work before it stops the device, but stopping the device flushes
the force feedback effects, which calls the driver's play_effect handler one
last time and queues the work again:

  hid_hw_stop() -> input_unregister_device() -> evdev_cleanup()
    -> input_flush_device() -> input_ff_flush() -> erase_effect()
    -> ml_ff_playback(dev, id, 0) -> ml_play_effects()
    -> winwing_play_effect() -> schedule_work(&data->rumble_work)

The data the work runs on is devm-allocated, and hid_device_remove() releases
the driver's devres group as soon as .remove returns, so the requeued work
runs on freed memory. Cancelling after hid_hw_stop() closes the window: once
the input device is gone nothing can queue the work again, and the driver
data is still valid until .remove returns.

Patch 2 initialises data->lights_lock, which winwing_led_write() has been
taking since the driver was merged. The mutex only ever gets the zeroing from
devm_kzalloc(); CONFIG_DEBUG_MUTEXES and lockdep both flag it on the first
brightness write.

Patch 1 needs a device with a rumble motor to trigger; patch 2 affects every
supported device. Both were pointed out by the automated Sashiko review of
the earlier force feedback fix on linux-input, and confirmed by reading the
teardown path rather than by a crash. I have since exercised patch 1 on URSA MINOR sticks, with the URSA
MINOR series (posted separately, on top of this one) applied: unloading the
module while a 5 s rumble effect is playing. ftrace shows the chain above
taking place inside hid_hw_stop(), and the requeued work running before
winwing_remove() returns; no warning or oops (on a kernel without KASAN).

A third, related issue is deliberately left out of this series. The LED class
devices are registered with devm_led_classdev_register(), so they outlive
hid_hw_stop() by the length of the devres pass that hid_device_remove() runs
after .remove returns. A sysfs brightness write in that window reaches
hid_hw_output_report() on a stopped device; usbhid returns an error once its
output URB pointer has been cleared, but that check is not serialised against
usbhid_stop(). Fixing it inside the driver means dropping devm for the LEDs
and unwinding them by hand in the probe error paths - a fair amount of churn
for a narrow race, and the same shape exists in other HID drivers that
register LEDs with devm, so it may belong in the HID core instead. I have a
driver-side patch ready and will post it separately if you prefer that.

René Onier (2):
  HID: winwing: fix use-after-free of the rumble work
  HID: winwing: initialize the lights_lock mutex

 drivers/hid/hid-winwing.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)


base-commit: 145c2b2e9a5c0f794fb4009bcb072ab19f8ccfcd
-- 
2.55.0