Thread (34 messages) flat view 34 messages, 4 authors, 7h ago

Re: [PATCH v5 03/10] ring-buffer: Make cpu_buffer::free_page a buffer_data_read_page

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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] `ring_buffer_alloc_read_page()` returning a cached page with an obsolete order causes `tracing_buffers_read()` to permanently return EOF to userspace.
--
--- Patch [3]: [PATCH v5 03/10] ring-buffer: Make cpu_buffer::free_page a buffer_data_read_page ---
commit 19f06657067d8dbcf108f6bacc6885d031739166
Author: Vincent Donnefort [off-list ref]

    ring-buffer: Make cpu_buffer::free_page a buffer_data_read_page

This commit changes the free_page cache to store the page order alongside
the page address. This fixes a mismatch when a cached reader page is
discarded after a concurrent ring buffer resize changes the global
subbuf_order.
quoted hunk ↗ jump to hunk
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index ec4f5a0c93e85..b6fa258aafe2c 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -6962,9 +6962,9 @@ ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu)
 	local_irq_save(flags);
 	arch_spin_lock(&cpu_buffer->lock);
 
-	if (cpu_buffer->free_page) {
-		bpage->data = cpu_buffer->free_page;
-		cpu_buffer->free_page = NULL;
+	if (cpu_buffer->free_page.data) {
+		*bpage = cpu_buffer->free_page;
+		cpu_buffer->free_page.data = NULL;
[Severity: High]
Does this struct assignment cause tracing_buffers_read() to permanently
return EOF to userspace during a concurrent resize?

When a resize happens in ring_buffer_subbuf_order_set(),
buffer->subbuf_order is updated first:

	buffer->subbuf_order = order;

	/* Make sure all new buffers are allocated, before deleting... */
	for_each_buffer_cpu(buffer, cpu) {

This opens a large race window before cpu_buffer->free_page.data is
cleared for each CPU.

If a user reads from trace_pipe_raw during this window,
ring_buffer_alloc_read_page() allocates bpage and initially sets
bpage->order to the new buffer->subbuf_order. However, the struct
assignment *bpage = cpu_buffer->free_page copies the old free_page,
overwriting bpage->order with the old order.

Later in tracing_buffers_read(), the caller caches this mismatched page
and sets spare_size to the new size:

		} else {
			info->spare_cpu = iter->cpu_file;
			info->spare_size = page_size;
		}

Because the orders don't match, ring_buffer_read_page() rejects the page:

	if (data_page->order != buffer->subbuf_order)
		return -1;

This returns -1, which tracing_buffers_read() treats as EOF. Subsequent
reads see info->spare_size == page_size, reuse the bad page, and
permanently return EOF.
 	}
 
 	arch_spin_unlock(&cpu_buffer->lock);
-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813131152.3589632-1-vdonnefort@google.com?part=3
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help