linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] HID: appletb-kbd: fix slab use-after-free bug in appletb_kbd_probe
@ 2025-06-23 23:08 Qasim Ijaz
  2025-06-24  5:27 ` Aditya Garg
  0 siblings, 1 reply; 3+ messages in thread
From: Qasim Ijaz @ 2025-06-23 23:08 UTC (permalink / raw)
  To: jikos, bentiss; +Cc: gargaditya08, linux-input, linux-kernel, stable

In probe appletb_kbd_probe() a "struct appletb_kbd *kbd" is allocated
via devm_kzalloc() to store touch bar keyboard related data.
Later on if backlight_device_get_by_name() finds a backlight device
with name "appletb_backlight" a timer (kbd->inactivity_timer) is setup
with appletb_inactivity_timer() and the timer is armed to run after
appletb_tb_dim_timeout (60) seconds.

A use-after-free is triggered when failure occurs after the timer is 
armed. This ultimately means probe failure occurs and as a result the 
"struct appletb_kbd *kbd" which is device managed memory is freed. 
After 60 seconds the timer will have expired and __run_timers will 
attempt to access the timer (kbd->inactivity_timer) however the kdb 
structure has been freed causing a use-after free.

[   71.636938] ==================================================================
[   71.637915] BUG: KASAN: slab-use-after-free in __run_timers+0x7ad/0x890
[   71.637915] Write of size 8 at addr ffff8881178c5958 by task swapper/1/0
[   71.637915] 
[   71.637915] CPU: 1 UID: 0 PID: 0 Comm: swapper/1 Not tainted 6.16.0-rc2-00318-g739a6c93cc75-dirty #12 PREEMPT(voluntary) 
[   71.637915] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[   71.637915] Call Trace:
[   71.637915]  <IRQ>
[   71.637915]  dump_stack_lvl+0x53/0x70
[   71.637915]  print_report+0xce/0x670
[   71.637915]  ? __run_timers+0x7ad/0x890
[   71.637915]  kasan_report+0xce/0x100
[   71.637915]  ? __run_timers+0x7ad/0x890
[   71.637915]  __run_timers+0x7ad/0x890
[   71.637915]  ? __pfx___run_timers+0x10/0x10
[   71.637915]  ? update_process_times+0xfc/0x190
[   71.637915]  ? __pfx_update_process_times+0x10/0x10
[   71.637915]  ? _raw_spin_lock_irq+0x80/0xe0
[   71.637915]  ? _raw_spin_lock_irq+0x80/0xe0
[   71.637915]  ? __pfx__raw_spin_lock_irq+0x10/0x10
[   71.637915]  run_timer_softirq+0x141/0x240
[   71.637915]  ? __pfx_run_timer_softirq+0x10/0x10
[   71.637915]  ? __pfx___hrtimer_run_queues+0x10/0x10
[   71.637915]  ? kvm_clock_get_cycles+0x18/0x30
[   71.637915]  ? ktime_get+0x60/0x140
[   71.637915]  handle_softirqs+0x1b8/0x5c0
[   71.637915]  ? __pfx_handle_softirqs+0x10/0x10
[   71.637915]  irq_exit_rcu+0xaf/0xe0
[   71.637915]  sysvec_apic_timer_interrupt+0x6c/0x80
[   71.637915]  </IRQ>
[   71.637915] 
[   71.637915] Allocated by task 39:
[   71.637915]  kasan_save_stack+0x33/0x60
[   71.637915]  kasan_save_track+0x14/0x30
[   71.637915]  __kasan_kmalloc+0x8f/0xa0
[   71.637915]  __kmalloc_node_track_caller_noprof+0x195/0x420
[   71.637915]  devm_kmalloc+0x74/0x1e0
[   71.637915]  appletb_kbd_probe+0x37/0x3c0
[   71.637915]  hid_device_probe+0x2d1/0x680
[   71.637915]  really_probe+0x1c3/0x690
[   71.637915]  __driver_probe_device+0x247/0x300
[   71.637915]  driver_probe_device+0x49/0x210
[...]
[   71.637915] 
[   71.637915] Freed by task 39:
[   71.637915]  kasan_save_stack+0x33/0x60
[   71.637915]  kasan_save_track+0x14/0x30
[   71.637915]  kasan_save_free_info+0x3b/0x60
[   71.637915]  __kasan_slab_free+0x37/0x50
[   71.637915]  kfree+0xcf/0x360
[   71.637915]  devres_release_group+0x1f8/0x3c0
[   71.637915]  hid_device_probe+0x315/0x680
[   71.637915]  really_probe+0x1c3/0x690
[   71.637915]  __driver_probe_device+0x247/0x300
[   71.637915]  driver_probe_device+0x49/0x210
[...]

The root cause of the issue is that the timer is not disarmed 
on failure paths leading to it remaining active and accessing 
freed memory. To fix this call timer_delete_sync() to deactivate 
the 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: Qasim Ijaz <qasdev00@gmail.com>
---
 drivers/hid/hid-appletb-kbd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-appletb-kbd.c b/drivers/hid/hid-appletb-kbd.c
index e06567886e50..a70b1c519105 100644
--- a/drivers/hid/hid-appletb-kbd.c
+++ b/drivers/hid/hid-appletb-kbd.c
@@ -438,8 +438,10 @@ static int appletb_kbd_probe(struct hid_device *hdev, const struct hid_device_id
 	return 0;
 
 close_hw:
-	if (kbd->backlight_dev)
+	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);
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] HID: appletb-kbd: fix slab use-after-free bug in appletb_kbd_probe
  2025-06-23 23:08 [PATCH] HID: appletb-kbd: fix slab use-after-free bug in appletb_kbd_probe Qasim Ijaz
@ 2025-06-24  5:27 ` Aditya Garg
  2025-06-24 11:56   ` Qasim Ijaz
  0 siblings, 1 reply; 3+ messages in thread
From: Aditya Garg @ 2025-06-24  5:27 UTC (permalink / raw)
  To: Qasim Ijaz, jikos, bentiss; +Cc: linux-input, linux-kernel, stable



On 24 June 2025 4:38:12 am IST, Qasim Ijaz <qasdev00@gmail.com> wrote:
>In probe appletb_kbd_probe() a "struct appletb_kbd *kbd" is allocated
>via devm_kzalloc() to store touch bar keyboard related data.
>Later on if backlight_device_get_by_name() finds a backlight device
>with name "appletb_backlight" a timer (kbd->inactivity_timer) is setup
>with appletb_inactivity_timer() and the timer is armed to run after
>appletb_tb_dim_timeout (60) seconds.
>
>A use-after-free is triggered when failure occurs after the timer is 
>armed. This ultimately means probe failure occurs and as a result the 
>"struct appletb_kbd *kbd" which is device managed memory is freed. 
>After 60 seconds the timer will have expired and __run_timers will 
>attempt to access the timer (kbd->inactivity_timer) however the kdb 
>structure has been freed causing a use-after free.
>
>[   71.636938] ==================================================================
>[   71.637915] BUG: KASAN: slab-use-after-free in __run_timers+0x7ad/0x890
>[   71.637915] Write of size 8 at addr ffff8881178c5958 by task swapper/1/0
>[   71.637915] 
>[   71.637915] CPU: 1 UID: 0 PID: 0 Comm: swapper/1 Not tainted 6.16.0-rc2-00318-g739a6c93cc75-dirty #12 PREEMPT(voluntary) 
>[   71.637915] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
>[   71.637915] Call Trace:
>[   71.637915]  <IRQ>
>[   71.637915]  dump_stack_lvl+0x53/0x70
>[   71.637915]  print_report+0xce/0x670
>[   71.637915]  ? __run_timers+0x7ad/0x890
>[   71.637915]  kasan_report+0xce/0x100
>[   71.637915]  ? __run_timers+0x7ad/0x890
>[   71.637915]  __run_timers+0x7ad/0x890
>[   71.637915]  ? __pfx___run_timers+0x10/0x10
>[   71.637915]  ? update_process_times+0xfc/0x190
>[   71.637915]  ? __pfx_update_process_times+0x10/0x10
>[   71.637915]  ? _raw_spin_lock_irq+0x80/0xe0
>[   71.637915]  ? _raw_spin_lock_irq+0x80/0xe0
>[   71.637915]  ? __pfx__raw_spin_lock_irq+0x10/0x10
>[   71.637915]  run_timer_softirq+0x141/0x240
>[   71.637915]  ? __pfx_run_timer_softirq+0x10/0x10
>[   71.637915]  ? __pfx___hrtimer_run_queues+0x10/0x10
>[   71.637915]  ? kvm_clock_get_cycles+0x18/0x30
>[   71.637915]  ? ktime_get+0x60/0x140
>[   71.637915]  handle_softirqs+0x1b8/0x5c0
>[   71.637915]  ? __pfx_handle_softirqs+0x10/0x10
>[   71.637915]  irq_exit_rcu+0xaf/0xe0
>[   71.637915]  sysvec_apic_timer_interrupt+0x6c/0x80
>[   71.637915]  </IRQ>
>[   71.637915] 
>[   71.637915] Allocated by task 39:
>[   71.637915]  kasan_save_stack+0x33/0x60
>[   71.637915]  kasan_save_track+0x14/0x30
>[   71.637915]  __kasan_kmalloc+0x8f/0xa0
>[   71.637915]  __kmalloc_node_track_caller_noprof+0x195/0x420
>[   71.637915]  devm_kmalloc+0x74/0x1e0
>[   71.637915]  appletb_kbd_probe+0x37/0x3c0
>[   71.637915]  hid_device_probe+0x2d1/0x680
>[   71.637915]  really_probe+0x1c3/0x690
>[   71.637915]  __driver_probe_device+0x247/0x300
>[   71.637915]  driver_probe_device+0x49/0x210
>[...]
>[   71.637915] 
>[   71.637915] Freed by task 39:
>[   71.637915]  kasan_save_stack+0x33/0x60
>[   71.637915]  kasan_save_track+0x14/0x30
>[   71.637915]  kasan_save_free_info+0x3b/0x60
>[   71.637915]  __kasan_slab_free+0x37/0x50
>[   71.637915]  kfree+0xcf/0x360
>[   71.637915]  devres_release_group+0x1f8/0x3c0
>[   71.637915]  hid_device_probe+0x315/0x680
>[   71.637915]  really_probe+0x1c3/0x690
>[   71.637915]  __driver_probe_device+0x247/0x300
>[   71.637915]  driver_probe_device+0x49/0x210
>[...]
>
>The root cause of the issue is that the timer is not disarmed 
>on failure paths leading to it remaining active and accessing 
>freed memory. To fix this call timer_delete_sync() to deactivate 
>the 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: Qasim Ijaz <qasdev00@gmail.com>
>---
> drivers/hid/hid-appletb-kbd.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/hid/hid-appletb-kbd.c b/drivers/hid/hid-appletb-kbd.c
>index e06567886e50..a70b1c519105 100644
>--- a/drivers/hid/hid-appletb-kbd.c
>+++ b/drivers/hid/hid-appletb-kbd.c
>@@ -438,8 +438,10 @@ static int appletb_kbd_probe(struct hid_device *hdev, const struct hid_device_id
> 	return 0;
> 
> close_hw:
>-	if (kbd->backlight_dev)
>+	if (kbd->backlight_dev) {
> 		put_device(&kbd->backlight_dev->dev);
>+		timer_delete_sync(&kbd->inactivity_timer);
>+	}

Ok this makes sense. Probably execute time_delete_sync only when there is kbd->backlight_dev in appletb_kbd_remove as well. Currently it is being executed unconditionally.

> 	hid_hw_close(hdev);
> stop_hw:
> 	hid_hw_stop(hdev);

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] HID: appletb-kbd: fix slab use-after-free bug in appletb_kbd_probe
  2025-06-24  5:27 ` Aditya Garg
@ 2025-06-24 11:56   ` Qasim Ijaz
  0 siblings, 0 replies; 3+ messages in thread
From: Qasim Ijaz @ 2025-06-24 11:56 UTC (permalink / raw)
  To: Aditya Garg; +Cc: jikos, bentiss, linux-input, linux-kernel, stable

On Tue, Jun 24, 2025 at 10:57:05AM +0530, Aditya Garg wrote:
> Ok this makes sense. Probably execute time_delete_sync only when there is kbd->backlight_dev in appletb_kbd_remove as well. Currently it is being executed unconditionally.

Good point, will resend v2

Thanks,
Qasim
> 
> > 	hid_hw_close(hdev);
> > stop_hw:
> > 	hid_hw_stop(hdev);

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-06-24 11:56 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-23 23:08 [PATCH] HID: appletb-kbd: fix slab use-after-free bug in appletb_kbd_probe Qasim Ijaz
2025-06-24  5:27 ` Aditya Garg
2025-06-24 11:56   ` Qasim Ijaz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).