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

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

[Hyeonsu Choi]

input_dev->name, phys, and uniq are directly assigned from hid_device.
It causes a slab-use-after-free bug if the input device outlives the parent HID device during teardown.
Fix this by using devm_kstrdup() tied to &hid->dev for name, phys, and uniq strings.
The string lifetime is tied to the parent HID device, preventing a potential slab-use-after-free bug.
Also add OOM error handling for these allocations.

Reported-by: syzbot+aec1962419768c5caf88@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=aec1962419768c5caf88
Signed-off-by: Hyeonsu Choi <chlgustn3171@gmail.com>
---
 drivers/hid/hid-input.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 3487600cadb4..da6b338f18ba 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -2125,10 +2125,21 @@ static struct hid_input *hidinput_allocate(struct hid_device *hid,
 	input_dev->close = hidinput_close;
 	input_dev->setkeycode = hidinput_setkeycode;
 	input_dev->getkeycode = hidinput_getkeycode;
+        
+	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);
+	if (!input_dev->phys)
+		goto fail;
+
+	input_dev->uniq = devm_kstrdup(&hid->dev, hid->uniq, GFP_KERNEL);
+	if (!input_dev->uniq)
+		goto fail;
 
-	input_dev->name = hidinput->name ? hidinput->name : hid->name;
-	input_dev->phys = hid->phys;
-	input_dev->uniq = hid->uniq;
 	input_dev->id.bustype = hid->bus;
 	input_dev->id.vendor  = hid->vendor;
 	input_dev->id.product = hid->product;
@@ -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");
-- 
2.53.0

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

[sashiko-bot]

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

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

[chlgustn3171]

On Tue, Jul 14, 2026 at 9:18 PM sashiko-bot wrote:
> [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)?

Thanks for the review.

Using &hid->dev here is intentional.
Using the input device for devm-managed input_dev strings is
problematic; see CVE-2023-53454, where the allocation was moved to the
HID device because the input device unregister path may still use the
name for uevents.

> [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?

The name/phys/uniq copies are attached to &hid->dev, so the clean-up
is handled by HID device devres.
However, if it is better to explicitly unwind, I can add that in v2.

Thanks,
Hyeonsu

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

[chlgustn3171]

Adding maintainers and mailing list to CC. Sorry for the omission.

On Tue, Jul 14, 2026 at 10:33 PM chlgustn3171 <chlgustn3171@gmail.com> wrote:
>
> On Tue, Jul 14, 2026 at 9:18 PM sashiko-bot wrote:
> > [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)?
>
> Thanks for the review.
>
> Using &hid->dev here is intentional.
> Using the input device for devm-managed input_dev strings is
> problematic; see CVE-2023-53454, where the allocation was moved to the
> HID device because the input device unregister path may still use the
> name for uevents.
>
> > [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?
>
> The name/phys/uniq copies are attached to &hid->dev, so the clean-up
> is handled by HID device devres.
> However, if it is better to explicitly unwind, I can add that in v2.
>
> Thanks,
> Hyeonsu