[PATCH v3 1/2] HID: uclogic: fix UAF on inrange_timer at teardown and probe error

[PATCH v3 1/2] 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).

Fixes: 01309e29eb95 ("HID: uclogic: Support in-range reporting emulation")
Cc: stable@vger.kernel.org
Assisted-by: 0sec
Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
---
v3: resend as a 2-patch series. 1/2 is the v2 timer fix, unchanged. 2/2 adds
    the desc_ptr leak fix in the probe error path, flagged by the Sashiko AI
    review on v2. No functional change to this patch since v2.
v2: shut the timer down on the probe error path too, and clarify in the commit
    message why uclogic differs from the letsketch precedent (pen_input is
    freed inside hid_hw_stop(), so the timer must be shut down before it).

 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

[PATCH v3 2/2] HID: uclogic: fix desc_ptr leak in probe error path

[Doruk Tan Ozturk]

uclogic_probe() calls uclogic_params_get_desc(), which stores a
kmalloc-allocated replacement report descriptor into drvdata->desc_ptr.
If a later init step (hid_parse() or hid_hw_start()) fails, probe jumps
to the "failure" label, which only runs uclogic_params_cleanup() on
drvdata->params and returns. The device core does not call
uclogic_remove() when probe fails, so the kfree(drvdata->desc_ptr) that
uclogic_remove() normally performs never runs, leaking the descriptor.

drvdata itself is devm-allocated and freed automatically, but desc_ptr
is a plain kmalloc/krealloc buffer and must be freed explicitly. Free it
on the probe error path. The kfree is gated on params_initialized, under
which desc_ptr is either NULL (a no-op) or the allocated descriptor.

Found by 0sec (https://0sec.ai).

Fixes: 9614219e9310 ("HID: uclogic: Extract tablet parameter discovery into a module")
Cc: stable@vger.kernel.org
Assisted-by: 0sec
Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
---
 drivers/hid/hid-uclogic-core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
index d74f98efa879..ad9914ed0c71 100644
--- a/drivers/hid/hid-uclogic-core.c
+++ b/drivers/hid/hid-uclogic-core.c
@@ -265,8 +265,10 @@ static int uclogic_probe(struct hid_device *hdev,
 	return 0;
 failure:
 	/* Assume "remove" might not be called if "probe" failed */
-	if (params_initialized)
+	if (params_initialized) {
+		kfree(drvdata->desc_ptr);
 		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
-- 
2.43.0

Re: [PATCH v3 2/2] HID: uclogic: fix desc_ptr leak in probe error path

[sashiko-bot]

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

Pre-existing issues:
- [High] Use-After-Free of `drvdata->pen_input` in `uclogic_inrange_timeout()` when `hidinput_connect()` fails during probe.
--

--- Patch [2]: [PATCH v3 2/2] HID: uclogic: fix desc_ptr leak in probe error path ---
commit b37d55bfcac132529f188bc93436f75e1da9c66f
Author: Doruk Tan Ozturk <doruk@0sec.ai>

HID: uclogic: fix desc_ptr leak in probe error path

This commit fixes a memory leak in the uclogic_probe() error path. It
ensures that drvdata->desc_ptr is explicitly freed when probe fails, since
the normal cleanup in uclogic_remove() is not called in this scenario.

> diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
> index d74f98efa8794..ad9914ed0c717 100644
> --- a/drivers/hid/hid-uclogic-core.c
> +++ b/drivers/hid/hid-uclogic-core.c
> @@ -265,8 +265,10 @@ static int uclogic_probe(struct hid_device *hdev,
>  	return 0;
>  failure:

[Severity: High]
This is a pre-existing issue, but is there a use-after-free risk involving
drvdata->pen_input if hidinput_connect() fails during hid_hw_start()?

During probe, uclogic_input_configured() (drivers/hid/hid-uclogic-core.c)
stores a pointer to the input device before registration completes:

		/* Remember the input device so we can simulate events */
		drvdata->pen_input = hi->input;

If a subsequent input registration fails in hidinput_connect()
(drivers/hid/hid-input.c), the code unwinds and frees all inputs:

out_unwind:
	/* unwind the ones we already registered */
	hidinput_disconnect(hid);

	return -1;

Because the driver sets .raw_event, hid_connect() ignores this -1 return
value. As a result, hid_hw_start() returns 0, the driver binds successfully,
and this failure block is bypassed.

When a pen report later arrives and arms drvdata->inrange_timer, the timer
callback uclogic_inrange_timeout() dereferences the freed pointer:

	struct input_dev *input = drvdata->pen_input;

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

Could this sequence lead to a dangling pointer being dereferenced?

>  	/* Assume "remove" might not be called if "probe" failed */
> -	if (params_initialized)
> +	if (params_initialized) {
> +		kfree(drvdata->desc_ptr);
>  		uclogic_params_cleanup(&drvdata->params);
> +	}
>  	/*
>  	 * If hid_hw_start() started I/O and then failed, raw_event may have
[ ... ]

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

Re: [PATCH v3 1/2] HID: uclogic: fix UAF on inrange_timer at teardown and probe error

[sashiko-bot]

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

Pre-existing issues:
- [High] Use-After-Free on `drvdata->pen_input` during input registration failure.
- [Medium] Memory leak of `drvdata->desc_ptr` on probe error path.
--

--- Patch [1]: [PATCH v3 1/2] HID: uclogic: fix UAF on inrange_timer at teardown and probe error ---
commit 2397dc01e99d0cc8ef59607ebbc881c2a6cb3354
Author: Doruk Tan Ozturk <doruk@0sec.ai>

HID: uclogic: fix UAF on inrange_timer at teardown and probe error
    
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.

[ ... ]

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

[Severity: Medium]
This is a pre-existing issue, but does this error path leak the dynamically
allocated replacement report descriptor buffer?

In uclogic_probe(), uclogic_params_get_desc() allocates a concatenated report
descriptor and assigns it to drvdata->desc_ptr. If subsequent operations such
as hid_parse() or hid_hw_start() fail, the code jumps to the failure path.

The cleanup logic calls uclogic_params_cleanup(&drvdata->params), which frees
params->desc_ptr, but it does not free drvdata->desc_ptr.

(Note: The commit message claims a 2/2 patch fixes this, but that patch is
absent from the provided repository state at the end of the series).

> +	/*
> +	 * 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 is a pre-existing issue, but can this lead to a Use-After-Free on
drvdata->pen_input if input registration fails?

During uclogic_probe(), hid_hw_start() calls hid_connect() which attempts to
register input devices via hidinput_connect(). As part of this process,
uclogic_input_configured() assigns the allocated input_dev to
drvdata->pen_input:

drivers/hid/hid-uclogic-core.c:uclogic_input_configured() {
    ...
    if (hi->report->id == params->pen.id) {
        /* Remember the input device so we can simulate events */
        drvdata->pen_input = hi->input;
    }
    ...
}

If input_register_device() fails, the code unwinds and frees the input_dev
structure via hidinput_disconnect(), but drvdata->pen_input is never cleared
and remains dangling.

Because the uclogic_driver defines a raw_event callback, hid_connect() still
returns success (0) even if no listeners successfully claimed the device.
Consequently, hid_hw_start() and the driver probe succeed.

If the USB device subsequently sends an in-range pen report,
uclogic_raw_event_pen() rearms inrange_timer. When the timer expires,
uclogic_inrange_timeout() unconditionally dereferences the dangling
drvdata->pen_input 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);
    ...
}

>  	return rc;
>  }

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