Re: [PATCH v5 01/10] ring-buffer: Free cpu_buffer::free_page with subbuf_order
From: sashiko-bot@kernel.org
Date: 2026-08-13 13:54:40
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 [off-list ref]
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.
quoted hunk ↗ jump to hunk
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/20260813131152.3589632-1-vdonnefort@google.com?part=1