On Fri,  6 Jun 2025 12:12:17 +0300
Dmitry Antipov <[email protected]> wrote:

> In 'ring_buffer_subbuf_order_set()', enlarge critical section to
> ensure that error handling takes place with per-buffer mutex hold,
> thus preventing list corruption 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]>
> ---
>  kernel/trace/ring_buffer.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index e24509bd0af5..2028a24d6418 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -6908,9 +6908,6 @@ int ring_buffer_subbuf_order_set(struct trace_buffer 
> *buffer, int order)
>       buffer->subbuf_order = old_order;
>       buffer->subbuf_size = old_size;
>  
> -     atomic_dec(&buffer->record_disabled);

There's no reason to move the record_disable. Enabling recording here is
fine, as the pages being freed have not been added to the ring buffer.

> -     mutex_unlock(&buffer->mutex);
> -
>       for_each_buffer_cpu(buffer, cpu) {
>               cpu_buffer = buffer->buffers[cpu];
>  
> @@ -6923,6 +6920,9 @@ int ring_buffer_subbuf_order_set(struct trace_buffer 
> *buffer, int order)
>               }
>       }
>  
> +     atomic_dec(&buffer->record_disabled);
> +     mutex_unlock(&buffer->mutex);

As this moves the mutex to the end, we can instead just remove the
mutex_unlock()s and replace the mutex() with:

        guard(mutex)(&buffer->mutex);

Care to send a v2?

-- Steve


> +
>       return err;
>  }
>  EXPORT_SYMBOL_GPL(ring_buffer_subbuf_order_set);


Reply via email to