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

[sashiko-bot@kernel.org]

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