Re: [PATCH] HID: input: allocate input_dev name/phys/uniq using hid device devres

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Use-After-Free of input_dev string fields (name, phys, uniq) on device unbind.
- [Low] Memory leak of devm_kstrdup strings in the error path.
--

--- Patch [1]: [PATCH] HID: input: allocate input_dev name/phys/uniq using hid device devres ---
commit ae1934cecc0a7213604df29b6d6486b4090d9b54
Author: Hyeonsu Choi <chlgustn3171@gmail.com>

HID: input: allocate input_dev name/phys/uniq using hid device devres

This commit modifies hidinput_allocate() to dynamically allocate the
input_dev name, phys, and uniq strings using devm_kstrdup() tied to
the parent HID device. It also introduces an error handling path to
clean up if the allocations fail.

> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -2126,9 +2126,20 @@ static struct hid_input *hidinput_allocate(struct hid_device *hid,
[ ... ]
> -	input_dev->name = hidinput->name ? hidinput->name : hid->name;
> -	input_dev->phys = hid->phys;
> -	input_dev->uniq = hid->uniq;
> +	const char *name = hidinput->name ? hidinput->name : hid->name;
> +
> +	input_dev->name = devm_kstrdup(&hid->dev, name, GFP_KERNEL);
> +	if (!input_dev->name)
> +		goto fail;
> +
> +	input_dev->phys = devm_kstrdup(&hid->dev, hid->phys, GFP_KERNEL);

[Severity: High]
If a userspace process holds an open file descriptor for the evdev node
(/dev/input/eventX), the input_dev will outlive the parent hid->dev.
When the HID device is unbound, devres will free these strings.

Can this cause a use-after-free when userspace later queries the
name, phys, or uniq strings via ioctl (e.g., EVIOCGNAME)?

> +	if (!input_dev->phys)
> +		goto fail;
> +
> +	input_dev->uniq = devm_kstrdup(&hid->dev, hid->uniq, GFP_KERNEL);
> +	if (!input_dev->uniq)
> +		goto fail;
> +
[ ... ]
> @@ -2144,6 +2155,8 @@ static struct hid_input *hidinput_allocate(struct hid_device *hid,
>  	return hidinput;
>  
>  fail:
> +	if (hidinput)
> +                kfree(hidinput->name);
>  	kfree(hidinput);
>  	input_free_device(input_dev);
>  	hid_err(hid, "Out of memory during hid input probe\n");

[Severity: Low]
If devm_kstrdup() fails for phys or uniq, the code jumps here to the
error path. The earlier successful devm_kstrdup() allocations remain
tied to &hid->dev.

If the caller continues the device probe successfully after this failure,
will these strings be leaked for the lifetime of the HID device?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714120052.255085-1-chlgustn3171@gmail.com?part=1