Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Critical] Race condition during ring buffer resize leads to mismatched subbuffer order, causing heap overflow and buddy allocator corruption. -- --- Patch [2]: [PATCH v5 02/10] ring-buffer: Hold cpu_buffer::lock when resizing a subbuf --- commit b0f06308baef769af600659941453301acb7bea1 Author: Vincent Donnefort <[email protected]> ring-buffer: Hold cpu_buffer::lock when resizing a subbuf This commit adds locking around the clearing of cpu_buffer->free_page in ring_buffer_subbuf_order_set() to prevent races with readers allocating or freeing read pages. > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c > index a3d28b2e2c94a..ec4f5a0c93e85 100644 > --- a/kernel/trace/ring_buffer.c > +++ b/kernel/trace/ring_buffer.c > @@ -7431,8 +7431,10 @@ int ring_buffer_subbuf_order_set(struct trace_buffer > *buffer, int order) > cpu_buffer->nr_pages = cpu_buffer->nr_pages_to_update; > cpu_buffer->nr_pages_to_update = 0; > > + arch_spin_lock(&cpu_buffer->lock); > old_free_data_page = cpu_buffer->free_page; > cpu_buffer->free_page = NULL; > + arch_spin_unlock(&cpu_buffer->lock); [Severity: Critical] This is a pre-existing issue, but does adding this lock fully close the race window with ring_buffer_alloc_read_page()? Looking at ring_buffer_subbuf_order_set(), buffer->subbuf_order = order; is set much earlier in the function, before allocating the new pages for each CPU. During this window, a concurrent reader calling ring_buffer_alloc_read_page() can see the new global order but still claim the old, smaller free_page from the per-CPU buffer: ring_buffer_alloc_read_page() { bpage->order = buffer->subbuf_order; // gets new, larger order ... if (cpu_buffer->free_page) { bpage->data = cpu_buffer->free_page; // gets old, smaller page cpu_buffer->free_page = NULL; } } Later, if ring_buffer_free_read_page() is called, it will use the mismatched bpage->order to free the old page, corrupting the buddy allocator. Similarly, if ring_buffer_read_page() swaps this undersized page into the ring buffer, producers will overflow the heap. Should buffer->subbuf_order be updated after the per-CPU free pages are safely cleared or replaced? > > rb_head_page_activate(cpu_buffer); > -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
