Re: [PATCH v4 6/9] ring-buffer: Dynamically calculate max_data_size

[Vincent Donnefort <vdonnefort@google.com>]

On Wed, Aug 12, 2026 at 03:56:42PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [Critical] Concurrent execution of ring_buffer_alloc_read_page() and ring_buffer_subbuf_order_set() leads to buddy allocator corruption and potential OOB memory access due to a mismatched page order when taking cpu_buffer->free_page.
> --
> 
> --- Patch [6]: [PATCH v4 6/9] ring-buffer: Dynamically calculate max_data_size ---
> commit a7794ac711440e6d89d8e099e4a88707ad083ed7
> Author: Vincent Donnefort <vdonnefort@google.com>
> 
> ring-buffer: Dynamically calculate max_data_size
> 
> The ring buffer order can be dynamically modified and temporarily
> disables writing to do so. It is therefore safe to use the updated value
> to calculate the maximum event size which can be written onto the ring
> buffer.
> 
> > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> > index f62d6853ee5c0..64bf4ac853f57 100644
> > --- a/kernel/trace/ring_buffer.c
> > +++ b/kernel/trace/ring_buffer.c
> > @@ -598,7 +598,6 @@ struct trace_buffer {
> >  	struct ring_buffer_meta		*meta;
> >  
> >  	unsigned int			subbuf_order;
> > -	unsigned int			max_data_size;
> >  };
> >  
> >  static __always_inline unsigned int rb_subbuf_size(struct trace_buffer *buffer)
> 
> [ ... ]
> 
> [Severity: Critical]
> This is a pre-existing issue, but can concurrent execution of
> ring_buffer_alloc_read_page() and ring_buffer_subbuf_order_set() lead to buddy
> allocator corruption and potential out-of-bounds memory access?
> 
> When a user reads /sys/kernel/tracing/per_cpu/cpuX/trace_pipe_raw, it calls
> ring_buffer_alloc_read_page(). If another thread concurrently resizes the ring
> buffer via /sys/kernel/tracing/buffer_subbuf_order, it calls
> ring_buffer_subbuf_order_set().
> 
> In ring_buffer_subbuf_order_set(), buffer->subbuf_order is updated locklessly
> with respect to the reader:
> 
> kernel/trace/ring_buffer.c:ring_buffer_subbuf_order_set() {
>     ...
> 	buffer->subbuf_order = order;
> 
> 	/* Make sure all new buffers are allocated, before deleting the old ones */
> 	for_each_buffer_cpu(buffer, cpu) {
>     ...
> }
> 
> Before ring_buffer_subbuf_order_set() reaches its second loop to clear
> cpu_buffer->free_page, ring_buffer_alloc_read_page() can fetch the new
> order, grab the old page from cpu_buffer->free_page, and assign the new
> order to bpage->order:
> 
> kernel/trace/ring_buffer.c:ring_buffer_alloc_read_page() {
>     ...
> 	if (cpu_buffer->free_page) {
> 		bpage->data = cpu_buffer->free_page;
> 		cpu_buffer->free_page = NULL;
> 	}
>     ...
> }
> 
> When this page is later freed or swapped into the ring buffer, could this
> cause memory corruption because writers believe the subbuffer size is the
> new order, leading them to overwrite past the old order page?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260812153311.2328812-1-vdonnefort@google.com?part=6

Ha this is something I was meant to fix as well... we can't just blindly take
cpu_buffer::free_page! 

I think the best is to make cpu_buffer->free_page a buffer_data_read_page so we
have an order associated to it!

-- 
Vincent