platform/x86: asus-wmi: fix use-after-free unregistering kbd_led led
Denis Benato
maintainerPATCH 1/1 platform/x86: asus-wmi: fix use-after-free unregistering kbd_led led
kbd_led_update_all() registers asus->kbd_led with devm_led_classdev_register() on the platform device, but kbd_led is embedded in struct asus_wmi, which asus_wmi_remove() frees at its end. The driver core calls .remove() before devres_release_all(), so the deferred devres release runs led_classdev_unregister() on memory that has already been freed. Whether that faults depends on whether the slab has been reused in between, which makes the crash on module unload intermittent and silently corrupts the led classdev state in the remaining runs.
Unregister the led classdev explicitly in asus_wmi_led_exit(), after the led workqueue has been destroyed: kbd_led_work can then neither run nor be queued anymore, since asus_ref.asus is NULL too, so kbd_led_registered is stable and the classdev can be released while the memory embedding it is still alive. devm_led_classdev_unregister() also drops the devres entry, leaving nothing to run at unbind cleanup time; it warns when no devres entry exists, hence the kbd_led_registered check.
--- drivers/platform/x86/asus-wmi.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
drivers/platform/x86/asus-wmi.c +12 -0
@@ -2088,6 +2088,18 @@ static void asus_wmi_led_exit(struct asus_wmi *asus)if (asus->led_workqueue)destroy_workqueue (asus->led_workqueue); + +/* +* kbd_led is registered lazily by kbd_led_work: now that the +* workqueue is destroyed and asus_ref.asus is NULL, the work can +* neither run nor be queued anymore, furthermore leaving it to +* devres would run the unregister from devres_release_all(), +* after .remove() returned and the struct asus_wmi embedding +* kbd_led has been freed. +*/ +if (asus->kbd_led_registered) +devm_led_classdev_unregister (&asus->platform_device->dev, + &asus->kbd_led); }static int asus_wmi_led_init (struct asus_wmi *asus)
Signature · 1 line
2.47.3
Ilpo Järvinen
maintainerQuoted 11 lines · Denis Benato
kbd_led_update_all()registersasus->kbd_ledwithdevm_led_classdev_register()on the platform device, butkbd_ledis embedded in structasus_wmi, whichasus_wmi_remove()frees at its end. The driver core calls .remove() beforedevres_release_all(), so the deferred devres release runsled_classdev_unregister()on memory that has already been freed. Whether that faults depends on whether the slab has been reused in between, which makes the crash on module unload intermittent and silently corrupts the led classdev state in the remaining runs.[...]
Thank you for your contribution, it has been applied to my local review-ilpo-next branch. Note it will show up in the public platform-drivers-x86/review-ilpo-next branch only once I've pushed my local branch there, which might take a while.
FYI [if applicable to your patch], as per Linus' policy change, also fixes are mostly routed through for-next unless the fix is for a commit introduced in the most recent cycle or is clearly a regression fix.
The list of commits applied:
[1/1] platform/x86: asus-wmi: fix use-after-free unregistering kbd_led led
commit: 2f5005e261ed758254767a32c24587bfacc6db82
--
i.