[PATCH] HID: corsair: do not re-schedule LED worker after it has been cancelled

Chen Changcheng posted 1 patch 1 month, 1 week ago
There is a newer version of this series
drivers/hid/hid-corsair.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
[PATCH] HID: corsair: do not re-schedule LED worker after it has been cancelled
Posted by Chen Changcheng 1 month, 1 week ago
From: Chen Changcheng <chenchangcheng@kylinos.cn>

Commit eb51c9f8cb4f0 ("HID: corsair: cancel worker before unregistering
LED to fix use-after-free") moved cancel_work_sync() ahead of
led_classdev_unregister() in k90_cleanup_backlight() and
k90_cleanup_macro_functions().  led_classdev_unregister() internally
calls led_set_brightness(LED_OFF), which reaches the driver's
k90_brightness_set() callback.  Since that callback schedules the worker
unconditionally, the worker was re-queued after cancel_work_sync() had
drained it, and the subsequent kfree() freed a still-active work_struct:

    ODEBUG: free active (active state 0) object type: work_struct
            hint: k90_record_led_work

The removed flag check inside the worker itself only stops it from
dereferencing freed memory once it runs; it cannot prevent the re-queue.

Fix this by making k90_brightness_set() a no-op once removed is set, so
the LED_OFF update issued from led_classdev_unregister() can no longer
re-schedule the worker after it has been cancelled.  Also apply the
cancel-before-unregister ordering to the probe error path
(k90_init_macro_functions() fail_sysfs) for consistency.

Fixes: eb51c9f8cb4f0 ("HID: corsair: cancel worker before unregistering LED to fix use-after-free")
Reported-by: syzbot+0a031a76585d1c7e737d@syzkaller.appspotmail.com
Signed-off-by: Chen Changcheng <chenchangcheng@kylinos.cn>
---
 drivers/hid/hid-corsair.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-corsair.c b/drivers/hid/hid-corsair.c
index 278c6efb565d..73b3c1ff78c6 100644
--- a/drivers/hid/hid-corsair.c
+++ b/drivers/hid/hid-corsair.c
@@ -194,6 +194,9 @@ static void k90_brightness_set(struct led_classdev *led_cdev,
 {
 	struct k90_led *led = container_of(led_cdev, struct k90_led, cdev);
 
+	if (led->removed)
+		return;
+
 	led->brightness = brightness;
 	schedule_work(&led->work);
 }
@@ -507,8 +510,8 @@ static int k90_init_macro_functions(struct hid_device *dev)
 
 fail_sysfs:
 	k90->record_led.removed = true;
-	led_classdev_unregister(&k90->record_led.cdev);
 	cancel_work_sync(&k90->record_led.work);
+	led_classdev_unregister(&k90->record_led.cdev);
 fail_record_led:
 	kfree(k90->record_led.cdev.name);
 fail_record_led_alloc:
-- 
2.25.1
[PATCH] HID: corsair: do not re-schedule LED worker after it has been cancelled
Posted by Jeffin Philip 4 weeks ago
On Mon, 17 Aug 2026 15:28:31 +0800, Chen Changcheng wrote:
>Commit eb51c9f8cb4f0 ("HID: corsair: cancel worker before unregistering
>LED to fix use-after-free") moved cancel_work_sync() ahead of
>led_classdev_unregister() in k90_cleanup_backlight() and
>k90_cleanup_macro_functions().  led_classdev_unregister() internally
>calls led_set_brightness(LED_OFF), which reaches the driver's
>k90_brightness_set() callback.  Since that callback schedules the worker
>unconditionally, the worker was re-queued after cancel_work_sync() had
>drained it, and the subsequent kfree() freed a still-active work_struct:
>
>    ODEBUG: free active (active state 0) object type: work_struct
>            hint: k90_record_led_work
>
>The removed flag check inside the worker itself only stops it from
>dereferencing freed memory once it runs; it cannot prevent the re-queue.
>
>Fix this by making k90_brightness_set() a no-op once removed is set, so
>the LED_OFF update issued from led_classdev_unregister() can no longer
>re-schedule the worker after it has been cancelled.  Also apply the
>cancel-before-unregister ordering to the probe error path
>(k90_init_macro_functions() fail_sysfs) for consistency.

This UAF[1] can be prevented by your patch, would you consider adding that too
in your patch?

[1]: https://lore.kernel.org/all/6a937393.1d9ded08.62e62.0113.GAE@google.com/#R

Thanks,
Jeffin.
Re: [PATCH 0/2] HID: corsair: fix two use-after-free bugs on device removal
Posted by Chen Changcheng 4 weeks ago
Hi Jeffin,

Thanks for pointing this out.

The first two patches of the series have already been merged into
v7.3, while the follow-up fix (making k90_brightness_set() a no-op
once removed is set) has not been merged yet.  Since the original
thread is about the two merged patches, I think it is cleaner to start
a new thread for the remaining fix rather than appending it as a fixup
to this one.

I will submit the patch as a separate submission shortly, with both
syzbot reports tagged so syzbot can verify the fix.

Thanks again for the report.
Chen Changcheng