Re: [PATCH v5 1/3] HID: roccat: use device_is_registered() to check whether device is available
flat view
From: sashiko-bot@kernel.org
Date: 2026-09-29 07:14:52
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 [off-list ref]
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.
quoted hunk ↗ jump to hunk
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
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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.
quoted hunk ↗ jump to hunk
+} + static void roccat_free_device(struct roccat_device *device) { int i;
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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; }
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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