The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH v2] HID: hid-lenovo-go: cancel cfg_setup work in hid_go_cfg_remove()
       [not found] <20260515161830.E6E2BC2BCB3@smtp.kernel.org>
@ 2026-05-15 17:45 ` Manish Khadka
  0 siblings, 0 replies; only message in thread
From: Manish Khadka @ 2026-05-15 17:45 UTC (permalink / raw)
  To: linux-input
  Cc: Derek J . Clark, Mark Pearson, Jiri Kosina, Benjamin Tissoires,
	linux-kernel

hid_go_cfg_probe() initialises drvdata.go_cfg_setup and schedules it
to run 2 ms later:

    INIT_DELAYED_WORK(&drvdata.go_cfg_setup, &cfg_setup);
    schedule_delayed_work(&drvdata.go_cfg_setup, msecs_to_jiffies(2));

cfg_setup() dereferences drvdata.hdev to issue MCU command requests.
hid_go_cfg_remove() tears down sysfs and stops the HID device, but
never drains the delayed work.  If the device is unbound within the
2 ms scheduling delay (a probe failure rolling back via remove, or a
fast rmmod after probe), the work fires after hid_destroy_device()
has dropped its reference and released the underlying hdev struct,
leaving cfg_setup() with a stale drvdata.hdev pointer.

Mirror the sibling driver hid-lenovo-go-s.c, whose hid_gos_cfg_remove()
already calls cancel_delayed_work_sync() on its analogous work, and
drain go_cfg_setup at the top of hid_go_cfg_remove().  The cancel
must come before guard(mutex)(&drvdata.cfg_mutex) because cfg_setup()
acquires that mutex; reversing the order would deadlock.

Fixes: d69ccfcbc955 ("HID: hid-lenovo-go: Add Lenovo Legion Go Series HID Driver")
Cc: stable@vger.kernel.org
Signed-off-by: Manish Khadka <maskmemanish@gmail.com>
---

v1 -> v2:
  - Address Sashiko AI review feedback:
    * [Low] Correct the inaccurate description of how drvdata.hdev
      becomes stale.  hid_set_drvdata(hdev, NULL) only clears the
      per-hdev driver_data, not the global drvdata.hdev; the actual
      stale-pointer mechanism is hid_destroy_device()'s put_device()
      releasing the underlying hdev struct.  Commit message and the
      inline comment in hid_go_cfg_remove() are corrected accordingly.
  - The other six review points (ABBA deadlock with sysfs_remove_groups,
    global static drvdata corruption on multi-device/rebind, devm vs.
    explicit teardown race, probe error-path leaks, mcu_property_out
    unconditional return 0, schedule_delayed_work boolean misuse)
    are all valid pre-existing bugs in the driver but predate and are
    independent of this patch.  Each deserves its own fix; will be
    addressed in separate patches.
  Thanks to Sashiko AI review for all seven points.
 drivers/hid/hid-lenovo-go.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/hid/hid-lenovo-go.c b/drivers/hid/hid-lenovo-go.c
index d4d26c783356..b9d8cde53136 100644
--- a/drivers/hid/hid-lenovo-go.c
+++ b/drivers/hid/hid-lenovo-go.c
@@ -2408,6 +2408,15 @@ static int hid_go_cfg_probe(struct hid_device *hdev,
 
 static void hid_go_cfg_remove(struct hid_device *hdev)
 {
+	/*
+	 * cfg_setup is scheduled from hid_go_cfg_probe() with a 2 ms delay
+	 * and dereferences drvdata.hdev.  Drain it here before tearing
+	 * down so the workqueue cannot run after hid_destroy_device()'s
+	 * put_device() has released the underlying hdev and dereference
+	 * a stale drvdata.hdev pointer.
+	 */
+	cancel_delayed_work_sync(&drvdata.go_cfg_setup);
+
 	guard(mutex)(&drvdata.cfg_mutex);
 	sysfs_remove_groups(&hdev->dev.kobj, top_level_attr_groups);
 	hid_hw_close(hdev);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2026-05-15 17:45 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260515161830.E6E2BC2BCB3@smtp.kernel.org>
2026-05-15 17:45 ` [PATCH v2] HID: hid-lenovo-go: cancel cfg_setup work in hid_go_cfg_remove() Manish Khadka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox