Re: [PATCH v3 4/4] HID: roccat: use kref to manage device instances

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] hid: roccat: UaF in roccat_device_available() due to unlocked device->dev access
--

commit 8a69ab520bd366b55b30a21617d1af38b7f66c41
Author: Dmitry Antipov <dmantipov@yandex.ru>

HID: roccat: use kref to manage device instances

This commit introduces a kref-based lifecycle for roccat_device instances
to resolve memory leaks and use-after-free issues between open/release
and connect/disconnect pairs. The device slot is now released in
roccat_free_device() instead of roccat_disconnect().

Link: https://sashiko.dev/#/patchset/20260902094551.200587-1-dmantipov@yandex.ru?part=2

> diff --git a/drivers/hid/hid-roccat.c b/drivers/hid/hid-roccat.c
> index 1e1a76c453bd2..bdf8857509294 100644
> --- a/drivers/hid/hid-roccat.c
> +++ b/drivers/hid/hid-roccat.c
[ ... ]
> @@ -366,18 +376,20 @@ void roccat_disconnect(int minor)
>  
>  	mutex_lock(&devices_lock);
>  	device = devices[minor];
> +	if (WARN_ON(!device))
> +		goto out;
>  
> -	device_destroy(device->dev->class, MKDEV(roccat_major, minor));
> -	device->dev = NULL;
> -	devices[minor] = NULL;
> +	if (!WARN_ON(!roccat_device_available(device))) {
> +		device_destroy(device->dev->class, MKDEV(roccat_major, minor));
> +		device->dev = NULL;
> +	}

[Severity: High]
Can an unlocked reader racing with this code trigger a use-after-free?

If a userspace process calls roccat_read() or roccat_poll() on the device
node concurrently with roccat_disconnect(), they will invoke
roccat_device_available().

Since those read and poll paths are unlocked, a thread could see
device->dev as non-NULL in roccat_device_available():

drivers/hid/hid-roccat.c:roccat_device_available() {
    return device->dev ? device_is_registered(device->dev) : false;
}

If it is preempted right after the ternary check but before calling
device_is_registered(), and this roccat_disconnect() code then runs,
destroying device->dev and setting it to NULL, the reader thread will
resume and dereference the freed pointer.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916080013.56388-1-dmantipov@yandex.ru?part=4