drivers/hid/hid-roccat.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-)
roccat_disconnect() dropped devices_lock between reading device->exist
and later checking device->open to decide whether to free the device,
letting a concurrent roccat_release() free it first and turning the
later check into a use-after-free. Fix by keeping the whole lookup,
state update, and free-or-keep decision inside one devices_lock
critical section.
roccat_connect() also published the device via devices[minor] and
device_create() before initializing device->readers_lock and
device->cbuf_lock, letting a racing open() on the newly created node
take an uninitialized mutex. Fix by completing all initialization
before publishing the device.
Reported-by: syzbot+c492a9e154f81127551f@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=c492a9e154f81127551f
Tested-by: syzbot+c492a9e154f81127551f@syzkaller.appspotmail.com
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
drivers/hid/hid-roccat.c | 32 +++++++++++++++-----------------
1 file changed, 15 insertions(+), 17 deletions(-)
diff --git a/drivers/hid/hid-roccat.c b/drivers/hid/hid-roccat.c
index 4f15eb951039..1d57f8a44a3d 100644
--- a/drivers/hid/hid-roccat.c
+++ b/drivers/hid/hid-roccat.c
@@ -316,6 +316,14 @@ int roccat_connect(const struct class *klass, struct hid_device *hid, int report
if (!device)
return -ENOMEM;
+ init_waitqueue_head(&device->wait);
+ INIT_LIST_HEAD(&device->readers);
+ mutex_init(&device->readers_lock);
+ mutex_init(&device->cbuf_lock);
+ device->hid = hid;
+ device->exist = 1;
+ device->cbuf_end = 0;
+ device->report_size = report_size;
mutex_lock(&devices_lock);
for (minor = 0; minor < ROCCAT_MAX_DEVICES; ++minor) {
@@ -324,37 +332,28 @@ int roccat_connect(const struct class *klass, struct hid_device *hid, int report
break;
}
- if (minor < ROCCAT_MAX_DEVICES) {
- devices[minor] = device;
- } else {
+ if (minor >= ROCCAT_MAX_DEVICES) {
mutex_unlock(&devices_lock);
kfree(device);
return -EINVAL;
}
+ device->minor = minor;
+
device->dev = device_create(klass, &hid->dev,
MKDEV(roccat_major, minor), NULL,
"%s%s%d", "roccat", hid->driver->name, minor);
if (IS_ERR(device->dev)) {
- devices[minor] = NULL;
mutex_unlock(&devices_lock);
temp = PTR_ERR(device->dev);
kfree(device);
return temp;
}
- mutex_unlock(&devices_lock);
+ devices[minor] = device;
- init_waitqueue_head(&device->wait);
- INIT_LIST_HEAD(&device->readers);
- mutex_init(&device->readers_lock);
- mutex_init(&device->cbuf_lock);
- device->minor = minor;
- device->hid = hid;
- device->exist = 1;
- device->cbuf_end = 0;
- device->report_size = report_size;
+ mutex_unlock(&devices_lock);
return minor;
}
@@ -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.43.0On Thu, Sep 3, 2026 at 5:42 PM Deepanshu Kartikey <kartikey406@gmail.com> wrote:
>
> roccat_disconnect() dropped devices_lock between reading device->exist
> and later checking device->open to decide whether to free the device,
> letting a concurrent roccat_release() free it first and turning the
> later check into a use-after-free. Fix by keeping the whole lookup,
> state update, and free-or-keep decision inside one devices_lock
> critical section.
>
> roccat_connect() also published the device via devices[minor] and
> device_create() before initializing device->readers_lock and
> device->cbuf_lock, letting a racing open() on the newly created node
> take an uninitialized mutex. Fix by completing all initialization
> before publishing the device.
>
> Reported-by: syzbot+c492a9e154f81127551f@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=c492a9e154f81127551f
> Tested-by: syzbot+c492a9e154f81127551f@syzkaller.appspotmail.com
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> ---
> drivers/hid/hid-roccat.c | 32 +++++++++++++++-----------------
> 1 file changed, 15 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/hid/hid-roccat.c b/drivers/hid/hid-roccat.c
> index 4f15eb951039..1d57f8a44a3d 100644
> --- a/drivers/hid/hid-roccat.c
> +++ b/drivers/hid/hid-roccat.c
> @@ -316,6 +316,14 @@ int roccat_connect(const struct class *klass, struct hid_device *hid, int report
> if (!device)
> return -ENOMEM;
>
> + init_waitqueue_head(&device->wait);
> + INIT_LIST_HEAD(&device->readers);
> + mutex_init(&device->readers_lock);
> + mutex_init(&device->cbuf_lock);
> + device->hid = hid;
> + device->exist = 1;
> + device->cbuf_end = 0;
> + device->report_size = report_size;
> mutex_lock(&devices_lock);
>
> for (minor = 0; minor < ROCCAT_MAX_DEVICES; ++minor) {
> @@ -324,37 +332,28 @@ int roccat_connect(const struct class *klass, struct hid_device *hid, int report
> break;
> }
>
> - if (minor < ROCCAT_MAX_DEVICES) {
> - devices[minor] = device;
> - } else {
> + if (minor >= ROCCAT_MAX_DEVICES) {
> mutex_unlock(&devices_lock);
> kfree(device);
> return -EINVAL;
> }
>
> + device->minor = minor;
> +
> device->dev = device_create(klass, &hid->dev,
> MKDEV(roccat_major, minor), NULL,
> "%s%s%d", "roccat", hid->driver->name, minor);
>
> if (IS_ERR(device->dev)) {
> - devices[minor] = NULL;
> mutex_unlock(&devices_lock);
> temp = PTR_ERR(device->dev);
> kfree(device);
> return temp;
> }
>
> - mutex_unlock(&devices_lock);
> + devices[minor] = device;
>
> - init_waitqueue_head(&device->wait);
> - INIT_LIST_HEAD(&device->readers);
> - mutex_init(&device->readers_lock);
> - mutex_init(&device->cbuf_lock);
> - device->minor = minor;
> - device->hid = hid;
> - device->exist = 1;
> - device->cbuf_end = 0;
> - device->report_size = report_size;
> + mutex_unlock(&devices_lock);
>
> return minor;
> }
> @@ -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.43.0
>
Gentle Reminder. Please let me know the status of this patch.
Thanks
Deepanshu
© 2016 - 2026 Red Hat, Inc.