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

[Aditya Garg]

I got the bug just once so it's possible that it was a firmware related bug in the touchbar as well.

> On 22 Apr 2026, at 11:31 AM, Sangyun Kim <sangyun.kim@xxxxxxxxx> wrote:
> 
> On Mon, Apr 20, 2026 at 06:17:36 PM +0530, Aditya Garg wrote:
> 
>> 
>> 
>>> On 4/20/26 10:43, 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
>>> 
>>> 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.
>> 
>> Although it was just once, and I was never able to reproduce it again.
>> 
>> Have you tested it on your Machine btw?
>> 
>> 
> 
> Hi,
> 
> I have not tested this series on actual Apple Touch Bar hardware on my
> side, as I do not have access to such a machine locally. All testing on
> my side was done under QEMU with a uhid-based setup.
> 
> Because of that, I cannot say much about the one-off case where the
> Touch Bar did not restore the normal mode after releasing Fn. I have not
> been able to reproduce that specific behavior in my setup.
> 
> For patch 1, however, I was able to reproduce the teardown UAF in the
> uhid/QEMU setup and got the following KASAN report.
> 
> [   56.040407] ==================================================================
> [   56.042444] BUG: KASAN: slab-use-after-free in __run_timer_base.part.0+0x861/0x910
> [   56.044962] Write of size 8 at addr ffff88801b7e8958 by task swapper/0/0
> [   56.049092] CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Tainted: G                 N  7.0.0-dirty #2 PREEMPT(full)
> [   56.049967] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
> [   56.050843] Call Trace:
> [   56.051146]  <IRQ>
> [   56.052394]  __run_timer_base.part.0+0x861/0x910
> [   56.053123]  run_timer_softirq+0xd1/0x190
> 
> [   56.075012] Allocated by task 11:
> [   56.077221]  devm_kmalloc+0x70/0x1d0
> [   56.077545]  appletb_kbd_probe+0x65/0x470 [hid_appletb_kbd]
> [   56.085606]  uhid_device_add_worker+0x3b/0x100
> 
> [   56.088719] Freed by task 11:
> [   56.091296]  devres_release_group+0x1fd/0x3d0
> [   56.091844]  hid_device_probe+0x4db/0x7d0
> [   56.096916]  uhid_device_add_worker+0x3b/0x100
> 
> [   56.123572]  backlight_device_set_brightness+0x77/0x280
> [   56.123902]  appletb_inactivity_timer+0xe9/0x190 [hid_appletb_kbd]
> [   56.123967]  call_timer_fn+0x163/0x4a0
> [   56.124338]  __run_timer_base.part.0+0x575/0x910
> [   56.124711]  run_timer_softirq+0xd1/0x190
> 
> Patch 2 also matches what I saw in the same setup. On the unpatched
> tree, I can reproduce:
> 
> [   56.120488] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:591
> [   56.121118] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 0, name: swapper/0
> [   56.123080]  __mutex_lock+0xda/0x21c0
> [   56.123572]  backlight_device_set_brightness+0x77/0x280
> [   56.123902]  appletb_inactivity_timer+0xe9/0x190 [hid_appletb_kbd]
> [   56.124338]  __run_timer_base.part.0+0x575/0x910
> [   56.124711]  run_timer_softirq+0xd1/0x190
> 
> After applying patch 2, that warning no longer appeared in the timer
> reproducer in my uhi/QEMU runs. I also added a small UHID input trigger
> to exercise appletb_kbd_hid_event(), and in that setup brightness
> restored from 1 back to 2 in 5/5 iterations after the synthetic key
> event.
> 
> The limitation is that this is still UHID-only coverage. I do not have
> native Touch Bar hardware, and pure UHID does not model a real internal
> Apple keyboard/trackpad closely enough for me to claim coverage of the
> appletb_kbd_inp_event() path or real USB IRQ-context behavior.
> 
> Thanks,
> Sangyun