[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)

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Reordering hid_hw_stop() before timer_delete_sync() introduces a Use-After-Free on the input_dev structure.
--

--- Patch [1]: [PATCH] HID: uclogic: fix use-after-free of inrange_timer on remove ---
commit 43bb5d633bc4358be3fe68adce4de35e0008e236
Author: Ibrahim Hashimov <security@auditcode.ai>

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

This commit attempts to fix a timer re-arming race condition during driver
removal by stopping the hardware with hid_hw_stop() before calling
timer_delete_sync() on the inrange_timer.

> diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
> index b73f09d26688a..925178d8feb40 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);
> +	/*

[ ... ]

> +	 * 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);

[Severity: High]
Does this reordering introduce a use-after-free on the input device memory?

When hid_hw_stop() returns, it has already disconnected and freed the input
devices, including the one pointed to by drvdata->pen_input. If the 100ms
timer fires after the input devices are freed but before timer_delete_sync()
completes, uclogic_inrange_timeout() will run and dereference the dangling
drvdata->pen_input pointer:

drivers/hid/hid-uclogic-core.c:uclogic_inrange_timeout()
    ...
    input_report_abs(input, ABS_PRESSURE, 0);

To prevent the timer from being re-armed without violating the safe teardown
order, would it be better to keep the timer cancellation before hid_hw_stop()
but replace timer_delete_sync() with timer_shutdown_sync()?

>  	kfree(drvdata->desc_ptr);
>  	uclogic_params_cleanup(&drvdata->params);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713121042.2321-1-security@auditcode.ai?part=1