[PATCH v7 2/3] HID: roccat: use kref to manage device instances
flat view
COOLING13d
SUPERSEDED
From: Dmitry Antipov <hidden>
Date: 2026-10-19 16:53:03
Subsystem:
hid core layer, roccat drivers, the rest · Maintainers:
Jiri Kosina, Benjamin Tissoires, Stefan Achatz, Linus Torvalds
Status: Superseded by v8 (posted 2026-10-22). [go to v8]
Revision v7 (3 revisions archived).
Revisions (3)
- v6
- v7 current [diff against previous]
- v8 [diff against previous]
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: <redacted> Reported-by: <redacted> 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 <redacted> --- v7: use restored device status flag again 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 | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/drivers/hid/hid-roccat.c b/drivers/hid/hid-roccat.c
index c76929c0456e..5cc523635445 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; atomic_t exist; wait_queue_head_t wait; struct device *dev;
@@ -75,12 +76,20 @@ static bool roccat_device_available(struct roccat_device *device) return atomic_read(&device->exist); } -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); }
@@ -174,7 +183,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;
@@ -196,6 +205,7 @@ static int roccat_open(struct inode *inode, struct file *file) --device->open; goto exit_err_readers; } + kref_get(&device->ref); } reader->device = device;
@@ -234,9 +244,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);
@@ -348,6 +357,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; atomic_set(&device->exist, 1);
@@ -369,19 +379,19 @@ void roccat_disconnect(int minor) mutex_lock(&devices_lock); device = devices[minor]; - atomic_set(&device->exist, 0); - - device_destroy(device->dev->class, MKDEV(roccat_major, minor)); + if (WARN_ON(!device)) + goto out; - devices[minor] = NULL; + if (!WARN_ON(!atomic_cmpxchg(&device->exist, 1, 0))) + device_destroy(device->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 +406,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