[PATCH] HID: wiimote: initialise rumble_worker once per device

[Jinmo Yang <jinmo44.yang@gmail.com>]

wdata->rumble_worker is a single work_struct in struct wiimote_data, but
two different modules called INIT_WORK() on it:

  wiimod_rumble_probe()  devtype module, once per device
  wiimod_pro_probe()     extension module, once per extension hotplug

These have independent lifetimes.  The devtype module is loaded once by
wiimote_modules_load() and stays until wiimote_destroy(), while the
extension module is loaded and unloaded any number of times by
wiimote_ext_load() / wiimote_ext_unload().  wiimote_ext_load() calls the
extension probe with no lock and no cancel_work_sync(), and for the
common EXT_NONE -> EXT_PRO_CONTROLLER transition the preceding
wiimote_ext_unload() is a no-op because wiimod_ext_table[] maps both
EXT_NONE and EXT_UNKNOWN to wiimod_dummy, which has no .remove.

So nothing stops the force-feedback path, which is still live in the
base module, from having queued the work already:

	/* wiimod_rumble_play() */
	wdata->state.cache_rumble = value;
	schedule_work(&wdata->rumble_worker);

INIT_WORK() on a queued work item is memory corruption.  __INIT_WORK()
does

	(_work)->data = (atomic_long_t) WORK_DATA_INIT();
	INIT_LIST_HEAD(&(_work)->entry);

which clears the pool reference and the PENDING bit, and re-points
->entry at itself while the item is still linked into the worker pool
list, leaving its neighbours pointing at a node that has left the list.

Note that CONFIG_DEBUG_OBJECTS_WORK hides the impact rather than just
reporting it: work_fixup_init() calls cancel_work_sync() for the caller,
so a debug kernel prints a warning and then repairs the state.  That is
the kernel stating what the driver should have done.  Production kernels
have no such repair.

Initialise the work once in wiimote_create(), which is where the object
that contains it is created and which already initialises
wdata->queue.worker, wdata->init_worker and wdata->timer, and drop both
module-level INIT_WORK() calls.  wiimote_create() runs as the first
statement of wiimote_hid_probe(), before hid_parse() and hid_hw_start(),
so it strictly precedes the existence of any input device and therefore
of any path that could queue the work.

This does not affect the deadlock fix in commit f50f9aabf32d ("HID:
wiimote: fix FF deadlock").  That fix is the offload itself - caching
the value and deferring to a worker instead of taking state.lock inside
the FF callback - and this patch leaves wiimod_rumble_play(),
wiimod_rumble_worker() and state.cache_rumble untouched.  Only the
placement of the initialisation changes.

The two halves come from different actors, and both are available on a
stock Android phone without root.  The extension hotplug half needs only
/dev/uhid, which is group 3011 and held by the shell.  The shell cannot
write to an evdev node - SELinux grants it read access only - so the
force-feedback half comes from an ordinary app calling
InputDevice.getVibrator().vibrate(), which reaches EVIOCSFF through
system_server; VIBRATE is a normal permission and is granted
automatically.  A paired Bluetooth HID device can supply the hotplug
half instead of /dev/uhid.

The ordering matters: the device has to be created without an extension
so that the devtype resolves to a GEN10/GEN20 that includes
WIIMOD_RUMBLE, because WIIMOTE_DEV_PRO_CONTROLLER does not, and the
extension has to be plugged afterwards.

Measured before the change:

  Pixel 11, stock Android 17 user build, SELinux enforcing, no KASAN and
  no DEBUG_OBJECTS at runtime, kernel 6.12.81: reboots with
  ro.boot.bootreason=kernel_panic one to two seconds after the two paths
  start overlapping.

    Unable to handle kernel NULL pointer dereference at virtual address
      0000000000000008
    CPU: 5 Comm: kworker/5:4
    Workqueue:  wiimod_rumble_worker (events)
    pc : process_scheduled_works+0xd4/0x810
    Kernel panic - not syncing: Oops: Fatal exception

  The empty workqueue name in that line is itself the corruption:
  print_worker_info() reads wq->name with copy_from_kernel_nofault() and
  it failed, while another CPU in the same dump shows the normal form,
  "Workqueue: events wiimote_init_worker" - the task that called
  wiimod_pro_probe().

  x86_64 with CONFIG_KASAN_GENERIC and CONFIG_DEBUG_OBJECTS_WORK both
  enabled, 15 s in: KASAN says nothing, because INIT_WORK() writes to a
  valid field of a live object, and DEBUG_OBJECTS_WORK reports

    ODEBUG: init active (active state 0) object type: work_struct
      hint: wiimod_rumble_worker+0x0/0x70
    WARNING: lib/debugobjects.c:629 at debug_print_object
    Workqueue: events wiimote_init_worker
     __debug_object_init+0x1ff/0x3b0
     __init_work+0x51/0x60
     wiimod_pro_probe+0x28/0xba0
     wiimote_init_worker.cold+0xb6a/0xe88

After the change the same reproducer drives 36774 extension hotplugs and
612156 force-feedback plays in 60 s on that x86_64 kernel with no ODEBUG,
WARNING or KASAN output, and the rumble reports still arrive, so the
force-feedback path is unaffected.

Fixes: f50f9aabf32d ("HID: wiimote: fix FF deadlock")
Cc: stable@vger.kernel.org
Signed-off-by: Jinmo Yang <jinmo44.yang@gmail.com>
---
Notes for reviewers, not for the commit message:

 - base-commit below is the public next-20260925 tag, but the three files
   touched are byte-identical in linux-next, hid/for-next, hid/master and
   hid/for-7.4/*, and the patch applies to all of them unchanged, so pick
   whichever tree suits.

 - It conflicts textually with the pending series "[PATCH v4 0/4] HID:
   wiimote: new LED behavior on connect + scoped_guards" from Rafael Passos
   (<20260817213840.1053216-1-rafael@rcpassos.me>), whose 4/4 rewrites
   wiimote_create() and drops the call. Happy to rebase on top of that if it
   goes in first; I kept this one standalone because it is a stable
   candidate and that series is a refactor.

 - The reproducer is a reactive /dev/uhid wiimote emulator, plus a
   reflection-only dex run through app_process to drive the force-feedback
   half on Android. I can post either if that would help.

 drivers/hid/hid-wiimote-core.c    | 1 +
 drivers/hid/hid-wiimote-modules.c | 5 +----
 drivers/hid/hid-wiimote.h         | 1 +
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-wiimote-core.c b/drivers/hid/hid-wiimote-core.c
index 63c4fa8fbb9b..1946c23867c8 100644
--- a/drivers/hid/hid-wiimote-core.c
+++ b/drivers/hid/hid-wiimote-core.c
@@ -1754,6 +1754,7 @@ static struct wiimote_data *wiimote_create(struct hid_device *hdev)
 	wdata->state.cmd_battery = 0xff;
 
 	INIT_WORK(&wdata->init_worker, wiimote_init_worker);
+	INIT_WORK(&wdata->rumble_worker, wiimod_rumble_worker);
 	timer_setup(&wdata->timer, wiimote_init_timeout, 0);
 
 	return wdata;
diff --git a/drivers/hid/hid-wiimote-modules.c b/drivers/hid/hid-wiimote-modules.c
index dccb78bb3afd..c5b48065638e 100644
--- a/drivers/hid/hid-wiimote-modules.c
+++ b/drivers/hid/hid-wiimote-modules.c
@@ -117,7 +117,7 @@ static const struct wiimod_ops wiimod_keys = {
  */
 
 /* used by wiimod_rumble and wiipro_rumble */
-static void wiimod_rumble_worker(struct work_struct *work)
+void wiimod_rumble_worker(struct work_struct *work)
 {
 	struct wiimote_data *wdata = container_of(work, struct wiimote_data,
 						  rumble_worker);
@@ -155,8 +155,6 @@ static int wiimod_rumble_play(struct input_dev *dev, void *data,
 static int wiimod_rumble_probe(const struct wiimod_ops *ops,
 			       struct wiimote_data *wdata)
 {
-	INIT_WORK(&wdata->rumble_worker, wiimod_rumble_worker);
-
 	set_bit(FF_RUMBLE, wdata->input->ffbit);
 	if (input_ff_create_memless(wdata->input, NULL, wiimod_rumble_play))
 		return -ENOMEM;
@@ -1865,7 +1863,6 @@ static int wiimod_pro_probe(const struct wiimod_ops *ops,
 	int ret, i;
 	unsigned long flags;
 
-	INIT_WORK(&wdata->rumble_worker, wiimod_rumble_worker);
 	wdata->state.calib_pro_sticks[0] = 0;
 	wdata->state.calib_pro_sticks[1] = 0;
 	wdata->state.calib_pro_sticks[2] = 0;
diff --git a/drivers/hid/hid-wiimote.h b/drivers/hid/hid-wiimote.h
index 9c12f63f6dd2..c63cd0a68e0c 100644
--- a/drivers/hid/hid-wiimote.h
+++ b/drivers/hid/hid-wiimote.h
@@ -262,6 +262,7 @@ enum wiiproto_reqs {
 #define dev_to_wii(pdev) hid_get_drvdata(to_hid_device(pdev))
 
 void __wiimote_schedule(struct wiimote_data *wdata);
+void wiimod_rumble_worker(struct work_struct *work);
 
 extern void wiiproto_req_drm(struct wiimote_data *wdata, __u8 drm);
 extern void wiiproto_req_rumble(struct wiimote_data *wdata, __u8 rumble);

base-commit: f5f84daefcd92d7a630066635ecea1433ed5eac7
-- 
2.53.0