[PATCH v2 1/4] HID: roccat: fix locking in roccat_connect() and roccat_disconnect()

[PATCH v2 1/4] HID: roccat: fix locking in roccat_connect() and roccat_disconnect()

[Dmitry Antipov]

Extend critical section in roccat_connect() to ensure that partially
initialized 'struct roccat_device' is never exposed in 'devices' list,
and do the same in roccat_disconnect() to avoid racy 'devices' access
against roccat_release().

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v2: unchanged since v1
---
 drivers/hid/hid-roccat.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/hid-roccat.c b/drivers/hid/hid-roccat.c
index 4f15eb951039..5deb6da8d4f7 100644
--- a/drivers/hid/hid-roccat.c
+++ b/drivers/hid/hid-roccat.c
@@ -344,8 +344,6 @@ int roccat_connect(const struct class *klass, struct hid_device *hid, int report
 		return temp;
 	}
 
-	mutex_unlock(&devices_lock);
-
 	init_waitqueue_head(&device->wait);
 	INIT_LIST_HEAD(&device->readers);
 	mutex_init(&device->readers_lock);
@@ -356,6 +354,7 @@ int roccat_connect(const struct class *klass, struct hid_device *hid, int report
 	device->cbuf_end = 0;
 	device->report_size = report_size;
 
+	mutex_unlock(&devices_lock);
 	return minor;
 }
 EXPORT_SYMBOL_GPL(roccat_connect);
@@ -369,15 +368,12 @@ void roccat_disconnect(int minor)
 
 	mutex_lock(&devices_lock);
 	device = devices[minor];
-	mutex_unlock(&devices_lock);
 
 	device->exist = 0; /* TODO exist maybe not needed */
 
 	device_destroy(device->dev->class, MKDEV(roccat_major, minor));
 
-	mutex_lock(&devices_lock);
 	devices[minor] = NULL;
-	mutex_unlock(&devices_lock);
 
 	if (device->open) {
 		hid_hw_close(device->hid);
@@ -385,6 +381,8 @@ void roccat_disconnect(int minor)
 	} else {
 		roccat_free_device(device);
 	}
+
+	mutex_unlock(&devices_lock);
 }
 EXPORT_SYMBOL_GPL(roccat_disconnect);
 
-- 
2.55.0

[PATCH v2 2/4] HID: roccat: fix device access in roccat_release()

[Dmitry Antipov]

In roccat_release(), access the device using file-specific
reader data rather than global array, thus preventing the
case when original device was disconnected and a new device
has connected using the same slot indexed by minor number.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20260902094551.200587-1-dmantipov@yandex.ru?part=2
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v2: initial version to join the series
---
 drivers/hid/hid-roccat.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/hid/hid-roccat.c b/drivers/hid/hid-roccat.c
index 5deb6da8d4f7..007778922c21 100644
--- a/drivers/hid/hid-roccat.c
+++ b/drivers/hid/hid-roccat.c
@@ -211,24 +211,19 @@ static int roccat_open(struct inode *inode, struct file *file)
 
 static int roccat_release(struct inode *inode, struct file *file)
 {
-	unsigned int minor = iminor(inode);
 	struct roccat_reader *reader = file->private_data;
-	struct roccat_device *device;
-
-	mutex_lock(&devices_lock);
+	struct roccat_device *device = reader->device;
 
-	device = devices[minor];
-	if (!device) {
-		mutex_unlock(&devices_lock);
-		pr_emerg("roccat device with minor %d doesn't exist\n", minor);
+	if (WARN_ON_ONCE(!device))
 		return -ENODEV;
-	}
 
 	mutex_lock(&device->readers_lock);
 	list_del(&reader->node);
 	mutex_unlock(&device->readers_lock);
 	kfree(reader);
 
+	mutex_lock(&devices_lock);
+
 	if (!--device->open) {
 		/* removing last reader */
 		if (device->exist) {
-- 
2.55.0

[PATCH v2 3/4] HID: roccat: examine readers to check whether the device is opened

[Dmitry Antipov]

Use list_empty() to check whether the device has active readers,
thus dropping explicit 'open' flag from 'struct roccat_device'.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v2: initial version to join the series
---
 drivers/hid/hid-roccat.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/hid/hid-roccat.c b/drivers/hid/hid-roccat.c
index 007778922c21..d1df4ec34a1e 100644
--- a/drivers/hid/hid-roccat.c
+++ b/drivers/hid/hid-roccat.c
@@ -39,7 +39,6 @@ struct roccat_report {
 struct roccat_device {
 	unsigned int minor;
 	int report_size;
-	int open;
 	int exist;
 	wait_queue_head_t wait;
 	struct device *dev;
@@ -177,18 +176,15 @@ static int roccat_open(struct inode *inode, struct file *file)
 
 	mutex_lock(&device->readers_lock);
 
-	if (!device->open++) {
+	if (list_empty(&device->readers)) {
 		/* power on device on adding first reader */
 		error = hid_hw_power(device->hid, PM_HINT_FULLON);
-		if (error < 0) {
-			--device->open;
+		if (error < 0)
 			goto exit_err_readers;
-		}
 
 		error = hid_hw_open(device->hid);
 		if (error < 0) {
 			hid_hw_power(device->hid, PM_HINT_NORMAL);
-			--device->open;
 			goto exit_err_readers;
 		}
 	}
@@ -213,18 +209,20 @@ static int roccat_release(struct inode *inode, struct file *file)
 {
 	struct roccat_reader *reader = file->private_data;
 	struct roccat_device *device = reader->device;
+	bool open;
 
 	if (WARN_ON_ONCE(!device))
 		return -ENODEV;
 
 	mutex_lock(&device->readers_lock);
 	list_del(&reader->node);
+	open = !list_empty(&device->readers);
 	mutex_unlock(&device->readers_lock);
 	kfree(reader);
 
 	mutex_lock(&devices_lock);
 
-	if (!--device->open) {
+	if (!open) {
 		/* removing last reader */
 		if (device->exist) {
 			hid_hw_power(device->hid, PM_HINT_NORMAL);
@@ -360,6 +358,7 @@ EXPORT_SYMBOL_GPL(roccat_connect);
 void roccat_disconnect(int minor)
 {
 	struct roccat_device *device;
+	bool open;
 
 	mutex_lock(&devices_lock);
 	device = devices[minor];
@@ -370,7 +369,11 @@ void roccat_disconnect(int minor)
 
 	devices[minor] = NULL;
 
-	if (device->open) {
+	mutex_lock(&device->readers_lock);
+	open = !list_empty(&device->readers);
+	mutex_unlock(&device->readers_lock);
+
+	if (open) {
 		hid_hw_close(device->hid);
 		wake_up_interruptible(&device->wait);
 	} else {
-- 
2.55.0

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

[Dmitry Antipov]

Use kref to manage 'struct roccat_device' instances and fix
UaF-triggering race between roccat_open()/roccat_release()
and roccat_connect()/roccat_disconnect() pairs.

Reported-by: syzbot+d632e93ffcd1452bc61e@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=d632e93ffcd1452bc61e
Link: https://sashiko.dev/#/patchset/20260902094551.200587-1-dmantipov@yandex.ru?part=2
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v2: unconditionally get/put device reference during
    first open and last close, respectively (Sashiko)
---
 drivers/hid/hid-roccat.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/hid-roccat.c b/drivers/hid/hid-roccat.c
index d1df4ec34a1e..32927b20bcfe 100644
--- a/drivers/hid/hid-roccat.c
+++ b/drivers/hid/hid-roccat.c
@@ -40,6 +40,7 @@ struct roccat_device {
 	unsigned int minor;
 	int report_size;
 	int exist;
+	struct kref ref;
 	wait_queue_head_t wait;
 	struct device *dev;
 	struct hid_device *hid;
@@ -69,10 +70,12 @@ static struct roccat_device *devices[ROCCAT_MAX_DEVICES];
 /* protects modifications of devices array */
 static DEFINE_MUTEX(devices_lock);
 
-static void roccat_free_device(struct roccat_device *device)
+static void roccat_free_device(struct kref *ref)
 {
+	struct roccat_device *device;
 	int i;
 
+	device = container_of(ref, struct roccat_device, ref);
 	for (i = 0; i < ROCCAT_CBUF_SIZE; i++)
 		kfree(device->cbuf[i].value);
 	kfree(device);
@@ -187,6 +190,7 @@ static int roccat_open(struct inode *inode, struct file *file)
 			hid_hw_power(device->hid, PM_HINT_NORMAL);
 			goto exit_err_readers;
 		}
+		kref_get(&device->ref);
 	}
 
 	reader->device = device;
@@ -227,9 +231,8 @@ static int roccat_release(struct inode *inode, struct file *file)
 		if (device->exist) {
 			hid_hw_power(device->hid, PM_HINT_NORMAL);
 			hid_hw_close(device->hid);
-		} else {
-			roccat_free_device(device);
 		}
+		kref_put(&device->ref, roccat_free_device);
 	}
 
 	mutex_unlock(&devices_lock);
@@ -341,6 +344,7 @@ int roccat_connect(const struct class *klass, struct hid_device *hid, int report
 	INIT_LIST_HEAD(&device->readers);
 	mutex_init(&device->readers_lock);
 	mutex_init(&device->cbuf_lock);
+	kref_init(&device->ref);
 	device->minor = minor;
 	device->hid = hid;
 	device->exist = 1;
@@ -362,6 +366,8 @@ void roccat_disconnect(int minor)
 
 	mutex_lock(&devices_lock);
 	device = devices[minor];
+	if (!device)
+		goto out;
 
 	device->exist = 0; /* TODO exist maybe not needed */
 
@@ -376,10 +382,10 @@ void roccat_disconnect(int minor)
 	if (open) {
 		hid_hw_close(device->hid);
 		wake_up_interruptible(&device->wait);
-	} else {
-		roccat_free_device(device);
 	}
 
+	kref_put(&device->ref, roccat_free_device);
+out:
 	mutex_unlock(&devices_lock);
 }
 EXPORT_SYMBOL_GPL(roccat_disconnect);
-- 
2.55.0

Re: [PATCH v2 1/4] HID: roccat: fix locking in roccat_connect() and roccat_disconnect()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] `roccat_report_event()` calls `mutex_lock()` while executing in atomic context.
--

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

HID: roccat: fix locking in roccat_connect() and roccat_disconnect()

This commit extends the critical section in roccat_connect() to avoid
exposing a partially initialized device in the devices list. It also
updates roccat_disconnect() to prevent racy access against
roccat_release().

> diff --git a/drivers/hid/hid-roccat.c b/drivers/hid/hid-roccat.c
> index 4f15eb9510396..5deb6da8d4f74 100644
> --- a/drivers/hid/hid-roccat.c
> +++ b/drivers/hid/hid-roccat.c

[ ... ]

[Severity: High]
This is a pre-existing issue, but does roccat_report_event() call
mutex_lock() while executing in an atomic context?

The roccat_report_event() function is invoked by the driver's raw_event
handler, which is called synchronously by the HID core from the USB URB
completion handler. Since URB completion handlers execute in atomic context,
acquiring sleeping locks here triggers a 'scheduling while atomic' bug.

drivers/hid/hid-roccat.c:roccat_report_event() {
    ...
	mutex_lock(&device->readers_lock);
	mutex_lock(&device->cbuf_lock);
    ...
}

This appears to be reachable whenever a supported Roccat device sends
specific hardware events, such as when a user presses a profile switch
button on the mouse. Should this code use a spinlock or defer the work
instead?

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

Re: [PATCH v2 2/4] HID: roccat: fix device access in roccat_release()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [Critical] HID roccat sub-drivers free their private driver data before calling hid_hw_stop(), causing a Use-After-Free in the raw_event callback.
- [High] roccat_report_event() accesses devices[minor] locklessly and does not check for NULL, leading to a NULL pointer dereference during device disconnect.
- [High] roccat_report_event() acquires sleepable mutexes from an atomic interrupt context.
--

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

HID: roccat: fix device access in roccat_release()

In roccat_release(), access the device using file-specific reader data rather
than global array, thus preventing the case when original device was
disconnected and a new device has connected using the same slot indexed by minor
number.

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 5deb6da8d4f74..007778922c21a 100644
> --- a/drivers/hid/hid-roccat.c
> +++ b/drivers/hid/hid-roccat.c

[ ... ]

> int roccat_report_event(int minor, u8 const *data)
> {
> 	struct roccat_device *device;
> 	struct roccat_reader *reader;
> 	struct roccat_report *report;
> 	uint8_t *new_value;
> 
> 	device = devices[minor];
> 
> 	new_value = kmemdup(data, device->report_size, GFP_ATOMIC);

[Severity: High]
This isn't a bug introduced by this patch, but does roccat_report_event()
access devices[minor] locklessly and without checking for NULL?

If a device is disconnected, roccat_disconnect() sets devices[minor] = NULL
and drops the device reference. Because sub-drivers like hid-roccat-kone fail
to call hid_hw_stop() before roccat_disconnect(), incoming USB interrupts can
still trigger roccat_report_event() concurrently.

Could this lead to a NULL pointer dereference when kmemdup() accesses
device->report_size?

> 	if (!new_value)
> 		return -ENOMEM;
> 
> 	mutex_lock(&device->readers_lock);
> 	mutex_lock(&device->cbuf_lock);

[Severity: High]
This is a pre-existing issue, but does this code acquire sleepable mutexes from
an atomic interrupt context?

roccat_report_event() is called by raw_event callbacks in HID sub-drivers,
which execute in atomic interrupt context (URB completion handlers like
usbhid_irq_in).

Can calling mutex_lock() here cause a "scheduling while atomic" panic, or
a deadlock if the lock is contended?

> drivers/hid/hid-roccat-kone.c:
> static void kone_remove_specials(struct hid_device *hdev)
> {
> 	struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
> 	struct kone_device *kone;
> 
> 	if (intf->cur_altsetting->desc.bInterfaceProtocol
> 			== USB_INTERFACE_PROTOCOL_MOUSE) {
> 		kone = hid_get_drvdata(hdev);
> 		if (kone->roccat_claimed)
> 			roccat_disconnect(kone->chrdev_minor);
> 		kfree(hid_get_drvdata(hdev));
> 	}
> }
> 
> static void kone_remove(struct hid_device *hdev)
> {
> 	kone_remove_specials(hdev);
> 	hid_hw_stop(hdev);
> }

[Severity: Critical]
This isn't a bug introduced by this patch, but do HID roccat sub-drivers free
their private driver data before calling hid_hw_stop()?

During device unbinding/removal, kone_remove_specials() is called which frees
the driver data with kfree(hid_get_drvdata(hdev)). However, hid_hw_stop() is
called after this free.

Since hid_hw_stop() is responsible for stopping hardware interrupts, an incoming
USB report can trigger kone_raw_event() concurrently:

drivers/hid/hid-roccat-kone.c:kone_raw_event() {
	struct kone_device *kone = hid_get_drvdata(hdev);
	...
	if (kone == NULL)
		return 0;
	...
	if (kone->roccat_claimed)
		kone_report_to_chrdev(kone, event);
}

Can this result in a Use-After-Free memory corruption when the raw_event
callback dereferences the freed driver data?

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

Re: [PATCH v2 3/4] HID: roccat: examine readers to check whether the device is opened

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Caching `list_empty()` state outside `devices_lock` in `roccat_release()` creates a TOCTOU race with `roccat_open()` and `roccat_disconnect()`, leading to device state corruption and leaked hardware open counts.
--

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

HID: roccat: examine readers to check whether the device is opened

This patch replaces the explicit open counter in struct roccat_device with
a check against list_empty(&device->readers). The goal is to determine if
active readers exist without maintaining a separate integer count.

> diff --git a/drivers/hid/hid-roccat.c b/drivers/hid/hid-roccat.c
> index 007778922c21a..d1df4ec34a1e6 100644
> --- a/drivers/hid/hid-roccat.c
> +++ b/drivers/hid/hid-roccat.c
[ ... ]
> @@ -213,18 +209,20 @@ static int roccat_release(struct inode *inode, struct file *file)
>  {
>  	struct roccat_reader *reader = file->private_data;
>  	struct roccat_device *device = reader->device;
> +	bool open;
>  
>  	if (WARN_ON_ONCE(!device))
>  		return -ENODEV;
>  
>  	mutex_lock(&device->readers_lock);
>  	list_del(&reader->node);
> +	open = !list_empty(&device->readers);
>  	mutex_unlock(&device->readers_lock);

[Severity: High]
Does evaluating the list state here and dropping the lock introduce a race
condition?

If another thread calls roccat_open() before devices_lock is acquired below,
it will observe an empty list, power on the device, and add a new reader.

>  	kfree(reader);
>  
>  	mutex_lock(&devices_lock);
>  
> -	if (!--device->open) {
> +	if (!open) {
>  		/* removing last reader */
>  		if (device->exist) {
>  			hid_hw_power(device->hid, PM_HINT_NORMAL);
>  			hid_hw_close(device->hid);
>  		}

[Severity: High]
Since this uses the cached open flag, could it unconditionally power down and
close the hardware here, cutting off the newly added reader from the
concurrent roccat_open() call?

[ ... ]
> @@ -370,7 +369,11 @@ void roccat_disconnect(int minor)
>  
>  	devices[minor] = NULL;
>  
> -	if (device->open) {
> +	mutex_lock(&device->readers_lock);
> +	open = !list_empty(&device->readers);
> +	mutex_unlock(&device->readers_lock);
> +
> +	if (open) {
>  		hid_hw_close(device->hid);
>  		wake_up_interruptible(&device->wait);
>  	} else {

[Severity: High]
Can a concurrent roccat_disconnect() and roccat_release() race here,
resulting in neither function closing the hardware?

If roccat_release() removes the last reader, it evaluates the local open
variable to false. If roccat_disconnect() then acquires readers_lock, it will
also evaluate open to false since the list is already empty, and it will skip
calling hid_hw_close().

When roccat_release() subsequently acquires devices_lock, it observes that
device->exist is 0 (set earlier by roccat_disconnect()) and also skips calling
hid_hw_close().

Will this result in a permanent leak of the HID hardware open count?

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [High] A TOCTOU race condition between `roccat_release` and `roccat_open` causes a new reader's device to be incorrectly powered down and closed.

Pre-existing issues:
- [High] Data path UAF and NULL pointer dereference in `roccat_report_event` due to lockless device access.
- [High] Custom Roccat drivers violate HID subsystem lifecycle rules by freeing their private `drvdata` before stopping the hardware, causing a UAF in their `raw_event` handlers.
- [High] A race condition between `roccat_release` and `roccat_disconnect` causes `hid_hw_close()` and `hid_hw_power()` to be skipped entirely, permanently leaking `ll_open_count` and power references.
--

commit 0e2654cfadc18d56d9b77ac333344436bcca55c7
Author: Dmitry Antipov <dmantipov@yandex.ru>

HID: roccat: use kref to manage device instances

Use kref to manage 'struct roccat_device' instances and fix
UaF-triggering race between roccat_open()/roccat_release()
and roccat_connect()/roccat_disconnect() pairs.

Reported-by: syzbot+d632e93ffcd1452bc61e@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=d632e93ffcd1452bc61e
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 d1df4ec34a1e6..32927b20bcfe0 100644
> --- a/drivers/hid/hid-roccat.c
> +++ b/drivers/hid/hid-roccat.c
[ ... ]
> @@ -227,9 +231,8 @@ static int roccat_release(struct inode *inode, struct file *file)
>  		if (device->exist) {
>  			hid_hw_power(device->hid, PM_HINT_NORMAL);
>  			hid_hw_close(device->hid);
> -		} else {
> -			roccat_free_device(device);
>  		}
> +		kref_put(&device->ref, roccat_free_device);
>  	}
>  
>  	mutex_unlock(&devices_lock);

[Severity: High]
Can a newly opened reader's device be incorrectly closed and powered down due
to a TOCTOU race?

Looking at roccat_release(), the open status is determined early while holding
device->readers_lock. A concurrent roccat_open() can then observe the empty
list, power on the device, and add its reader node.

When roccat_release() later acquires devices_lock, it acts on the stale open
variable (which is false):

CPU1: roccat_release()
    mutex_lock(&device->readers_lock);
    open = !list_empty(&device->readers); // false
    mutex_unlock(&device->readers_lock);

CPU2: roccat_open()
    mutex_lock(&devices_lock);
    mutex_lock(&device->readers_lock);
    // list is empty, power on and open hardware
    hid_hw_power(device->hid, PM_HINT_FULLON);
    ...
    mutex_unlock(&device->readers_lock);
    mutex_unlock(&devices_lock);

CPU1: roccat_release() (resuming)
    mutex_lock(&devices_lock);
    if (!open) {
        hid_hw_power(device->hid, PM_HINT_NORMAL);
        hid_hw_close(device->hid);
    }

Does this clobber the concurrent open and leave the newly connected reader with
a powered down, closed hardware state?

> @@ -362,6 +366,8 @@ void roccat_disconnect(int minor)
>  
>  	mutex_lock(&devices_lock);
>  	device = devices[minor];
> +	if (!device)
> +		goto out;

[Severity: High]
This isn't a bug introduced by this patch, but does roccat_report_event() risk
a NULL pointer dereference or use-after-free because it accesses devices[minor]
locklessly?

When the device is unplugged, roccat_disconnect() sets devices[minor] = NULL
and drops the kref. Since the transport hardware hasn't been stopped yet (due
to the delayed hid_hw_close), incoming USB events can concurrently trigger
kone_raw_event(), which calls into roccat_report_event():

drivers/hid/hid-roccat.c:roccat_report_event() {
    ...
    device = devices[minor];

    new_value = kmemdup(data, device->report_size, GFP_ATOMIC);
    ...
}

If devices[minor] is read as NULL here, accessing device->report_size will
trigger a panic. If it is read right before kref_put frees it, does this result
in a use-after-free, defeating the intent of the kref fix?

> @@ -376,10 +382,10 @@ void roccat_disconnect(int minor)
>  	if (open) {
>  		hid_hw_close(device->hid);
>  		wake_up_interruptible(&device->wait);
> -	} else {
> -		roccat_free_device(device);
>  	}
>  
> +	kref_put(&device->ref, roccat_free_device);
> +out:
>  	mutex_unlock(&devices_lock);
>  }

[Severity: High]
This is a pre-existing issue, but can a race between roccat_release() and
roccat_disconnect() permanently leak the ll_open_count and power references by
skipping hid_hw_close() entirely?

If roccat_disconnect() sets device->exist = 0 concurrently with the last reader
calling roccat_release(), both paths appear to bypass the mandatory cleanup:

CPU1: roccat_disconnect()
    device->exist = 0;
    // open is false because reader is removing itself
    if (open) { ... } // skipped

CPU2: roccat_release()
    // open is false
    if (!open) {
        if (device->exist) {
            // skipped because exist is 0
            hid_hw_close(device->hid);
        }
    }

Does this leave the hardware permanently active and prevent proper unbinding?


While reviewing the UAF fixes in the roccat driver, I noticed another
pre-existing UAF in the kone driver's lifecycle handling:

[Severity: High]
This isn't a bug introduced by this patch, but does the kone driver violate
HID lifecycle rules by freeing its private driver data before stopping the
hardware?

Because kfree(hid_get_drvdata(hdev)) is called before hid_hw_stop(hdev),
incoming USB events can still be delivered during this window.

drivers/hid/hid-roccat-kone.c:kone_remove_specials() {
    ...
        kfree(hid_get_drvdata(hdev));
    ...
}

drivers/hid/hid-roccat-kone.c:kone_remove() {
    kone_remove_specials(hdev);
    hid_hw_stop(hdev);
}

If an interrupt arrives before hid_hw_stop(), kone_raw_event() accesses the
freed kone structure:

drivers/hid/hid-roccat-kone.c:kone_raw_event() {
    struct kone_device *kone = hid_get_drvdata(hdev);
    struct kone_mouse_event *event = (struct kone_mouse_event *)data;
    ...
    if (memcmp(&kone->last_mouse_event.tilt, &event->tilt, 5))
    ...
}

Can this use-after-free be triggered locally by unplugging the device while
USB events are active?

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