On Fri,  6 Jun 2025 14:22:42 +0300
Dmitry Antipov <[email protected]> wrote:

> In 'ring_buffer_subbuf_order_set()', prefer mutex guard over explicit
> mutex operations to ensure that the buffer remains locked everywhere
> where its innards may be changed, thus preventing per-CPU sub-buffer
> lists corruptions and other concurrency-related issues.
> 
> Reported-by: [email protected]
> Closes: https://syzkaller.appspot.com/bug?extid=05d673e83ec640f0ced9
> Fixes: f9b94daa542a8 ("ring-buffer: Set new size of the ring buffer sub page")
> Signed-off-by: Dmitry Antipov <[email protected]>

Looks good to me.

Reviewed-by: Masami Hiramatsu (Google) <[email protected]>

Thanks,

> ---
> v2: use mutex guard as suggested by Steven
> ---
>  kernel/trace/ring_buffer.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index e24509bd0af5..00fc38d70e86 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -6795,7 +6795,7 @@ int ring_buffer_subbuf_order_set(struct trace_buffer 
> *buffer, int order)
>       old_size = buffer->subbuf_size;
>  
>       /* prevent another thread from changing buffer sizes */
> -     mutex_lock(&buffer->mutex);
> +     guard(mutex)(&buffer->mutex);
>       atomic_inc(&buffer->record_disabled);
>  
>       /* Make sure all commits have finished */
> @@ -6900,7 +6900,6 @@ int ring_buffer_subbuf_order_set(struct trace_buffer 
> *buffer, int order)
>       }
>  
>       atomic_dec(&buffer->record_disabled);
> -     mutex_unlock(&buffer->mutex);
>  
>       return 0;
>  
> @@ -6909,7 +6908,6 @@ int ring_buffer_subbuf_order_set(struct trace_buffer 
> *buffer, int order)
>       buffer->subbuf_size = old_size;
>  
>       atomic_dec(&buffer->record_disabled);
> -     mutex_unlock(&buffer->mutex);
>  
>       for_each_buffer_cpu(buffer, cpu) {
>               cpu_buffer = buffer->buffers[cpu];
> -- 
> 2.49.0
> 


-- 
Masami Hiramatsu (Google) <[email protected]>

Reply via email to