Thread (34 messages) flat view 34 messages, 4 authors, 51m ago

Re: [PATCH v5 02/10] ring-buffer: Hold cpu_buffer::lock when resizing a subbuf

From: sashiko-bot@kernel.org
Date: 2026-08-13 14:01:02

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 [off-list ref]

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.
quoted hunk ↗ jump to hunk
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/20260813131152.3589632-1-vdonnefort@google.com?part=2
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help