[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

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Multiple HID drivers suffer from race conditions during device removal due to incorrect or missing timer teardown.
- [High] In `hid-sony.c`, `sony_remove()` fails to synchronously kill the `ghl_urb` before freeing it, leading to a Use-After-Free in the URB callback.
--

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

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

uclogic_remove() cancels the pen in-range timer and then stops the
device. 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.

> diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
> index b73f09d26688a..0b8a83fa6c5be 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);

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing this
fix for the teardown race in uclogic, I noticed multiple other HID drivers
suffer from similar pre-existing race conditions during device removal.

For instance, drivers/hid/hid-letsketch.c lacks a .remove callback entirely
in letsketch_driver:

static struct hid_driver letsketch_driver = {
	.name = "letsketch",
	.id_table = letsketch_devices,
	.probe = letsketch_probe,
	.raw_event = letsketch_raw_event,
};

This means the inrange_timer is never explicitly canceled and can outlive
the driver's devres cleanup.

In drivers/hid/hid-multitouch.c mt_remove() and drivers/hid/hid-prodikeys.c
pk_remove(), the code calls timer_delete_sync() before hid_hw_stop(), leaving
the exact same window where incoming reports can re-arm the timer.

Additionally, in drivers/hid/hid-appleir.c appleir_remove() calls
hid_hw_stop() before timer_delete_sync(). Because hid_hw_stop() frees the
hid-input device, this can cause a use-after-free if the timer fires in this
window.

Would it make sense to apply this same timer_shutdown_sync() pattern and
add the missing remove callbacks to these other HID drivers as well?

[Severity: High]
This is also a pre-existing issue, but while looking at these removal paths,
I spotted another teardown race in drivers/hid/hid-sony.c during sony_remove().

The cleanup sequence is:

    timer_delete_sync(&sc->ghl_poke_timer);
    usb_free_urb(sc->ghl_urb);

If ghl_poke_timer fires right before it is canceled, it submits ghl_urb.
The usb_free_urb() call only decrements the reference count; it does not
cancel an in-flight transfer.

When the URB later completes, ghl_magic_poke_cb() will dereference the
freed sc pointer to re-arm the timer, leading to a use-after-free.

Should this code be using usb_kill_urb() prior to freeing the URB to
ensure any in-flight transfers are synchronously terminated?

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

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