Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Critical] A data race and locking mismatch between `ring_buffer_subbuf_order_set()`, `ring_buffer_alloc_read_page()`, and `ring_buffer_free_read_page()` allows `cpu_buffer->free_page` to cache a page of an old order while `buffer->subbuf_order` reflects a new larger order. This leads to heap buffer overflow and buddy allocator corruption. -- --- Patch [1]: [PATCH v5 01/10] ring-buffer: Free cpu_buffer::free_page with subbuf_order --- commit f550c0bfeadcc22eca47316e1820b4d91f6ef261 Author: Vincent Donnefort <[email protected]> ring-buffer: Free cpu_buffer::free_page with subbuf_order When sub-buffers use an order greater than 0, cpu_buffer->free_page is allocated with subbuf_order. Use the correct order for cpu_buffer->free_page. > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c > index 2667992f0aa29..a3d28b2e2c94a 100644 > --- a/kernel/trace/ring_buffer.c > +++ b/kernel/trace/ring_buffer.c > @@ -2631,7 +2631,7 @@ static void rb_free_cpu_buffer(struct > ring_buffer_per_cpu *cpu_buffer) > free_buffer_page(bpage); > } > > - free_page((unsigned long)cpu_buffer->free_page); > + free_pages((unsigned long)cpu_buffer->free_page, > cpu_buffer->buffer->subbuf_order); [Severity: Critical] This is a pre-existing issue, but does this change expose a buddy allocator corruption if cpu_buffer->free_page contains a page of a smaller order than buffer->subbuf_order? A race condition between ring_buffer_subbuf_order_set() and ring_buffer_free_read_page() can cause a smaller order page to be stashed in cpu_buffer->free_page while buffer->subbuf_order reflects a new larger order. If a trace reader calls ring_buffer_free_read_page(), it performs a lockless check of buffer->subbuf_order: kernel/trace/ring_buffer.c:ring_buffer_free_read_page() { ... if (page_ref_count(page) > 1 || data_page->order != buffer->subbuf_order) goto out; ... } If a concurrent write to buffer_subbuf_size_kb calls ring_buffer_subbuf_order_set(), it updates buffer->subbuf_order without holding cpu_buffer->lock, and clears cpu_buffer->free_page under reader_lock. The trace reader then acquires cpu_buffer->lock and stashes the old smaller page into cpu_buffer->free_page, missing the clearance because of the lock mismatch (reader_lock vs cpu_buffer->lock): kernel/trace/ring_buffer.c:ring_buffer_free_read_page() { ... raw_spin_lock_irqsave(&cpu_buffer->lock, flags); if (!cpu_buffer->free_page) { cpu_buffer->free_page = dpage; ... } During teardown, free_pages() here would free the undersized page using the new larger order, causing buddy allocator corruption. Additionally, if a reader allocates this stashed page via ring_buffer_read_page(), it is swapped into the active ring buffer where writers will overflow its bounds. Can the locking be synchronized to prevent caching an old-order page? > > kfree(cpu_buffer); > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
