Re: [PATCH] ring-buffer: Check resize_disabled before publishing the new subbuf order
flat view
From: sashiko-bot@kernel.org
Date: 2026-09-12 10:56:59
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Missing cpus_read_lock() allows concurrent CPU hotplug events to crash ring_buffer_subbuf_order_set() via uninitialized list dereference or NULL pointer dereference. -- commit d554518f86e022de0069fc76033b9d71e9046148 Author: David Carlier [off-list ref] ring-buffer: Check resize_disabled before publishing the new subbuf order This commit checks if resizing is disabled on any CPU before applying the new sub-buffer order. This aims to prevent a race condition with mmap fast paths mapping already-mapped CPUs with the new order. Cc: stable@vger.kernel.org
quoted hunk ↗ jump to hunk
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c index 9c03a555a6ba2..d7e5e4620d096 100644 --- a/kernel/trace/ring_buffer.c +++ b/kernel/trace/ring_buffer.c@@ -7474,6 +7474,14 @@ int ring_buffer_subbuf_order_set(struct trace_buffer *buffer, int order) old_capacity = rb_subbuf_capacity(buffer); + /* The mmap fast path reads subbuf_order without buffer->mutex. */ + for_each_buffer_cpu(buffer, cpu) { + if (!cpumask_test_cpu(cpu, buffer->cpumask)) + continue; + if (atomic_read(&buffer->buffers[cpu]->resize_disabled)) + return -EBUSY; + } +
[Severity: High]
Can this new loop race with concurrent CPU hotplug events and trigger a NULL
pointer dereference?
Unlike ring_buffer_resize(), ring_buffer_subbuf_order_set() doesn't appear
to take cpus_read_lock(). Meanwhile, trace_rb_cpu_prepare() can update
buffer->cpumask without holding buffer->mutex.
On weakly ordered architectures, could the lack of an smp_rmb() here cause
the reader to observe a newly added CPU in buffer->cpumask but load a NULL
buffer->buffers[cpu]?
Additionally, without cpus_read_lock(), if a CPU is added to the cpumask
between the allocation loop and the final page update loop later in this
function, new_pages will be an empty list head:
ring_buffer_subbuf_order_set() {
...
/* One page was allocated for the reader page */
cpu_buffer->reader_page = list_entry(cpu_buffer->new_pages.next,
struct buffer_page, list);
list_del_init(&cpu_buffer->reader_page->list);
...
}
Could this calculate a fake pointer overlapping the cpu_buffer struct and
corrupt memory when modified?
atomic_inc(&buffer->record_disabled); /* Make sure all commits have finished */
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260912103938.1127021-1-devnexen@gmail.com?part=1