On Wed, Aug 12, 2026 at 03:56:42PM +0000, [email protected] 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 <[email protected]> > > 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/[email protected]?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
