public inbox for linux-input@vger.kernel.org
 help / color / mirror / Atom feed
* [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