Re: [PATCH v2] HID: appleir: fix UAF on pending key_up_timer in remove()

[Jiri Kosina]

On Fri, 15 May 2026, Manish Khadka wrote:

> 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 4db2af929279 ("HID:
> appletb-kbd: fix UAF in inactivity-timer cleanup path") and has been
> present since the driver was introduced.
> 
> Fixes: 9a4a5574ce42 ("HID: appleir: add support for Apple ir devices")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Manish Khadka <maskmemanish@xxxxxxxxx>

Applied, thank you.

-- 
Jiri Kosina
SUSE Labs