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

[sashiko-bot@kernel.org]

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