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

[Aditya Garg]

On 28 April 2026 10:03:03 pm IST, Jiri Kosina <jikos@xxxxxxxxxx> wrote:
>On Mon, 20 Apr 2026, Aditya Garg 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
>> > 
>> >   drivers/hid/hid-appletb-kbd.c | 56 ++++++++++++++++++++++-------------
>> >   1 file changed, 36 insertions(+), 20 deletions(-)
>> > 
>> 
>> I had a very weird bug just once. And that was when I pressed fn key, upon
>> releasing, the touchbar mode did not restore to normal.
>
>You mean with this patch applied, correct?

Yes after applying that patch.

>