[PATCH] HID: uclogic: fix use-after-free of inrange_timer on remove

[PATCH] HID: uclogic: fix use-after-free of inrange_timer on remove

[Ibrahim Hashimov]

uclogic_remove() tears down in the wrong order:

	timer_delete_sync(&drvdata->inrange_timer);
	hid_hw_stop(hdev);

hid_hw_stop() is what actually quiesces the device: it calls
hid_disconnect() (which frees hid->inputs via hidinput_disconnect())
and only then stops the underlying transport, which is what finally
kills the still-submitted interrupt-IN URB. Until hid_hw_stop()
returns, the device can still deliver pen reports, and every pen
report with pen->inrange == UCLOGIC_PARAMS_PEN_INRANGE_NONE runs:

	mod_timer(&drvdata->inrange_timer, jiffies + msecs_to_jiffies(100));

from uclogic_raw_event_pen(). Calling timer_delete_sync() *before*
hid_hw_stop() only guarantees the timer is idle at that instant - it
does not guarantee it stays idle, because the report path that can
re-arm it is not shut off until hid_hw_stop() returns, several lines
later. A report landing in that window re-arms inrange_timer after
it was supposedly cancelled; uclogic_remove() then returns,
devm_kzalloc() frees drvdata (and hidinput_disconnect() has already
freed the input device drvdata->pen_input points at), and roughly
100 ms later uclogic_inrange_timeout() fires on that freed memory -
a use-after-free in timer-softirq context.

Reaching that window needs nothing exotic: a malicious or
malfunctioning UC-Logic/Huion USB device (or a manual sysfs unbind
racing in-flight reports from a legitimate one) delivering a single
pen report while uclogic_remove() is running is enough to retrigger
mod_timer() after the old timer_delete_sync() call.

Fix this the same way several other HID drivers whose timers are
re-armed from the report path already do it (e.g. hid-appleir.c's
key_up_timer, hid-nvidia-shield.c's psy_stats_timer,
hid-wiimote-core.c's timer, wacom_sys.c's idleprox_timer): quiesce
the hardware with hid_hw_stop() first, then delete the timer.
Once hid_hw_stop() has returned, uclogic_raw_event_pen() cannot run
again for this device, so the subsequent timer_delete_sync() is
guaranteed to be the last write to inrange_timer - there is no
window left in which a report can re-arm it.

This has been verified at runtime on a v6.19 KASAN-instrumented
kernel: a synthesized malicious USB UC-Logic device flooding pen
reports across uclogic_remove() reliably trips a KASAN use-after-free
on the freed hid->inputs list before this fix, in usbhid's
hid_check_keys_pressed()/hid_irq_in() report-delivery path rather
than in uclogic_inrange_timeout() itself, because that synchronous
fault preempts the ~100 ms timer callback. This reorder closes the
inrange_timer race by construction (timer_delete_sync() is now
provably the last write to it), but that captured report-path fault
is a separate, core-level race in usbhid and is not fixed by this
driver-local change.

Fixes: 01309e29eb95 ("HID: uclogic: Support in-range reporting emulation")
Cc: stable@vger.kernel.org
</content>

Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
Assisted-by: AuditCode-AI:2026.07
---
 drivers/hid/hid-uclogic-core.c | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
index b73f09d26688..925178d8feb4 100644
--- a/drivers/hid/hid-uclogic-core.c
+++ b/drivers/hid/hid-uclogic-core.c
@@ -548,8 +548,35 @@ static void uclogic_remove(struct hid_device *hdev)
 {
 	struct uclogic_drvdata *drvdata = hid_get_drvdata(hdev);
 
-	timer_delete_sync(&drvdata->inrange_timer);
+	/*
+	 * Quiesce the device, and the report-delivery path it feeds, before
+	 * tearing down anything the report path can still touch.
+	 *
+	 * hid_hw_stop() disconnects hid->inputs (hid_disconnect() ->
+	 * hidinput_disconnect()) and stops the underlying transport. Only
+	 * once it has returned is uclogic_raw_event_pen() guaranteed unable
+	 * to run again, since that is the only place inrange_timer gets
+	 * re-armed (mod_timer() on every report while pen->inrange ==
+	 * UCLOGIC_PARAMS_PEN_INRANGE_NONE).
+	 *
+	 * Calling timer_delete_sync() first, as the old order did, cancels
+	 * the timer while the device can still deliver reports: a report
+	 * processed between the timer_delete_sync() call and the eventual
+	 * URB shutdown inside hid_hw_stop() re-arms inrange_timer, which is
+	 * now left pending across the devm_kzalloc()'d drvdata (and
+	 * drvdata->pen_input) being freed. uclogic_inrange_timeout() then
+	 * fires ~100 ms later on freed memory.
+	 *
+	 * Stopping the hardware first and only then deleting the timer
+	 * closes that window: hid_hw_stop() will not return until the
+	 * device can no longer feed uclogic_raw_event_pen(), so the
+	 * timer_delete_sync() call below is guaranteed to be the last write
+	 * to inrange_timer. This also matches the ordering already used by
+	 * other HID drivers whose timers are re-armed from the report path,
+	 * e.g. hid-appleir.c's key_up_timer.
+	 */
 	hid_hw_stop(hdev);
+	timer_delete_sync(&drvdata->inrange_timer);
 	kfree(drvdata->desc_ptr);
 	uclogic_params_cleanup(&drvdata->params);
 }
-- 
2.50.1 (Apple Git-155)

[PATCH v2] HID: uclogic: fix use-after-free of inrange_timer on remove

[Ibrahim Hashimov]

uclogic_remove() cancels the pen in-range timer and then stops the
device:

	timer_delete_sync(&drvdata->inrange_timer);
	hid_hw_stop(hdev);

timer_delete_sync() only guarantees the timer is idle at that instant.
uclogic_raw_event_pen() keeps delivering pen reports until hid_hw_stop()
stops the transport several lines later, and every report with
pen->inrange == UCLOGIC_PARAMS_PEN_INRANGE_NONE re-arms the timer:

	mod_timer(&drvdata->inrange_timer, jiffies + msecs_to_jiffies(100));

A report landing between the timer_delete_sync() call and the transport
teardown in hid_hw_stop() re-arms inrange_timer after it was cancelled.
uclogic_remove() then returns and the devm drvdata is freed, while
hid_hw_stop() has already freed the input device drvdata->pen_input
points at, so when the timer fires ~100 ms later
uclogic_inrange_timeout() dereferences freed memory -- a use-after-free
in timer-softirq context.

Swapping the two calls is not a fix: stopping the device first frees
drvdata->pen_input via hidinput_disconnect() while the timer may still
be pending, so a timer already armed before removal fires on the freed
input device in the window before timer_delete_sync() runs.

Use timer_shutdown_sync() before hid_hw_stop() instead. It cancels the
timer, waits for a running callback while pen_input is still valid, and
prevents any further re-arming -- a later mod_timer() from an in-flight
report is silently ignored -- so the timer is provably dead before
hid_hw_stop() frees the inputs. This is the ordering the timer core
documents for this "timer re-armed from another path" teardown case.

Fixes: 01309e29eb95 ("HID: uclogic: Support in-range reporting emulation")
Cc: stable@vger.kernel.org
Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
Assisted-by: AuditCode-AI:2026.07
---
v2: v1 reordered hid_hw_stop() before timer_delete_sync(), which traded
    the re-arm use-after-free for one on the freed input device (thanks
    to sashiko-bot for spotting it). Keep the original order and use
    timer_shutdown_sync() to disarm the timer, which closes the re-arm
    race without ever touching pen_input after it is freed.

 drivers/hid/hid-uclogic-core.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
index b73f09d26688..0b8a83fa6c5b 100644
--- a/drivers/hid/hid-uclogic-core.c
+++ b/drivers/hid/hid-uclogic-core.c
@@ -548,7 +548,17 @@ static void uclogic_remove(struct hid_device *hdev)
 {
 	struct uclogic_drvdata *drvdata = hid_get_drvdata(hdev);
 
-	timer_delete_sync(&drvdata->inrange_timer);
+	/*
+	 * Shut the in-range timer down before stopping the device.
+	 * uclogic_raw_event_pen() re-arms inrange_timer on every pen report
+	 * and keeps running until hid_hw_stop() stops the transport, so a
+	 * plain timer_delete_sync() here can be undone by a report landing in
+	 * the window before hid_hw_stop().  timer_shutdown_sync() cancels the
+	 * timer and makes any later re-arm a no-op, so it is provably dead
+	 * before hid_hw_stop() frees the input device drvdata->pen_input
+	 * points at.
+	 */
+	timer_shutdown_sync(&drvdata->inrange_timer);
 	hid_hw_stop(hdev);
 	kfree(drvdata->desc_ptr);
 	uclogic_params_cleanup(&drvdata->params);
-- 
2.50.1 (Apple Git-155)