github-mirror/linux
1
0
Fork 0
mirror of https://github.com/torvalds/linux.git synced 2026-07-27 01:32:21 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

HID: appleir: fix UAF on pending key_up_timer in remove()

Browse Source 
appleir_remove() runs hid_hw_stop() before timer_delete_sync().
hid_hw_stop() synchronously unregisters the HID input device via
hid_disconnect() -> hidinput_disconnect() -> input_unregister_device(),
which drops the last reference and frees the underlying input_dev when
no userspace handle holds it open.

key_up_tick() reads appleir->input_dev and calls input_report_key() /
input_sync() on it.  The timer is armed from appleir_raw_event() with
a HZ/8 (~125 ms) timeout on every keydown and key-repeat report.  If a
key was pressed shortly before the device is disconnected, the timer
can fire after hid_hw_stop() has freed input_dev but before the
teardown drains it.

A simple reorder is not sufficient.  Putting the timer drain first
still leaves a window where a USB URB completion (raw_event) running
during hid_hw_stop() can call mod_timer() and re-arm the timer, which
then fires after hidinput_disconnect() has freed input_dev.  The same
URB-completion window also lets raw_event() reach key_up(), key_down()
and battery_flat() directly, all of which dereference
appleir->input_dev.

Introduce a 'removing' flag on struct appleir, gated by the existing
spinlock.  appleir_remove() sets the flag under the lock and then
shuts down the timer with timer_shutdown_sync(), which both drains any
in-flight callback and permanently disables further mod_timer() calls.
appleir_raw_event() and key_up_tick() bail out early if the flag is
set, so no path can arm or run the timer, or dereference
appleir->input_dev, after remove() has started tearing down.

The keyrepeat and flatbattery branches of appleir_raw_event()
previously called into the input layer without holding the spinlock;
take it now so the flag check is well-defined.  This incidentally
closes a pre-existing read-side race on appleir->current_key in the
keyrepeat branch.

This bug is structurally a sibling of commit 4db2af9292 ("HID:
appletb-kbd: fix UAF in inactivity-timer cleanup path") and has been
present since the driver was introduced.

Fixes: 9a4a5574ce ("HID: appleir: add support for Apple ir devices")
Cc: stable@vger.kernel.org
Signed-off-by: Manish Khadka <maskmemanish@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
This commit is contained in:
Manish Khadka 2026-05-15 23:17:52 +05:45 committed by Jiri Kosina
parent 46c8beeccd
commit 75fe87e19d
1 changed files with 35 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
drivers/hid/hid-appleir.c
View File

@ -109,9 +109,10 @@ struct appleir {
struct hid_device *hid;
unsigned short keymap[ARRAY_SIZE(appleir_key_table)];
struct timer_list key_up_timer; /* timer for key up */
spinlock_t lock; /* protects .current_key */
spinlock_t lock; /* protects .current_key, .removing */
int current_key; /* the currently pressed key */
int prev_key_idx; /* key index in a 2 packets message */
bool removing; /* set during teardown; gates input_dev access */
};
static int get_key(int data)
@ -172,7 +173,7 @@ static void key_up_tick(struct timer_list *t)
unsigned long flags;
spin_lock_irqsave(&appleir->lock, flags);
if (appleir->current_key) {
if (!appleir->removing && appleir->current_key) {
key_up(hid, appleir, appleir->current_key);
appleir->current_key = 0;
}
@ -195,6 +196,10 @@ static int appleir_raw_event(struct hid_device *hid, struct hid_report *report,
int index;
spin_lock_irqsave(&appleir->lock, flags);
if (appleir->removing) {
spin_unlock_irqrestore(&appleir->lock, flags);
goto out;
}
/*
* If we already have a key down, take it up before marking
* this one down
@ -229,17 +234,25 @@ static int appleir_raw_event(struct hid_device *hid, struct hid_report *report,
appleir->prev_key_idx = 0;
if (!memcmp(data, keyrepeat, sizeof(keyrepeat))) {
key_down(hid, appleir, appleir->current_key);
/*
* Remote doesn't do key up, either pull them up, in the test
* above, or here set a timer which pulls them up after 1/8 s
*/
mod_timer(&appleir->key_up_timer, jiffies + HZ / 8);
spin_lock_irqsave(&appleir->lock, flags);
if (!appleir->removing) {
key_down(hid, appleir, appleir->current_key);
/*
* Remote doesn't do key up, either pull them up, in
* the test above, or here set a timer which pulls them
* up after 1/8 s
*/
mod_timer(&appleir->key_up_timer, jiffies + HZ / 8);
}
spin_unlock_irqrestore(&appleir->lock, flags);
goto out;
}
if (!memcmp(data, flatbattery, sizeof(flatbattery))) {
battery_flat(appleir);
spin_lock_irqsave(&appleir->lock, flags);
if (!appleir->removing)
battery_flat(appleir);
spin_unlock_irqrestore(&appleir->lock, flags);
/* Fall through */
}
@ -318,8 +331,20 @@ static int appleir_probe(struct hid_device *hid, const struct hid_device_id *id)
static void appleir_remove(struct hid_device *hid)
{
struct appleir *appleir = hid_get_drvdata(hid);
unsigned long flags;
/*
* Mark the driver as tearing down so that any concurrent raw_event
* (e.g. from a USB URB completion that hid_hw_stop() has not yet
* killed) and the key_up_timer softirq stop touching input_dev
* before hid_hw_stop() frees it via hidinput_disconnect().
*/
spin_lock_irqsave(&appleir->lock, flags);
appleir->removing = true;
spin_unlock_irqrestore(&appleir->lock, flags);
timer_shutdown_sync(&appleir->key_up_timer);
hid_hw_stop(hid);
timer_delete_sync(&appleir->key_up_timer);
}
static const struct hid_device_id appleir_devices[] = {