[PATCH 0/2] HID: appletb-kbd: fix UAF and mutex-in-atomic in inactivity timer

Sangyun Kim posted 2 patches 1 month, 3 weeks ago
drivers/hid/hid-appletb-kbd.c | 56 ++++++++++++++++++++++-------------
1 file changed, 36 insertions(+), 20 deletions(-)
[PATCH 0/2] HID: appletb-kbd: fix UAF and mutex-in-atomic in inactivity timer
Posted by Sangyun Kim 1 month, 3 weeks ago
This series addresses two defects in hid-appletb-kbd's inactivity
timer subsystem.  The two patches target different bugs and are
logically independent; they are sent together because they touch the
same tear-down code and because the same maintainer will review both.

Patch 1 fixes a slab use-after-free with two related tear-down windows
introduced by commit 38224c472a03 ("HID: appletb-kbd: fix slab
use-after-free bug in appletb_kbd_probe"):

  A) Within "if (kbd->backlight_dev)" the order was
     put_device() then timer_delete_sync().  A concurrent
     hid_appletb_bl unbind between those two calls can drop the last
     devm reference and free the backlight_device; the still-armed
     inactivity timer softirq then dereferences the freed object
     through backlight_device_set_brightness() -> mutex_lock(&ops_lock).

  B) The "if (kbd->backlight_dev)" block ran before
     hid_hw_close()/hid_hw_stop(), so even after window A is closed a
     late ".event" callback from the HID core (USB URB completion on
     real hardware) can arrive between timer_delete_sync() and
     put_device(), reach reset_inactivity_timer(), re-arm the timer
     via mod_timer(), and reopen the same UAF.

Both windows produce the same KASAN slab-use-after-free on the object
allocated by devm_backlight_device_register().  Patch 1 closes them
together by moving hid_hw_close()/hid_hw_stop() before the backlight
cleanup and, inside that cleanup block, calling timer_delete_sync()
before put_device().  Shipping both as one commit avoids leaving
stable kernels in a half-fixed state where only window A is closed.

Patch 2 fixes a separate "sleeping function called from invalid
context" bug in the same subsystem.  The inactivity timer is a
struct timer_list, so the callback runs in softirq context and calls
backlight_device_set_brightness() -> mutex_lock() from atomic
context; reset_inactivity_timer() has the same issue on the
brightness-restore path (it is called from appletb_kbd_hid_event()
and appletb_kbd_inp_event(), which run in softirq/IRQ context on
real USB hardware).  Convert the inactivity timer to a delayed_work
and defer the brightness-restore call to a dedicated work_struct so
both sleeping calls run in process context.

Sangyun Kim (2):
  HID: appletb-kbd: fix UAF in inactivity-timer cleanup path
  HID: appletb-kbd: run inactivity autodim from workqueues

 drivers/hid/hid-appletb-kbd.c | 56 ++++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 20 deletions(-)

-- 
2.34.1
Re: [PATCH 0/2] HID: appletb-kbd: fix UAF and mutex-in-atomic in inactivity timer
Posted by Jiri Kosina 1 month ago
On Mon, 20 Apr 2026, Sangyun Kim wrote:

> This series addresses two defects in hid-appletb-kbd's inactivity
> timer subsystem.  The two patches target different bugs and are
> logically independent; they are sent together because they touch the
> same tear-down code and because the same maintainer will review both.
> 
> Patch 1 fixes a slab use-after-free with two related tear-down windows
> introduced by commit 38224c472a03 ("HID: appletb-kbd: fix slab
> use-after-free bug in appletb_kbd_probe"):
> 
>   A) Within "if (kbd->backlight_dev)" the order was
>      put_device() then timer_delete_sync().  A concurrent
>      hid_appletb_bl unbind between those two calls can drop the last
>      devm reference and free the backlight_device; the still-armed
>      inactivity timer softirq then dereferences the freed object
>      through backlight_device_set_brightness() -> mutex_lock(&ops_lock).
> 
>   B) The "if (kbd->backlight_dev)" block ran before
>      hid_hw_close()/hid_hw_stop(), so even after window A is closed a
>      late ".event" callback from the HID core (USB URB completion on
>      real hardware) can arrive between timer_delete_sync() and
>      put_device(), reach reset_inactivity_timer(), re-arm the timer
>      via mod_timer(), and reopen the same UAF.
> 
> Both windows produce the same KASAN slab-use-after-free on the object
> allocated by devm_backlight_device_register().  Patch 1 closes them
> together by moving hid_hw_close()/hid_hw_stop() before the backlight
> cleanup and, inside that cleanup block, calling timer_delete_sync()
> before put_device().  Shipping both as one commit avoids leaving
> stable kernels in a half-fixed state where only window A is closed.
> 
> Patch 2 fixes a separate "sleeping function called from invalid
> context" bug in the same subsystem.  The inactivity timer is a
> struct timer_list, so the callback runs in softirq context and calls
> backlight_device_set_brightness() -> mutex_lock() from atomic
> context; reset_inactivity_timer() has the same issue on the
> brightness-restore path (it is called from appletb_kbd_hid_event()
> and appletb_kbd_inp_event(), which run in softirq/IRQ context on
> real USB hardware).  Convert the inactivity timer to a delayed_work
> and defer the brightness-restore call to a dedicated work_struct so
> both sleeping calls run in process context.
> 
> Sangyun Kim (2):
>   HID: appletb-kbd: fix UAF in inactivity-timer cleanup path
>   HID: appletb-kbd: run inactivity autodim from workqueues

Both applied to hid.git#for-7.1/upstream-fixes, thanks.

-- 
Jiri Kosina
SUSE Labs