Re: [PATCH] ring-buffer: serialize read-page order with subbuffer resize

[Yousef Alhouseen]

Thanks, Steve. Moving the order reads under the existing per-CPU locks
is much tighter and avoids extending the allocation critical path.
I’ll compile and test this direction and send v2.

On Tue, 30 Jun 2026 10:14:25 -0400, Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> On Sun, 28 Jun 2026 02:46:53 +0200
> Yousef Alhouseen <alhouseenyousef@xxxxxxxxx> wrote:
>
> > ring_buffer_read_page() checks that its spare page has the current
> > subbuffer order before taking cpu_buffer->reader_lock. A concurrent
> > ring_buffer_subbuf_order_set() can change the order and replace the
> > reader page after that check. The reader then copies a larger subbuffer
> > into the old allocation, causing an out-of-bounds write.
> >
> > Keep spare-page allocation and release under buffer->mutex, which already
> > serializes order changes. Move the read-side order check under
> > reader_lock, the lock used by resize when replacing per-CPU pages.
> >
> > Fixes: f9b94daa542a ("ring-buffer: Set new size of the ring buffer sub page")
> > Reported-by: syzbot+2dd9d02f60775ce5c1fb@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Closes: https://syzkaller.appspot.com/bug?extid=2dd9d02f60775ce5c1fb
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Yousef Alhouseen <alhouseenyousef@xxxxxxxxx>
> > ---
> > kernel/trace/ring_buffer.c | 9 ++++++---
> > 1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> > index 56a328e94395..eed5d7cffdee 100644
> > --- a/kernel/trace/ring_buffer.c
> > +++ b/kernel/trace/ring_buffer.c
> > @@ -6950,6 +6950,8 @@ ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu)
> > if (!cpumask_test_cpu(cpu, buffer->cpumask))
> > return ERR_PTR(-ENODEV);
> >
> > + guard(mutex)(&buffer->mutex);
> > +
> > bpage = kzalloc_obj(*bpage);
>
> First, do not grab locks around allocations unless the are really needed.
> This is bad practice, as it extends the critical section and may even add
> the allocation locking to the lock chain.
>
> That said, just moving things around the current locks should work.
>
> Like this (not compiled nor tested):
>
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 56a328e94395..8352f935a223 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -6954,11 +6954,11 @@ ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu)
> if (!bpage)
> return ERR_PTR(-ENOMEM);
>
> - bpage->order = buffer->subbuf_order;
> cpu_buffer = buffer->buffers[cpu];
> local_irq_save(flags);
> arch_spin_lock(&cpu_buffer->lock);
>
> + bpage->order = buffer->subbuf_order;
> if (cpu_buffer->free_page) {
> bpage->data = cpu_buffer->free_page;
> cpu_buffer->free_page = NULL;
> @@ -7007,13 +7007,13 @@ void ring_buffer_free_read_page(struct trace_buffer *buffer, int cpu,
> * is different from the subbuffer order of the buffer -
> * we can't reuse it
> */
> - if (page_ref_count(page) > 1 || data_page->order != buffer->subbuf_order)
> + if (page_ref_count(page) > 1)
> goto out;
>
> local_irq_save(flags);
> arch_spin_lock(&cpu_buffer->lock);
>
> - if (!cpu_buffer->free_page) {
> + if (!cpu_buffer->free_page && data_page->order == buffer->subbuf_order)
> cpu_buffer->free_page = dpage;
> dpage = NULL;
> }
> @@ -7091,15 +7091,15 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
> if (!data_page || !data_page->data)
> return -1;
>
> - if (data_page->order != buffer->subbuf_order)
> - return -1;
> -
> dpage = data_page->data;
> if (!dpage)
> return -1;
>
> guard(raw_spinlock_irqsave)(&cpu_buffer->reader_lock);
>
> + if (data_page->order != buffer->subbuf_order)
> + return -1;
> +
> reader = rb_get_reader_page(cpu_buffer);
> if (!reader)
> return -1;
>
> -- Steve