[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

[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

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

[Dmitry Torokhov]

Hi,

On Tue, Jul 14, 2026 at 10:39:09PM +0900, chlgustn3171 wrote:
> 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.

Thank you for the patch. However, I do not believe it fixes anything, as
moving to devres will not extend (and may even shorten) life time of
name, phys and uniq. Normally they are character array members of HID
device, and HID device continues existing past drivers disconnecting
from it. OTOH devres-controlled resources will get freed when driver
unbinds from a device.

Additionally, HID devices are parents of input devices, and should stick
around because input devices hold references to their parents until
device_del() is called in input_unregister_device(). However before that
it will disconnect this input device from all the handlers, including
evdev, evdev will mark the device as !exist and evdev ioctl will not
attempt to process ioctls for such device.

I however am not sure why we see this syzkaller report. Maybe there is
memory corruption somewhere or maybe syzkaller itself gets confused when
tracing numerous allocations.

Thanks.

-- 
Dmitry

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

[chlgustn3171]

Hi Dmitry,

Thanks for the detailed explanation on devres lifecycle and device refcounting.

I agree the patch is unnecessary, and that the syzbot trace is likely
a false positive or external memory corruption. I will drop this
patch.

Thanks again for the review.

Regards, Hyeonsu


On Wed, Jul 15, 2026 at 2:57 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Hi,
>
> On Tue, Jul 14, 2026 at 10:39:09PM +0900, chlgustn3171 wrote:
> > 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.
>
> Thank you for the patch. However, I do not believe it fixes anything, as
> moving to devres will not extend (and may even shorten) life time of
> name, phys and uniq. Normally they are character array members of HID
> device, and HID device continues existing past drivers disconnecting
> from it. OTOH devres-controlled resources will get freed when driver
> unbinds from a device.
>
> Additionally, HID devices are parents of input devices, and should stick
> around because input devices hold references to their parents until
> device_del() is called in input_unregister_device(). However before that
> it will disconnect this input device from all the handlers, including
> evdev, evdev will mark the device as !exist and evdev ioctl will not
> attempt to process ioctls for such device.
>
> I however am not sure why we see this syzkaller report. Maybe there is
> memory corruption somewhere or maybe syzkaller itself gets confused when
> tracing numerous allocations.
>
> Thanks.
>
> --
> Dmitry