From: Benoît Sevens <bsevens@google.com>

roccat_report_event() iterates over the device->readers list without
holding the readers_lock. This allows a concurrent roccat_release() to
remove and free a reader while it's still being accessed, leading to a
use-after-free.

Protect the readers list traversal with the readers_lock mutex.

Signed-off-by: Benoît Sevens <bsevens@google.com>
---
 drivers/hid/hid-roccat.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/hid/hid-roccat.c b/drivers/hid/hid-roccat.c
index fd0ea52f7cba..d6fff53d4ee7 100644
--- a/drivers/hid/hid-roccat.c
+++ b/drivers/hid/hid-roccat.c
@@ -257,6 +257,7 @@ int roccat_report_event(int minor, u8 const *data)
 	if (!new_value)
 		return -ENOMEM;
 
+	mutex_lock(&device->readers_lock);
 	mutex_lock(&device->cbuf_lock);
 
 	report = &device->cbuf[device->cbuf_end];
@@ -279,6 +280,7 @@ int roccat_report_event(int minor, u8 const *data)
 	}
 
 	mutex_unlock(&device->cbuf_lock);
+	mutex_unlock(&device->readers_lock);
 
 	wake_up_interruptible(&device->wait);
 	return 0;
-- 
2.53.0.959.g497ff81fa9-goog

On Mon, 23 Mar 2026, FirstName LastName wrote:

> From: Benoît Sevens <bsevens@google.com>
> 
> roccat_report_event() iterates over the device->readers list without
> holding the readers_lock. This allows a concurrent roccat_release() to
> remove and free a reader while it's still being accessed, leading to a
> use-after-free.
> 
> Protect the readers list traversal with the readers_lock mutex.
> 
> Signed-off-by: Benoît Sevens <bsevens@google.com>
> ---
>  drivers/hid/hid-roccat.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/hid/hid-roccat.c b/drivers/hid/hid-roccat.c
> index fd0ea52f7cba..d6fff53d4ee7 100644
> --- a/drivers/hid/hid-roccat.c
> +++ b/drivers/hid/hid-roccat.c
> @@ -257,6 +257,7 @@ int roccat_report_event(int minor, u8 const *data)
>  	if (!new_value)
>  		return -ENOMEM;
>  
> +	mutex_lock(&device->readers_lock);
>  	mutex_lock(&device->cbuf_lock);
>  
>  	report = &device->cbuf[device->cbuf_end];
> @@ -279,6 +280,7 @@ int roccat_report_event(int minor, u8 const *data)
>  	}
>  
>  	mutex_unlock(&device->cbuf_lock);
> +	mutex_unlock(&device->readers_lock);

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs

Hi

Thanks for the patch! One comment below.

FirstName LastName <bsevens@google.com> wrote:
> From: Benoît Sevens <bsevens@google.com>
> 
> roccat_report_event() iterates over the device->readers list without
> holding the readers_lock. This allows a concurrent roccat_release() to
> remove and free a reader while it's still being accessed, leading to a
> use-after-free.
> 
> Protect the readers list traversal with the readers_lock mutex.
> 
> Signed-off-by: Benoît Sevens <bsevens@google.com>
> ---
>  drivers/hid/hid-roccat.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/hid/hid-roccat.c b/drivers/hid/hid-roccat.c
> index fd0ea52f7cba..d6fff53d4ee7 100644
> --- a/drivers/hid/hid-roccat.c
> +++ b/drivers/hid/hid-roccat.c
> @@ -257,6 +257,7 @@ int roccat_report_event(int minor, u8 const *data)
>  	if (!new_value)
>  		return -ENOMEM;
>  
> +	mutex_lock(&device->readers_lock);

As the readers are only accessed later, we could move the locking further
down. Not sure if it's worth it in this case.

Other than that this LGTM!

Reviewed-by: Silvan Jegen <s.jegen@gmail.com>

Cheers,
Silvan


>  	mutex_lock(&device->cbuf_lock);
>  
>  	report = &device->cbuf[device->cbuf_end];
> @@ -279,6 +280,7 @@ int roccat_report_event(int minor, u8 const *data)
>  	}
>  
>  	mutex_unlock(&device->cbuf_lock);
> +	mutex_unlock(&device->readers_lock);
>  
>  	wake_up_interruptible(&device->wait);
>  	return 0;