Commit 4db2af92 authored by Sangyun Kim's avatar Sangyun Kim Committed by Jiri Kosina
Browse files

HID: appletb-kbd: fix UAF in inactivity-timer cleanup path



Commit 38224c47 ("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: 38224c47 ("HID: appletb-kbd: fix slab use-after-free bug in appletb_kbd_probe")
Cc: stable@vger.kernel.org
Signed-off-by: default avatarSangyun Kim <sangyun.kim@snu.ac.kr>
Signed-off-by: default avatarJiri Kosina <jkosina@suse.com>
parent cac61b58
Loading
Loading
Loading
Loading
+8 −8
Original line number Diff line number Diff line
@@ -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)