* [PATCH 0/2] HID: appletb-kbd: fix UAF and mutex-in-atomic in inactivity timer
@ 2026-04-20 5:13 Sangyun Kim
2026-04-20 5:13 ` [PATCH 1/2] HID: appletb-kbd: fix UAF in inactivity-timer cleanup path Sangyun Kim
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Sangyun Kim @ 2026-04-20 5:13 UTC (permalink / raw)
To: jikos, bentiss; +Cc: qasdev00, gargaditya08, linux-input, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/2] HID: appletb-kbd: fix UAF in inactivity-timer cleanup path 2026-04-20 5:13 [PATCH 0/2] HID: appletb-kbd: fix UAF and mutex-in-atomic in inactivity timer Sangyun Kim @ 2026-04-20 5:13 ` Sangyun Kim 2026-04-20 5:13 ` [PATCH 2/2] HID: appletb-kbd: run inactivity autodim from workqueues Sangyun Kim 2026-04-20 12:47 ` [PATCH 0/2] HID: appletb-kbd: fix UAF and mutex-in-atomic in inactivity timer Aditya Garg 2 siblings, 0 replies; 6+ messages in thread From: Sangyun Kim @ 2026-04-20 5:13 UTC (permalink / raw) To: jikos, bentiss; +Cc: qasdev00, gargaditya08, linux-input, linux-kernel, stable Commit 38224c472a03 ("HID: appletb-kbd: fix slab use-after-free bug in appletb_kbd_probe") added timer_delete_sync(&kbd->inactivity_timer) to both the probe close_hw error path and appletb_kbd_remove(), but the way it was wired in left the inactivity timer reachable during driver tear-down via two distinct windows. Window A -- put_device() before timer_delete_sync(): put_device(&kbd->backlight_dev->dev); timer_delete_sync(&kbd->inactivity_timer); The inactivity_timer softirq reads kbd->backlight_dev and calls backlight_device_set_brightness() -> mutex_lock(&ops_lock). If a concurrent hid_appletb_bl unbind drops the last devm reference between these two calls, the backlight_device is freed and the mutex_lock() touches freed memory. Window B -- backlight cleanup before hid_hw_stop(): if (kbd->backlight_dev) { timer_delete_sync(...); put_device(...); } hid_hw_close(hdev); hid_hw_stop(hdev); Even after Window A is closed, hid_hw_close()/hid_hw_stop() still run afterwards, so a late ".event" callback from the HID core (USB URB completion on real Apple hardware) can arrive after timer_delete_sync() drained the softirq but before put_device() drops the reference. That callback reaches reset_inactivity_timer(), which calls mod_timer() and re-arms the timer. The freshly re-armed timer can then fire on the about-to-be-freed backlight_device. Both windows produce the same KASAN slab-use-after-free: BUG: KASAN: slab-use-after-free in __mutex_lock+0x1aab/0x21c0 Read of size 8 at addr ffff88803ee9a108 by task swapper/0/0 Call Trace: <IRQ> __mutex_lock backlight_device_set_brightness appletb_inactivity_timer call_timer_fn run_timer_softirq handle_softirqs Allocated by task N: devm_backlight_device_register appletb_bl_probe Freed by task M: (concurrent hid_appletb_bl unbind path) Close both windows at once by reworking the tear-down in appletb_kbd_remove() and in the probe close_hw error path so that 1) hid_hw_close()/hid_hw_stop() run before the backlight cleanup, guaranteeing no further .event callback can fire and re-arm the timer, and 2) inside the "if (kbd->backlight_dev)" block, timer_delete_sync() runs before put_device(), so the softirq is drained before the final reference is dropped. Fixes: 38224c472a03 ("HID: appletb-kbd: fix slab use-after-free bug in appletb_kbd_probe") Cc: stable@vger.kernel.org Signed-off-by: Sangyun Kim <sangyun.kim@snu.ac.kr> --- drivers/hid/hid-appletb-kbd.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/hid/hid-appletb-kbd.c b/drivers/hid/hid-appletb-kbd.c index 0fdc0968b9ef..8feac9e3589b 100644 --- a/drivers/hid/hid-appletb-kbd.c +++ b/drivers/hid/hid-appletb-kbd.c @@ -440,13 +440,13 @@ static int appletb_kbd_probe(struct hid_device *hdev, const struct hid_device_id unregister_handler: input_unregister_handler(&kbd->inp_handler); close_hw: - if (kbd->backlight_dev) { - put_device(&kbd->backlight_dev->dev); - timer_delete_sync(&kbd->inactivity_timer); - } hid_hw_close(hdev); stop_hw: hid_hw_stop(hdev); + if (kbd->backlight_dev) { + timer_delete_sync(&kbd->inactivity_timer); + put_device(&kbd->backlight_dev->dev); + } return ret; } @@ -457,13 +457,13 @@ static void appletb_kbd_remove(struct hid_device *hdev) appletb_kbd_set_mode(kbd, APPLETB_KBD_MODE_OFF); input_unregister_handler(&kbd->inp_handler); + hid_hw_close(hdev); + hid_hw_stop(hdev); + if (kbd->backlight_dev) { - put_device(&kbd->backlight_dev->dev); timer_delete_sync(&kbd->inactivity_timer); + put_device(&kbd->backlight_dev->dev); } - - hid_hw_close(hdev); - hid_hw_stop(hdev); } static int appletb_kbd_suspend(struct hid_device *hdev, pm_message_t msg) -- 2.34.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] HID: appletb-kbd: run inactivity autodim from workqueues 2026-04-20 5:13 [PATCH 0/2] HID: appletb-kbd: fix UAF and mutex-in-atomic in inactivity timer Sangyun Kim 2026-04-20 5:13 ` [PATCH 1/2] HID: appletb-kbd: fix UAF in inactivity-timer cleanup path Sangyun Kim @ 2026-04-20 5:13 ` Sangyun Kim 2026-04-20 12:47 ` [PATCH 0/2] HID: appletb-kbd: fix UAF and mutex-in-atomic in inactivity timer Aditya Garg 2 siblings, 0 replies; 6+ messages in thread From: Sangyun Kim @ 2026-04-20 5:13 UTC (permalink / raw) To: jikos, bentiss; +Cc: qasdev00, gargaditya08, linux-input, linux-kernel, stable The autodim code in hid-appletb-kbd takes backlight_device->ops_lock via backlight_device_set_brightness() -> mutex_lock() from two different atomic contexts: * appletb_inactivity_timer() is a struct timer_list callback, so it runs in softirq context. Every expiry triggers BUG: sleeping function called from invalid context at kernel/locking/mutex.c:591 Call Trace: <IRQ> __might_resched __mutex_lock backlight_device_set_brightness appletb_inactivity_timer call_timer_fn run_timer_softirq * reset_inactivity_timer() is called from appletb_kbd_hid_event() and appletb_kbd_inp_event(). On real USB hardware these run in softirq/IRQ context (URB completion and input-event dispatch). When the Touch Bar has already been dimmed or turned off, the reset path calls backlight_device_set_brightness() directly to restore brightness, producing the same warning. Both call sites hit the same mutex_lock()-from-atomic bug. Fix them together by moving the blocking work onto the system workqueue: * Convert the inactivity timer from struct timer_list to struct delayed_work; the callback (appletb_inactivity_work) now runs in process context where mutex_lock() is legal. * Add a dedicated struct work_struct restore_brightness_work and have reset_inactivity_timer() schedule it instead of calling backlight_device_set_brightness() directly. Cancel both works synchronously during driver tear-down alongside the existing backlight reference drop. The semantics are unchanged (same delays, same state transitions on dim, turn-off and user activity); only the execution context of the sleeping call changes. The timer field and callback are renamed to match their new type; reset_inactivity_timer() keeps its name because it is invoked from input event paths that read naturally as "reset the inactivity timer". Fixes: 93a0fc489481 ("HID: hid-appletb-kbd: add support for automatic brightness control while using the touchbar") Cc: stable@vger.kernel.org Signed-off-by: Sangyun Kim <sangyun.kim@snu.ac.kr> --- drivers/hid/hid-appletb-kbd.c | 44 ++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/drivers/hid/hid-appletb-kbd.c b/drivers/hid/hid-appletb-kbd.c index 8feac9e3589b..462010a75899 100644 --- a/drivers/hid/hid-appletb-kbd.c +++ b/drivers/hid/hid-appletb-kbd.c @@ -17,7 +17,7 @@ #include <linux/module.h> #include <linux/string.h> #include <linux/backlight.h> -#include <linux/timer.h> +#include <linux/workqueue.h> #include <linux/input/sparse-keymap.h> #include "hid-ids.h" @@ -62,7 +62,8 @@ struct appletb_kbd { struct input_handle kbd_handle; struct input_handle tpd_handle; struct backlight_device *backlight_dev; - struct timer_list inactivity_timer; + struct delayed_work inactivity_work; + struct work_struct restore_brightness_work; bool has_dimmed; bool has_turned_off; u8 saved_mode; @@ -164,16 +165,18 @@ static int appletb_tb_key_to_slot(unsigned int code) } } -static void appletb_inactivity_timer(struct timer_list *t) +static void appletb_inactivity_work(struct work_struct *work) { - struct appletb_kbd *kbd = timer_container_of(kbd, t, inactivity_timer); + struct appletb_kbd *kbd = container_of(to_delayed_work(work), + struct appletb_kbd, + inactivity_work); if (kbd->backlight_dev && appletb_tb_autodim) { if (!kbd->has_dimmed) { backlight_device_set_brightness(kbd->backlight_dev, 1); kbd->has_dimmed = true; - mod_timer(&kbd->inactivity_timer, - jiffies + secs_to_jiffies(appletb_tb_idle_timeout)); + mod_delayed_work(system_wq, &kbd->inactivity_work, + secs_to_jiffies(appletb_tb_idle_timeout)); } else if (!kbd->has_turned_off) { backlight_device_set_brightness(kbd->backlight_dev, 0); kbd->has_turned_off = true; @@ -181,16 +184,25 @@ static void appletb_inactivity_timer(struct timer_list *t) } } +static void appletb_restore_brightness_work(struct work_struct *work) +{ + struct appletb_kbd *kbd = container_of(work, struct appletb_kbd, + restore_brightness_work); + + if (kbd->backlight_dev) + backlight_device_set_brightness(kbd->backlight_dev, 2); +} + static void reset_inactivity_timer(struct appletb_kbd *kbd) { if (kbd->backlight_dev && appletb_tb_autodim) { if (kbd->has_dimmed || kbd->has_turned_off) { - backlight_device_set_brightness(kbd->backlight_dev, 2); kbd->has_dimmed = false; kbd->has_turned_off = false; + schedule_work(&kbd->restore_brightness_work); } - mod_timer(&kbd->inactivity_timer, - jiffies + secs_to_jiffies(appletb_tb_dim_timeout)); + mod_delayed_work(system_wq, &kbd->inactivity_work, + secs_to_jiffies(appletb_tb_dim_timeout)); } } @@ -408,9 +420,11 @@ static int appletb_kbd_probe(struct hid_device *hdev, const struct hid_device_id dev_err_probe(dev, -ENODEV, "Failed to get backlight device\n"); } else { backlight_device_set_brightness(kbd->backlight_dev, 2); - timer_setup(&kbd->inactivity_timer, appletb_inactivity_timer, 0); - mod_timer(&kbd->inactivity_timer, - jiffies + secs_to_jiffies(appletb_tb_dim_timeout)); + INIT_DELAYED_WORK(&kbd->inactivity_work, appletb_inactivity_work); + INIT_WORK(&kbd->restore_brightness_work, + appletb_restore_brightness_work); + mod_delayed_work(system_wq, &kbd->inactivity_work, + secs_to_jiffies(appletb_tb_dim_timeout)); } kbd->inp_handler.event = appletb_kbd_inp_event; @@ -444,7 +458,8 @@ static int appletb_kbd_probe(struct hid_device *hdev, const struct hid_device_id stop_hw: hid_hw_stop(hdev); if (kbd->backlight_dev) { - timer_delete_sync(&kbd->inactivity_timer); + cancel_delayed_work_sync(&kbd->inactivity_work); + cancel_work_sync(&kbd->restore_brightness_work); put_device(&kbd->backlight_dev->dev); } return ret; @@ -461,7 +476,8 @@ static void appletb_kbd_remove(struct hid_device *hdev) hid_hw_stop(hdev); if (kbd->backlight_dev) { - timer_delete_sync(&kbd->inactivity_timer); + cancel_delayed_work_sync(&kbd->inactivity_work); + cancel_work_sync(&kbd->restore_brightness_work); put_device(&kbd->backlight_dev->dev); } } -- 2.34.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] HID: appletb-kbd: fix UAF and mutex-in-atomic in inactivity timer 2026-04-20 5:13 [PATCH 0/2] HID: appletb-kbd: fix UAF and mutex-in-atomic in inactivity timer Sangyun Kim 2026-04-20 5:13 ` [PATCH 1/2] HID: appletb-kbd: fix UAF in inactivity-timer cleanup path Sangyun Kim 2026-04-20 5:13 ` [PATCH 2/2] HID: appletb-kbd: run inactivity autodim from workqueues Sangyun Kim @ 2026-04-20 12:47 ` Aditya Garg 2026-04-22 6:01 ` Sangyun Kim 2 siblings, 1 reply; 6+ messages in thread From: Aditya Garg @ 2026-04-20 12:47 UTC (permalink / raw) To: Sangyun Kim, jikos, bentiss; +Cc: qasdev00, linux-input, linux-kernel 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? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] HID: appletb-kbd: fix UAF and mutex-in-atomic in inactivity timer 2026-04-20 12:47 ` [PATCH 0/2] HID: appletb-kbd: fix UAF and mutex-in-atomic in inactivity timer Aditya Garg @ 2026-04-22 6:01 ` Sangyun Kim 2026-04-22 9:15 ` Aditya Garg 0 siblings, 1 reply; 6+ messages in thread From: Sangyun Kim @ 2026-04-22 6:01 UTC (permalink / raw) To: Aditya Garg; +Cc: jikos, bentiss, qasdev00, linux-input, linux-kernel 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] HID: appletb-kbd: fix UAF and mutex-in-atomic in inactivity timer 2026-04-22 6:01 ` Sangyun Kim @ 2026-04-22 9:15 ` Aditya Garg 0 siblings, 0 replies; 6+ messages in thread From: Aditya Garg @ 2026-04-22 9:15 UTC (permalink / raw) To: Sangyun Kim Cc: jikos@kernel.org, bentiss@kernel.org, qasdev00@gmail.com, linux-input@vger.kernel.org, linux-kernel@vger.kernel.org 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@snu.ac.kr> 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-04-22 9:15 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-04-20 5:13 [PATCH 0/2] HID: appletb-kbd: fix UAF and mutex-in-atomic in inactivity timer Sangyun Kim 2026-04-20 5:13 ` [PATCH 1/2] HID: appletb-kbd: fix UAF in inactivity-timer cleanup path Sangyun Kim 2026-04-20 5:13 ` [PATCH 2/2] HID: appletb-kbd: run inactivity autodim from workqueues Sangyun Kim 2026-04-20 12:47 ` [PATCH 0/2] HID: appletb-kbd: fix UAF and mutex-in-atomic in inactivity timer Aditya Garg 2026-04-22 6:01 ` Sangyun Kim 2026-04-22 9:15 ` Aditya Garg
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox