Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Medium] HID: corsair: use-after-free in k90_init_macro_functions()
error path [1]
--
commit 3a201ad222c1beccee0e1120afae987d661371db
Author: Andreas Claesson [off-list ref]
HID: corsair: cancel LED work again after unregistering the LED
This commit adds a second cancel_work_sync() call after
led_classdev_unregister() in k90_cleanup_backlight() and
k90_cleanup_macro_functions() to cancel work re-queued when
led_classdev_unregister() turns the LED off.
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/hid-corsair.c b/drivers/hid/hid-corsair.c
index 278c6efb565df..75c862f5f783f 100644
--- a/drivers/hid/hid-corsair.c
+++ b/drivers/hid/hid-corsair.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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);
+ /* unregister turns the LED off, which re-queues the work */
+ cancel_work_sync(&k90->record_led.work);
kfree(k90->record_led.cdev.name);
kfree(k90);
[Severity: Medium]
This problem wasn't introduced by this patch, but can the fail_sysfs error
path in k90_init_macro_functions() still cause a use-after-free on
led->cdev.dev [1]?
drivers/hid/hid-corsair.c:k90_init_macro_functions() {
...
fail_sysfs:
k90->record_led.removed = true;
led_classdev_unregister(&k90->record_led.cdev);
cancel_work_sync(&k90->record_led.work);
...
}
When sysfs_create_group() fails after led_classdev_register() has exposed
the record LED, fail_sysfs calls led_classdev_unregister() without calling
cancel_work_sync() first:
CPU0 CPU1
k90_init_macro_functions()
led_classdev_register(...)
k90_brightness_set()
schedule_work(&led->work)
k90_record_led_work()
if (led->removed)
sysfs_create_group(...)
k90->record_led.removed = true;
led_classdev_unregister(...)
led->cdev.dev->parent
<-- use-after-free
cancel_work_sync(...)
Calling cancel_work_sync(&k90->record_led.work) before
led_classdev_unregister() in fail_sysfs as well would match
k90_cleanup_macro_functions().
[1] https://sashiko.dev/#/bug/linux-2f786541-d064-428e-a1ab-d203d4112002
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-hid-corsair-k90-uaf-v1-1-0621c2c5c451@aclaesson.com?part=1