Re: [PATCH] HID: hiddev: Use kref to track struct hiddev lifetime

["Heitor Alves de Siqueira" <halves@igalia.com>]

Hi Dmitry,

On Mon Jun 8, 2026 at 3:44 PM -03, Dmitry Torokhov wrote:
> Hi Heitor,
>
> On Mon, Jun 08, 2026 at 01:33:03PM -0300, Heitor Alves de Siqueira wrote:
>> If a USB HID device is disconnected while userspace still holds the
>> hiddev node open, hiddev_disconnect() and hiddev_release() can race on
>> the embedded existancelock mutex. Syzbot has triggered this with kfree()
>> happening during the mutex slow path.
>> 
>> Fix by introducing a kref in struct hiddev, and moving kfree() into a
>> dedicated release callback. This way, struct hiddev will only be freed
>> after both hiddev_release() and hiddev_disconnect() are done.
>
> This looks like a common issue with usb_register_dev() that does not
> allow tying the lifetime of the created device, lifetime of user of the
> created device, and userspace accessing it. Ideally the class device
> would be embedded into struct hiddev, and tie its lifetime with lifetime
> of the chardev associated with it and userspace accessors using it. tie
> its lifetime with lifetime of the chardev associated with it and
> userspace accessors using it. See cdev_device_add() and how it is being
> used by multiple subsystems and how they handle class devices. 

Thank you for your review! I see now, usb_register_dev() does not seem
to be the best if we want to tie the lifetimes together... At least, I
don't think its intended for this usecase, especially if we want to
embed the cdev into struct hiddev. I've been looking at some of the
other drivers that use cdev_device_add(), and noticed that we'd have to
manually handle some things that usb_register_dev() conveniently does
for us (although most seem quite straightforward like minor allocation
and device creation).

I'll give it a try for hiddev, and make sure we don't change any of the
class device paths or leave any open ends with the rest of the usbhid
subsystem. One of the USB gadget drivers already uses cdev_device_add(),
so hopefully it won't be too much trouble.

Thanks,
Heitor