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

[sashiko-bot@kernel.org]

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