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
