[PATCH v5 0/3] Latest Roccat HID fixes

[PATCH v5 0/3] Latest Roccat HID fixes

[Dmitry Antipov]

Now d76994443eed ("HID: roccat: fix locking in roccat_connect() and
roccat_disconnect()") is landed, and these ones are expected to fix
https://syzkaller.appspot.com/bug?extid=d632e93ffcd1452bc61e and
https://syzkaller.appspot.com/bug?extid=c492a9e154f81127551f.

Dmitry Antipov (3):
  HID: roccat: use device_is_registered() to check whether device is available
  HID: roccat: fix device access in roccat_release()
  HID: roccat: use kref to manage device instances

 drivers/hid/hid-roccat.c | 63 +++++++++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 26 deletions(-)

-- 
2.55.0

[PATCH v5 1/3] HID: roccat: use device_is_registered() to check whether device is available

[Dmitry Antipov]

Introduce roccat_device_available() to check whether device
is actually available (i.e. not disconnected), thus removing
explicit 'exist' flag from 'struct roccat_device'.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v5: adjusted to match 7.3-rc5
v4: use READ_ONCE() and WRITE_ONCE() for device access
v3: 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 5deb6da8d4f7..96aa508111a1 100644
--- a/drivers/hid/hid-roccat.c
+++ b/drivers/hid/hid-roccat.c
@@ -40,7 +40,6 @@ struct roccat_device {
 	unsigned int minor;
 	int report_size;
 	int open;
-	int exist;
 	wait_queue_head_t wait;
 	struct device *dev;
 	struct hid_device *hid;
@@ -70,6 +69,13 @@ static struct roccat_device *devices[ROCCAT_MAX_DEVICES];
 /* protects modifications of devices array */
 static DEFINE_MUTEX(devices_lock);
 
+static bool roccat_device_available(struct roccat_device *device)
+{
+	struct device *dev = READ_ONCE(device->dev);
+
+	return dev ? device_is_registered(dev) : false;
+}
+
 static void roccat_free_device(struct roccat_device *device)
 {
 	int i;
@@ -105,7 +111,7 @@ static ssize_t roccat_read(struct file *file, char __user *buffer,
 				retval = -ERESTARTSYS;
 				break;
 			}
-			if (!device->exist) {
+			if (!roccat_device_available(device)) {
 				retval = -EIO;
 				break;
 			}
@@ -149,7 +155,7 @@ static __poll_t roccat_poll(struct file *file, poll_table *wait)
 	poll_wait(file, &reader->device->wait, wait);
 	if (reader->cbuf_start != reader->device->cbuf_end)
 		return EPOLLIN | EPOLLRDNORM;
-	if (!reader->device->exist)
+	if (!roccat_device_available(reader->device))
 		return EPOLLERR | EPOLLHUP;
 	return 0;
 }
@@ -231,7 +237,7 @@ static int roccat_release(struct inode *inode, struct file *file)
 
 	if (!--device->open) {
 		/* removing last reader */
-		if (device->exist) {
+		if (roccat_device_available(device)) {
 			hid_hw_power(device->hid, PM_HINT_NORMAL);
 			hid_hw_close(device->hid);
 		} else {
@@ -350,7 +356,6 @@ int roccat_connect(const struct class *klass, struct hid_device *hid, int report
 	mutex_init(&device->cbuf_lock);
 	device->minor = minor;
 	device->hid = hid;
-	device->exist = 1;
 	device->cbuf_end = 0;
 	device->report_size = report_size;
 
@@ -369,10 +374,8 @@ void roccat_disconnect(int minor)
 	mutex_lock(&devices_lock);
 	device = devices[minor];
 
-	device->exist = 0; /* TODO exist maybe not needed */
-
 	device_destroy(device->dev->class, MKDEV(roccat_major, minor));
-
+	WRITE_ONCE(device->dev, NULL);
 	devices[minor] = NULL;
 
 	if (device->open) {
-- 
2.55.0

[PATCH v5 2/3] 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>
---
v5: adjusted to match 7.3-rc5
v3, v4: unchanged
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 96aa508111a1..f50ae6b34e95 100644
--- a/drivers/hid/hid-roccat.c
+++ b/drivers/hid/hid-roccat.c
@@ -217,24 +217,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(!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 (roccat_device_available(device)) {
-- 
2.55.0

[PATCH v5 3/3] HID: roccat: use kref to manage device instances

[Dmitry Antipov]

Use kref to manage 'struct roccat_device' instances and fix both memory
leaks and UaF-triggering races between roccat_open()/roccat_release()
and roccat_connect()/roccat_disconnect() pairs. To avoid the scenario
when opened device is disconnected and its slot indexed by minor number
is reused by another device, release the slot in roccat_free_device()
rather than in roccat_disconnect(). This way, there is a time frame when
disconnected device may still occupy the slot; to reject such a device,
add extra roccat_device_available() checks to roccat_open() and
roccat_ioctl(). Add extra WARN_ON() device check to roccat_disconnect()
and a few debugging quirks to roccat_free_device() as well.

Reported-by: syzbot+d632e93ffcd1452bc61e@syzkaller.appspotmail.com
Reported-by: syzbot+c492a9e154f81127551f@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=d632e93ffcd1452bc61e
Closes: https://syzkaller.appspot.com/bug?extid=c492a9e154f81127551f
Link: https://sashiko.dev/#/patchset/20260902094551.200587-1-dmantipov@yandex.ru?part=2
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v5: adjusted to match 7.3-rc5
v4: unchanged
v3: release device slot in roccat_free_device(), add required checks
    to roccat_open() and roccat_ioctl(), few more debugging quirks
v2: unconditionally get/put device reference during
    first open and last close, respectively (Sashiko)
---
 drivers/hid/hid-roccat.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/drivers/hid/hid-roccat.c b/drivers/hid/hid-roccat.c
index f50ae6b34e95..d8dfc7f22026 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 open;
+	struct kref ref;
 	wait_queue_head_t wait;
 	struct device *dev;
 	struct hid_device *hid;
@@ -76,12 +77,20 @@ static bool roccat_device_available(struct roccat_device *device)
 	return dev ? device_is_registered(dev) : false;
 }
 
-static void roccat_free_device(struct roccat_device *device)
+static void roccat_free_device(struct kref *ref)
 {
+	struct roccat_device *device;
 	int i;
 
+	WARN_ON(!mutex_is_locked(&devices_lock));
+
+	device = container_of(ref, struct roccat_device, ref);
 	for (i = 0; i < ROCCAT_CBUF_SIZE; i++)
 		kfree(device->cbuf[i].value);
+
+	devices[device->minor] = NULL;
+	mutex_destroy(&device->readers_lock);
+	mutex_destroy(&device->cbuf_lock);
 	kfree(device);
 }
 
@@ -175,7 +184,7 @@ static int roccat_open(struct inode *inode, struct file *file)
 
 	device = devices[minor];
 
-	if (!device) {
+	if (!device || !roccat_device_available(device)) {
 		pr_emerg("roccat device with minor %d doesn't exist\n", minor);
 		error = -ENODEV;
 		goto exit_err_devices;
@@ -197,6 +206,7 @@ static int roccat_open(struct inode *inode, struct file *file)
 			--device->open;
 			goto exit_err_readers;
 		}
+		kref_get(&device->ref);
 	}
 
 	reader->device = device;
@@ -235,9 +245,8 @@ static int roccat_release(struct inode *inode, struct file *file)
 		if (roccat_device_available(device)) {
 			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);
@@ -349,6 +358,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->cbuf_end = 0;
@@ -368,18 +378,21 @@ 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));
-	WRITE_ONCE(device->dev, NULL);
-	devices[minor] = NULL;
+	if (!WARN_ON(!roccat_device_available(device))) {
+		device_destroy(device->dev->class, MKDEV(roccat_major, minor));
+		WRITE_ONCE(device->dev, NULL);
+	}
 
 	if (device->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);
@@ -394,7 +407,7 @@ static long roccat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	mutex_lock(&devices_lock);
 
 	device = devices[minor];
-	if (!device) {
+	if (!device || !roccat_device_available(device)) {
 		retval = -ENODEV;
 		goto out;
 	}
-- 
2.55.0

Re: [PATCH v5 1/3] HID: roccat: use device_is_registered() to check whether device is available

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] HID: roccat: Use-After-Free in roccat_device_available() via device_is_registered()
--

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

    HID: roccat: use device_is_registered() to check whether device is available

This patch introduces roccat_device_available() to check if a device is
available (using device_is_registered()), removing the explicit 'exist'
flag from struct roccat_device.

> diff --git a/drivers/hid/hid-roccat.c b/drivers/hid/hid-roccat.c
> index 5deb6da8d4f74..96aa508111a1b 100644
> --- a/drivers/hid/hid-roccat.c
> +++ b/drivers/hid/hid-roccat.c
[ ... ]
> @@ -70,6 +69,13 @@ static struct roccat_device *devices[ROCCAT_MAX_DEVICES];
>  /* protects modifications of devices array */
>  static DEFINE_MUTEX(devices_lock);
>  
> +static bool roccat_device_available(struct roccat_device *device)
> +{
> +	struct device *dev = READ_ONCE(device->dev);
> +
> +	return dev ? device_is_registered(dev) : false;

[Severity: High]
Are we dereferencing the cached dev pointer here without holding a reference
count?

If a thread reads the dev pointer here, gets preempted while device_destroy()
frees it, and then evaluates device_is_registered(dev), could this lead to a
Use-After-Free?

This is not fixed by subsequent patches in the series (e.g., commit
"HID: roccat: use kref to manage device instances"), which only refcount
the struct roccat_device, not the struct device.

> +}
> +
>  static void roccat_free_device(struct roccat_device *device)
>  {
>  	int i;
[ ... ]
> @@ -149,7 +155,7 @@ static __poll_t roccat_poll(struct file *file, poll_table *wait)
>  	poll_wait(file, &reader->device->wait, wait);
>  	if (reader->cbuf_start != reader->device->cbuf_end)
>  		return EPOLLIN | EPOLLRDNORM;
> -	if (!reader->device->exist)
> +	if (!roccat_device_available(reader->device))

[Severity: High]
Is it possible for a lockless caller like roccat_poll() to trigger the
Use-After-Free vulnerability mentioned above by reading dev concurrently?

>  		return EPOLLERR | EPOLLHUP;
>  	return 0;
>  }
[ ... ]
> @@ -369,10 +374,8 @@ void roccat_disconnect(int minor)
>  	mutex_lock(&devices_lock);
>  	device = devices[minor];
>  
> -	device->exist = 0; /* TODO exist maybe not needed */
> -
>  	device_destroy(device->dev->class, MKDEV(roccat_major, minor));
> -
> +	WRITE_ONCE(device->dev, NULL);

[Severity: High]
Can this sequence unregister and free the struct device memory concurrently
before the pointer is cleared, causing the Use-After-Free?

Could device_destroy() drop the final reference and free the struct device
memory immediately before WRITE_ONCE() clears the pointer?

>  	devices[minor] = NULL;
>  
>  	if (device->open) {

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

Re: (subset) [PATCH v5 0/3] Latest Roccat HID fixes

[Benjamin Tissoires]

On Tue, 29 Sep 2026 09:57:40 +0300, Dmitry Antipov wrote:
> Now d76994443eed ("HID: roccat: fix locking in roccat_connect() and
> roccat_disconnect()") is landed, and these ones are expected to fix
> https://syzkaller.appspot.com/bug?extid=d632e93ffcd1452bc61e and
> https://syzkaller.appspot.com/bug?extid=c492a9e154f81127551f.
> 
> Dmitry Antipov (3):
>   HID: roccat: use device_is_registered() to check whether device is available
>   HID: roccat: fix device access in roccat_release()
>   HID: roccat: use kref to manage device instances
> 
> [...]

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git (for-7.4/roccat), thanks!

[2/3] HID: roccat: fix device access in roccat_release()
      https://git.kernel.org/hid/hid/c/b0a42af46363

Cheers,
-- 
Benjamin Tissoires <bentiss@kernel.org>

Re: [PATCH v5 0/3] Latest Roccat HID fixes

[Benjamin Tissoires]

Hi Dmitry,

On Sep 29 2026, Dmitry Antipov wrote:
> Now d76994443eed ("HID: roccat: fix locking in roccat_connect() and
> roccat_disconnect()") is landed, and these ones are expected to fix
> https://syzkaller.appspot.com/bug?extid=d632e93ffcd1452bc61e and
> https://syzkaller.appspot.com/bug?extid=c492a9e154f81127551f.
> 
> Dmitry Antipov (3):
>   HID: roccat: use device_is_registered() to check whether device is available

The Sashiko report is actually more concerning than anything. Given
that, I don't think we should keep the patch in the series. Unless you
manage to fix all Sashiko comments, but it feels like a lot of effort
for little gain.

>   HID: roccat: fix device access in roccat_release()

I took this one (see other confirmation email).

>   HID: roccat: use kref to manage device instances

I wanted to take this one but it doesn't apply cleanly with patch 1
dropped. Can you respin it on top of for-7.4/roccat, please?


Cheers,
Benjamin

> 
>  drivers/hid/hid-roccat.c | 63 +++++++++++++++++++++++-----------------
>  1 file changed, 37 insertions(+), 26 deletions(-)
> 
> -- 
> 2.55.0
> 
> 

[PATCH v6 0/2] Last remaining Roccat HID fixes

[Dmitry Antipov]

Adjusted to and basically tested with for-7.4/roccat branch of
https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git;
https://syzkaller.appspot.com/bug?extid=d632e93ffcd1452bc61e and
https://syzkaller.appspot.com/bug?extid=c492a9e154f81127551f looks
passed as well.

I'm still thinking that roccat_device_available() is worth trying.
Anyway, let's make Sashiko to do one more round. In particular,

device_destroy(device->dev->class, MKDEV(roccat_major, minor));
WRITE_ONCE(device->dev, NULL);

was wrong because it opens tiny race window when the destroying
device may be examined with device_is_registered() called from
roccat_device_available(). This is expected to be fixed with:

struct device *dev = xchg(&device->dev, NULL);
device_destroy(dev->class, MKDEV(roccat_major, minor));

Dmitry Antipov (2):
  HID: roccat: use device_is_registered() to check whether device is available
  HID: roccat: use kref to manage device instances

 drivers/hid/hid-roccat.c | 49 +++++++++++++++++++++++++++-------------
 1 file changed, 33 insertions(+), 16 deletions(-)

-- 
2.56.0

[PATCH v6 1/2] HID: roccat: use device_is_registered() to check whether device is available

[Dmitry Antipov]

Introduce roccat_device_available() to check whether device
is actually available (i.e. not disconnected), thus removing
explicit 'exist' flag from 'struct roccat_device'.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v6: adjusted to match for-7.4/roccat branch of
    https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git,
    switch to xchg()/smp_load_acquire() and fix possible
    race due to wrong logic in roccat_disconnect()
v5: adjusted to match 7.3-rc5
v4: use READ_ONCE() and WRITE_ONCE() for device access
v3: initial version to join the series
---
 drivers/hid/hid-roccat.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/hid/hid-roccat.c b/drivers/hid/hid-roccat.c
index b3e69c79c4b4..882a6150efd2 100644
--- a/drivers/hid/hid-roccat.c
+++ b/drivers/hid/hid-roccat.c
@@ -40,7 +40,6 @@ struct roccat_device {
 	unsigned int minor;
 	int report_size;
 	int open;
-	int exist;
 	wait_queue_head_t wait;
 	struct device *dev;
 	struct hid_device *hid;
@@ -70,6 +69,13 @@ static struct roccat_device *devices[ROCCAT_MAX_DEVICES];
 /* protects modifications of devices array */
 static DEFINE_MUTEX(devices_lock);
 
+static bool roccat_device_available(struct roccat_device *device)
+{
+	struct device *dev = smp_load_acquire(&device->dev);
+
+	return dev ? device_is_registered(dev) : false;
+}
+
 static void roccat_free_device(struct roccat_device *device)
 {
 	int i;
@@ -105,7 +111,7 @@ static ssize_t roccat_read(struct file *file, char __user *buffer,
 				retval = -ERESTARTSYS;
 				break;
 			}
-			if (!device->exist) {
+			if (!roccat_device_available(device)) {
 				retval = -EIO;
 				break;
 			}
@@ -149,7 +155,7 @@ static __poll_t roccat_poll(struct file *file, poll_table *wait)
 	poll_wait(file, &reader->device->wait, wait);
 	if (reader->cbuf_start != reader->device->cbuf_end)
 		return EPOLLIN | EPOLLRDNORM;
-	if (!reader->device->exist)
+	if (!roccat_device_available(reader->device))
 		return EPOLLERR | EPOLLHUP;
 	return 0;
 }
@@ -226,7 +232,7 @@ static int roccat_release(struct inode *inode, struct file *file)
 
 	if (!--device->open) {
 		/* removing last reader */
-		if (device->exist) {
+		if (roccat_device_available(device)) {
 			hid_hw_power(device->hid, PM_HINT_NORMAL);
 			hid_hw_close(device->hid);
 		} else {
@@ -345,7 +351,6 @@ int roccat_connect(const struct class *klass, struct hid_device *hid, int report
 	mutex_init(&device->cbuf_lock);
 	device->minor = minor;
 	device->hid = hid;
-	device->exist = 1;
 	device->cbuf_end = 0;
 	device->report_size = report_size;
 
@@ -360,13 +365,13 @@ EXPORT_SYMBOL_GPL(roccat_connect);
 void roccat_disconnect(int minor)
 {
 	struct roccat_device *device;
+	struct device *dev;
 
 	mutex_lock(&devices_lock);
 	device = devices[minor];
 
-	device->exist = 0; /* TODO exist maybe not needed */
-
-	device_destroy(device->dev->class, MKDEV(roccat_major, minor));
+	dev = xchg(&device->dev, NULL);
+	device_destroy(dev->class, MKDEV(roccat_major, minor));
 
 	devices[minor] = NULL;
 
-- 
2.56.0

[PATCH v6 2/2] HID: roccat: use kref to manage device instances

[Dmitry Antipov]

Use kref to manage 'struct roccat_device' instances and fix both memory
leaks and UaF-triggering races between roccat_open()/roccat_release()
and roccat_connect()/roccat_disconnect() pairs. To avoid the scenario
when opened device is disconnected and its slot indexed by minor number
is reused by another device, release the slot in roccat_free_device()
rather than in roccat_disconnect(). This way, there is a time frame when
disconnected device may still occupy the slot; to reject such a device,
add extra roccat_device_available() checks to roccat_open() and
roccat_ioctl(). Add extra WARN_ON() device check to roccat_disconnect()
and a few debugging quirks to roccat_free_device() as well.

Reported-by: syzbot+d632e93ffcd1452bc61e@syzkaller.appspotmail.com
Reported-by: syzbot+c492a9e154f81127551f@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=d632e93ffcd1452bc61e
Closes: https://syzkaller.appspot.com/bug?extid=c492a9e154f81127551f
Link: https://sashiko.dev/#/patchset/20260902094551.200587-1-dmantipov@yandex.ru?part=2
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v6: adjusted to match for-7.4/roccat branch of
    https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git
v5: adjusted to match 7.3-rc5
v4: unchanged
v3: release device slot in roccat_free_device(), add required checks
    to roccat_open() and roccat_ioctl(), few more debugging quirks
v2: unconditionally get/put device reference during
    first open and last close, respectively (Sashiko)
---
 drivers/hid/hid-roccat.c | 34 +++++++++++++++++++++++-----------
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/drivers/hid/hid-roccat.c b/drivers/hid/hid-roccat.c
index 882a6150efd2..fdb9dc2d758f 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 open;
+	struct kref ref;
 	wait_queue_head_t wait;
 	struct device *dev;
 	struct hid_device *hid;
@@ -76,12 +77,20 @@ static bool roccat_device_available(struct roccat_device *device)
 	return dev ? device_is_registered(dev) : false;
 }
 
-static void roccat_free_device(struct roccat_device *device)
+static void roccat_free_device(struct kref *ref)
 {
+	struct roccat_device *device;
 	int i;
 
+	WARN_ON(!mutex_is_locked(&devices_lock));
+
+	device = container_of(ref, struct roccat_device, ref);
 	for (i = 0; i < ROCCAT_CBUF_SIZE; i++)
 		kfree(device->cbuf[i].value);
+
+	devices[device->minor] = NULL;
+	mutex_destroy(&device->readers_lock);
+	mutex_destroy(&device->cbuf_lock);
 	kfree(device);
 }
 
@@ -175,7 +184,7 @@ static int roccat_open(struct inode *inode, struct file *file)
 
 	device = devices[minor];
 
-	if (!device) {
+	if (!device || !roccat_device_available(device)) {
 		pr_emerg("roccat device with minor %d doesn't exist\n", minor);
 		error = -ENODEV;
 		goto exit_err_devices;
@@ -197,6 +206,7 @@ static int roccat_open(struct inode *inode, struct file *file)
 			--device->open;
 			goto exit_err_readers;
 		}
+		kref_get(&device->ref);
 	}
 
 	reader->device = device;
@@ -235,9 +245,8 @@ static int roccat_release(struct inode *inode, struct file *file)
 		if (roccat_device_available(device)) {
 			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);
@@ -349,6 +358,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->cbuf_end = 0;
@@ -365,23 +375,25 @@ EXPORT_SYMBOL_GPL(roccat_connect);
 void roccat_disconnect(int minor)
 {
 	struct roccat_device *device;
-	struct device *dev;
 
 	mutex_lock(&devices_lock);
 	device = devices[minor];
+	if (WARN_ON(!device))
+		goto out;
 
-	dev = xchg(&device->dev, NULL);
-	device_destroy(dev->class, MKDEV(roccat_major, minor));
+	if (!WARN_ON(!roccat_device_available(device))) {
+		struct device *dev = xchg(&device->dev, NULL);
 
-	devices[minor] = NULL;
+		device_destroy(dev->class, MKDEV(roccat_major, minor));
+	}
 
 	if (device->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);
@@ -396,7 +408,7 @@ static long roccat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	mutex_lock(&devices_lock);
 
 	device = devices[minor];
-	if (!device) {
+	if (!device || !roccat_device_available(device)) {
 		retval = -ENODEV;
 		goto out;
 	}
-- 
2.56.0

Re: [PATCH v6 1/2] HID: roccat: use device_is_registered() to check whether device is available

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] hid: Use-After-Free of struct device in roccat_device_available()
- [High] hid: roccat: UAF in roccat_device_available() during device removal
--

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

HID: roccat: use device_is_registered() to check whether device is available

This commit introduces roccat_device_available() to check if a device is
registered. It removes the explicit 'exist' flag from struct roccat_device.

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

[ ... ]

> @@ -70,6 +69,13 @@ static struct roccat_device *devices[ROCCAT_MAX_DEVICES];
>  /* protects modifications of devices array */
>  static DEFINE_MUTEX(devices_lock);
>  
> +static bool roccat_device_available(struct roccat_device *device)
> +{
> +	struct device *dev = smp_load_acquire(&device->dev);
> +
> +	return dev ? device_is_registered(dev) : false;

[Severity: High]
Does this create a Use-After-Free risk when evaluated concurrently with device
removal?

If roccat_read() or roccat_poll() fetches the 'dev' pointer via
smp_load_acquire() but is then preempted before calling
device_is_registered():

Thread 1 (roccat_read/roccat_poll)
roccat_device_available()
    struct device *dev = smp_load_acquire(&device->dev);
    <preempted>

Thread 2 (roccat_disconnect)
    dev = xchg(&device->dev, NULL);
    device_destroy(dev->class, MKDEV(roccat_major, minor));

Since device_destroy() synchronously drops the reference and can free the
underlying struct device memory, could Thread 1 then resume and pass the freed
'dev' pointer to device_is_registered(), dereferencing freed memory?

[ ... ]

> @@ -365,13 +370,13 @@ EXPORT_SYMBOL_GPL(roccat_connect);
>  void roccat_disconnect(int minor)
>  {
>  	struct roccat_device *device;
> +	struct device *dev;
>  
>  	mutex_lock(&devices_lock);
>  	device = devices[minor];
>  
> -	device->exist = 0; /* TODO exist maybe not needed */
> -
> -	device_destroy(device->dev->class, MKDEV(roccat_major, minor));
> +	dev = xchg(&device->dev, NULL);
> +	device_destroy(dev->class, MKDEV(roccat_major, minor));

[Severity: High]
Since roccat_device_available() can be called from read and poll handlers
without holding devices_lock, does swapping the pointer and immediately
destroying the device leave concurrent readers unprotected?

>  
>  	devices[minor] = NULL;
>

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

Re: [PATCH v6 2/2] HID: roccat: use kref to manage device instances

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] hid: roccat: UAF in roccat_device_available() during disconnect
--

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

HID: roccat: use kref to manage device instances

Use kref to manage 'struct roccat_device' instances and fix both memory
leaks and UaF-triggering races between roccat_open()/roccat_release()
and roccat_connect()/roccat_disconnect() pairs. To avoid the scenario
when opened device is disconnected and its slot indexed by minor number
is reused by another device, release the slot in roccat_free_device()
rather than in roccat_disconnect(). This way, there is a time frame when
disconnected device may still occupy the slot; to reject such a device,
add extra roccat_device_available() checks to roccat_open() and
roccat_ioctl(). Add extra WARN_ON() device check to roccat_disconnect()
and a few debugging quirks to roccat_free_device() as well.

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 92a9b9f14f591..581dedde6d66e 100644
> --- a/drivers/hid/hid-roccat.c
> +++ b/drivers/hid/hid-roccat.c
[ ... ]
> @@ -370,23 +380,25 @@ EXPORT_SYMBOL_GPL(roccat_connect);
>  void roccat_disconnect(int minor)
>  {
>  	struct roccat_device *device;
> -	struct device *dev;
>  
>  	mutex_lock(&devices_lock);
>  	device = devices[minor];
> +	if (WARN_ON(!device))
> +		goto out;
>  
> -	dev = xchg(&device->dev, NULL);
> -	device_destroy(dev->class, MKDEV(roccat_major, minor));
> +	if (!WARN_ON(!roccat_device_available(device))) {
> +		struct device *dev = xchg(&device->dev, NULL);
>  
> -	devices[minor] = NULL;
> +		device_destroy(dev->class, MKDEV(roccat_major, minor));
> +	}
>  
>  	if (device->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]
Can this concurrent disconnect path trigger a use-after-free in readers?

If CPU1 calls roccat_poll() or roccat_read(), they invoke
roccat_device_available() without holding devices_lock. That helper loads the
pointer locklessly:

drivers/hid/hid-roccat.c:roccat_device_available() {
	struct device *dev = smp_load_acquire(&device->dev);

	return dev ? device_is_registered(dev) : false;
}

If CPU1 is preempted right after smp_load_acquire(), and CPU2 runs
roccat_disconnect(), CPU2 will exchange device->dev to NULL and call
device_destroy(). This drops the final reference and frees the device memory.

When CPU1 resumes, it will pass the now-freed dev pointer to
device_is_registered(), causing a use-after-free when accessing
dev->kobj.state_in_sysfs.

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