[PATCH] HID: uclogic: fix UAF on inrange_timer at driver unbind

[PATCH] HID: uclogic: fix UAF on inrange_timer at driver unbind

[Doruk Tan Ozturk]

uclogic_remove() drained the in-range emulation timer before calling
hid_hw_stop():

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

The timer is re-armed from uclogic_raw_event_pen() with a 100 ms
timeout on every pen-in-range report via mod_timer(), and its callback
uclogic_inrange_timeout() dereferences drvdata to deliver a synthetic
BTN_TOOL_PEN release.

drvdata is allocated with devm_kzalloc(), so the HID core devm cleanup
frees it once uclogic_remove() returns. Because the timer is drained
before hid_hw_stop(), a pen report still completing inside
hid_hw_stop() can reach uclogic_raw_event_pen() and re-arm the timer
after the drain. devm then frees drvdata and the re-armed timer fires
on freed memory, a UAF read in uclogic_inrange_timeout(). This is a
disconnect race (USB unplug or rmmod).

Fix by mirroring the letsketch fix: call hid_hw_stop() first, which
synchronously kills the URBs that deliver raw_event(), so once it
returns no path can re-arm the timer. timer_shutdown_sync() then
drains any in-flight callback and permanently disables further
mod_timer() calls.

Found by 0sec (https://0sec.ai) using automated source analysis, as an
incomplete-fix twin of commit 46c8beeccd8a ("HID: letsketch: fix UAF on
inrange_timer at driver unbind"); not runtime-reproduced.

Fixes: 01309e29eb95 ("HID: uclogic: Support in-range reporting emulation")
Cc: stable@vger.kernel.org
Assisted-by: 0sec:claude-opus-4-8
Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
---
 drivers/hid/hid-uclogic-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
index b73f09d26688..c440013a609f 100644
--- a/drivers/hid/hid-uclogic-core.c
+++ b/drivers/hid/hid-uclogic-core.c
@@ -548,8 +548,8 @@ static void uclogic_remove(struct hid_device *hdev)
 {
 	struct uclogic_drvdata *drvdata = hid_get_drvdata(hdev);
 
-	timer_delete_sync(&drvdata->inrange_timer);
 	hid_hw_stop(hdev);
+	timer_shutdown_sync(&drvdata->inrange_timer);
 	kfree(drvdata->desc_ptr);
 	uclogic_params_cleanup(&drvdata->params);
 }
-- 
2.43.0

Re: [PATCH] HID: uclogic: fix UAF on inrange_timer at driver unbind

[sashiko-bot]

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

New issues:
- [Critical] Moving timer_shutdown_sync() after hid_hw_stop() introduces a Use-After-Free of drvdata->pen_input during driver unbind.

Pre-existing issues:
- [Critical] Missing timer cancellation in the uclogic_probe() error path leads to a Use-After-Free if initialization fails after I/O has started.
--

--- Patch [1]: [PATCH] HID: uclogic: fix UAF on inrange_timer at driver unbind ---
commit 0de50e5b91e3a9978c96d85e0aab45d43b0d98d6
Author: Doruk Tan Ozturk <doruk@0sec.ai>

HID: uclogic: fix UAF on inrange_timer at driver unbind

uclogic_remove() drained the in-range emulation timer before calling
hid_hw_stop(), leaving a window where a completing pen report could
re-arm the timer. This commit reorders the operations to call
hid_hw_stop() first, preventing re-arming before shutting down the timer.

> diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
> index b73f09d26688a..c440013a609fd 100644
> --- a/drivers/hid/hid-uclogic-core.c
> +++ b/drivers/hid/hid-uclogic-core.c
> @@ -548,8 +548,8 @@ static void uclogic_remove(struct hid_device *hdev)
>  {
>  	struct uclogic_drvdata *drvdata = hid_get_drvdata(hdev);
>  
> -	timer_delete_sync(&drvdata->inrange_timer);
>  	hid_hw_stop(hdev);
> +	timer_shutdown_sync(&drvdata->inrange_timer);

[Severity: Critical]
Can moving timer_shutdown_sync() after hid_hw_stop() introduce a
use-after-free on drvdata->pen_input?

hid_hw_stop() synchronously unregisters and frees the standard input
devices, which frees the memory pointed to by drvdata->pen_input.

If an incoming report arms the timer just before hid_hw_stop() is called,
the timer will remain active.

When it fires, the callback will unconditionally dereference the freed
pointer:

drivers/hid/hid-uclogic-core.c:uclogic_inrange_timeout()
	struct input_dev *input = drvdata->pen_input;

	if (input == NULL)
		return;
	input_report_abs(input, ABS_PRESSURE, 0);


[Severity: Critical]
This is a pre-existing issue, but can the uclogic_probe() error path
also lead to a use-after-free if initialization fails?

If hid_hw_start() briefly starts hardware I/O but subsequently fails
internally, an incoming report could trigger uclogic_raw_event_pen() and
arm the timer before the error path is taken:

drivers/hid/hid-uclogic-core.c:uclogic_probe()
	rc = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
	if (rc) {
		hid_err(hdev, "hw start failed\n");
		goto failure;
	}

	return 0;
failure:
	/* Assume "remove" might not be called if "probe" failed */
	if (params_initialized)
		uclogic_params_cleanup(&drvdata->params);
	return rc;

Because this cleanup block does not cancel the inrange_timer, the devm
framework will free the drvdata structure upon probe return.

This leaves the timer armed to execute on freed memory.

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260711073003.71012-1-doruk@0sec.ai?part=1

[PATCH v2] HID: uclogic: fix UAF on inrange_timer at teardown and probe error

[Doruk Tan Ozturk]

uclogic_probe() arms a per-device timer whose callback
uclogic_inrange_timeout() dereferences drvdata->pen_input, and
uclogic_raw_event_pen() re-arms it with a 100 ms timeout on every
in-range pen report.

uclogic_remove() drained the timer with timer_delete_sync() before
hid_hw_stop().  timer_delete_sync() does not block re-arming: a pen
report delivered before hid_hw_stop() kills the URBs can re-arm the timer
after it was drained.  hid_hw_stop() then frees the hidinput pen_input
(via hidinput_disconnect() -> input_unregister_device()), and the pending
timer fires on freed memory.

Use timer_shutdown_sync() instead, still before hid_hw_stop().  It drains
the callback while pen_input is still valid and permanently blocks
re-arming, so an in-flight raw_event cannot revive the timer; hid_hw_stop()
then frees pen_input with the timer already dead.

The probe error path had the same exposure: if hid_hw_start() started I/O
and then failed, raw_event may have armed the timer, which would fire on
the devm-freed drvdata after probe returns.  Shut the timer down there too.

Unlike letsketch, whose input devices are devm-allocated and outlive
hid_hw_stop(), uclogic's pen_input is freed inside hid_hw_stop(), so the
timer must be shut down before it rather than after.

Found by 0sec (https://0sec.ai) using automated source analysis; not
runtime-reproduced.

Fixes: 01309e29eb95 ("HID: uclogic: Support in-range reporting emulation")
Cc: stable@vger.kernel.org
Assisted-by: 0sec:claude-opus-4-8
Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
---
v2:
 - Shut the timer down *before* hid_hw_stop() rather than after.  v1
   mirrored the letsketch ordering (hid_hw_stop() first), but uclogic's
   pen_input is the hidinput device freed inside hid_hw_stop(), not a
   devm device that outlives it as in letsketch.  A timer armed just
   before the URBs are killed could still fire on the freed pen_input in
   the window before timer_shutdown_sync() drained it.  Running
   timer_shutdown_sync() before hid_hw_stop() drains the callback while
   pen_input is still valid and blocks re-arming, closing that window.
 - Also shut the timer down on the probe error path.

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

diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
index b73f09d26688..d74f98efa879 100644
--- a/drivers/hid/hid-uclogic-core.c
+++ b/drivers/hid/hid-uclogic-core.c
@@ -267,6 +267,13 @@ static int uclogic_probe(struct hid_device *hdev,
 	/* Assume "remove" might not be called if "probe" failed */
 	if (params_initialized)
 		uclogic_params_cleanup(&drvdata->params);
+	/*
+	 * If hid_hw_start() started I/O and then failed, raw_event may have
+	 * armed the timer; shut it down so it cannot fire on the devm-freed
+	 * drvdata after probe returns.
+	 */
+	if (drvdata)
+		timer_shutdown_sync(&drvdata->inrange_timer);
 	return rc;
 }
 
@@ -548,7 +555,15 @@ static void uclogic_remove(struct hid_device *hdev)
 {
 	struct uclogic_drvdata *drvdata = hid_get_drvdata(hdev);
 
-	timer_delete_sync(&drvdata->inrange_timer);
+	/*
+	 * timer_delete_sync() does not prevent re-arming, so a pen report
+	 * delivered before hid_hw_stop() kills the URBs could re-arm the
+	 * timer; hid_hw_stop() then frees the hidinput pen_input and the
+	 * pending timer fires on freed memory.  timer_shutdown_sync() drains
+	 * the callback while pen_input is still valid and permanently blocks
+	 * re-arming, so an in-flight raw_event cannot revive it.
+	 */
+	timer_shutdown_sync(&drvdata->inrange_timer);
 	hid_hw_stop(hdev);
 	kfree(drvdata->desc_ptr);
 	uclogic_params_cleanup(&drvdata->params);
-- 
2.43.0

Re: [PATCH v2] HID: uclogic: fix UAF on inrange_timer at teardown and probe error

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing kfree(drvdata->desc_ptr) in the uclogic_probe error path leads to a memory leak of the dynamically allocated replacement report descriptor.
--

--- Patch [1]: [PATCH v2] HID: uclogic: fix UAF on inrange_timer at teardown and probe error ---
commit 765009419dfdd98b6d556b9c30dc95cac5ee9252
Author: Doruk Tan Ozturk <doruk@0sec.ai>

HID: uclogic: fix UAF on inrange_timer at teardown and probe error

This commit replaces timer_delete_sync() with timer_shutdown_sync() to
prevent use-after-free bugs caused by the timer being re-armed during device
teardown. It also ensures the timer is properly shut down in the probe error
path.

> diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
> --- a/drivers/hid/hid-uclogic-core.c
> +++ b/drivers/hid/hid-uclogic-core.c
> @@ -267,6 +267,13 @@ static int uclogic_probe(struct hid_device *hdev,
>  	/* Assume "remove" might not be called if "probe" failed */
>  	if (params_initialized)
>  		uclogic_params_cleanup(&drvdata->params);
> +	/*
> +	 * If hid_hw_start() started I/O and then failed, raw_event may have
> +	 * armed the timer; shut it down so it cannot fire on the devm-freed
> +	 * drvdata after probe returns.
> +	 */
> +	if (drvdata)
> +		timer_shutdown_sync(&drvdata->inrange_timer);

[Severity: High]
This isn't a bug introduced by this patch, but does this error path leak
drvdata->desc_ptr?

If uclogic_probe() successfully allocates drvdata->desc_ptr via
uclogic_params_get_desc(), but a subsequent initialization step like
hid_parse() or hid_hw_start() fails, it jumps to the failure label. The
error path cleans up params and shuts down the timer, but it appears to miss
calling kfree(drvdata->desc_ptr) before returning.

>  	return rc;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260711080350.81108-1-doruk@0sec.ai?part=1