[PATCH] HID: corsair: cancel the LED work again after unregistering the LED

[PATCH] HID: corsair: cancel the LED work again after unregistering the LED

[Payal Lenka]

Commit eb51c9f8cb4f ("HID: corsair: cancel worker before unregistering
LED to fix use-after-free") cancels the LED work before calling
led_classdev_unregister(). But unless LED_RETAIN_AT_SHUTDOWN is set,
led_classdev_unregister() turns the LED off, and k90_brightness_set()
then queues the work again. The k90_led holding that work is freed right
after, with the work still queued. The removed flag does not help, as
the worker would read it from the freed memory.

This is caught by debugobjects when a device is disconnected:

  ODEBUG: free active (active state 0) object: 0000000097898442
    object type: work_struct hint: k90_record_led_work+0x0/0x1e0
  WARNING: lib/debugobjects.c:629 at debug_print_object+0x189/0x2a0

Cancel the work a second time after unregistering the LED, in both
k90_cleanup_backlight() and k90_cleanup_macro_functions(). The first
cancel stays, as it keeps a running worker from using led->cdev.dev while
the LED is unregistered.

The root cause analysis and this patch were prepared with the help of the
Claude Opus 5.5 coding assistant.

Fixes: eb51c9f8cb4f ("HID: corsair: cancel worker before unregistering LED to fix use-after-free")
Reported-by: syzbot+f8fb5598924c7e645553@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=f8fb5598924c7e645553
Cc: stable@vger.kernel.org
Signed-off-by: Payal Lenka <payalm.lenka@gmail.com>
---
Tested in QEMU (x86_64, KASAN, DEBUG_OBJECTS_WORK and DEBUG_OBJECTS_FREE)
with a C reproducer from the syzbot report, 120 seconds per run:

  no fix:                     warnings for both work items
  this patch:                 no warnings over 124 disconnects
  only the backlight cancel:  warnings for k90_record_led_work
  only the record LED cancel: warnings for k90_backlight_work

The reproducer does not exercise the race fixed by eb51c9f8cb4f, so that
is not tested here; the first cancel is kept for it. Not tested on real
K90 hardware.

 drivers/hid/hid-corsair.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/hid/hid-corsair.c b/drivers/hid/hid-corsair.c
index 278c6efb5..7e5ff3d52 100644
--- a/drivers/hid/hid-corsair.c
+++ b/drivers/hid/hid-corsair.c
@@ -526,6 +526,8 @@ static void k90_cleanup_backlight(struct hid_device *dev)
 		drvdata->backlight->removed = true;
 		cancel_work_sync(&drvdata->backlight->work);
 		led_classdev_unregister(&drvdata->backlight->cdev);
+		/* Unregistering turns the LED off, which queues the work again */
+		cancel_work_sync(&drvdata->backlight->work);
 		kfree(drvdata->backlight->cdev.name);
 		kfree(drvdata->backlight);
 	}
@@ -542,6 +544,8 @@ static void k90_cleanup_macro_functions(struct hid_device *dev)
 		k90->record_led.removed = true;
 		cancel_work_sync(&k90->record_led.work);
 		led_classdev_unregister(&k90->record_led.cdev);
+		/* Unregistering turns the LED off, which queues the work again */
+		cancel_work_sync(&k90->record_led.work);
 		kfree(k90->record_led.cdev.name);
 
 		kfree(k90);
-- 
2.43.0