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

Chen Changcheng posted 1 patch 3 weeks, 5 days ago
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 3 weeks, 5 days ago
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
Reported-by: syzbot+0b8bff5929865345b29e@syzkaller.appspotmail.com
https://lore.kernel.org/all/6a937393.1d9ded08.62e62.0113.GAE@google.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
Re: [PATCH] HID: corsair: do not re-schedule LED worker after it has been cancelled
Posted by Jiri Kosina 2 weeks, 1 day ago
On Mon, 31 Aug 2026, 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.
> 
> Fixes: eb51c9f8cb4f0 ("HID: corsair: cancel worker before unregistering LED to fix use-after-free")
> Reported-by: syzbot+0a031a76585d1c7e737d@syzkaller.appspotmail.com
> Reported-by: syzbot+0b8bff5929865345b29e@syzkaller.appspotmail.com
> https://lore.kernel.org/all/6a937393.1d9ded08.62e62.0113.GAE@google.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;
> +

Sashiko is right here about the race. Can you please fix that and 
resubmit? Thanks,

-- 
Jiri Kosina
SUSE Labs
Re: [PATCH] HID: corsair: do not re-schedule LED worker after it has been cancelled
Posted by Chen Changcheng 1 week, 5 days ago
Hi Jeffin,
Thanks again for the pointer.  Danish Khateeb has submitted a patch
that is code-identical to the fix I had prepared, using
disable_work_sync() to replace cancel_work_sync() in all teardown
paths. I have reviewed this patch:

	https://lore.kernel.org/all/20260904161531.160118-1-danishkhateeb03@gmail.com/


--
Chen Changcheng