[PATCH v4 0/9] ring-buffer: Fixes for subbuf resizing and persistent buffers

[PATCH v4 0/9] ring-buffer: Fixes for subbuf resizing and persistent buffers

[Vincent Donnefort]

This series addresses multiple issues discovered with the dynamic ring
buffer resizing.

Changelog:

v4:

  - Add rb_subbuf_start() helper (Steven)
  - kerneldoc additions
  - Fix races in trace_pipe_raw readers
  - Use rb_subbuf_capacity() in ring_buffer_subbuf_order_set()
  - use rb_page_capacity() in ring_buffer_map_get_reader (Sashiko)
  - Fix 32-bit overflow in ring_buffer_subbuf_order_set() (Sashiko)
  - Hold cpu_buffer::lock when modifying cpu_buffer->free_page in
    ring_buffer_subbuf_order_set (Sashiko)

v3 (https://lore.kernel.org/all/20260810125633.3344684-1-vdonnefort@google.com/):

  - Drop first 3 patches (Rebased on 7.2-rc7)
  - Add a patch to align "nr_pages" to unsigned int
  - Add a patch to remove useless trace_buffer::cpus
  - Add unsigned long cast for rb_subbuf_size()
  - subbuf_order fix for rb_free_cpu_buffer() (Sashiko)
  - Use __always_inline just like the other accessors for the hot-path.

v2 (https://lore.kernel.org/all/20260806211306.3704194-1-vdonnefort@google.com/):

  - Prevent resizing of the persistent ring buffer
  - Add missing bpage::order init
  - Rework subbuf_size/subbuf_order (Sashiko)
  - Remove ring_buffer_per_cpu::mapped
  - Dynamically calculate trace_buffer::max_data_size

v1 (https://lore.kernel.org/all/20260805153225.2096152-1-vdonnefort@google.com/)

Vincent Donnefort (9):
  ring-buffer: Free cpu_buffer->free_page with subbuf_order
  ring-buffer: Hold cpu_buffer::lock when resizing a subbuf
  ring-buffer: Fix subbuf resize race with ring buffer readers
  ring-buffer: Fix subbuf resize race in ring_buffer_alloc_read_page()
  tracing: Fix subbuf resize races in trace_pipe_raw readers
  ring-buffer: Dynamically calculate max_data_size
  ring-buffer: Remove trace_buffer::cpus
  ring-buffer: Remove ring_buffer_per_cpu::mapped
  ring-buffer: Make nr_pages unsigned int

 include/linux/ring_buffer.h          |   4 +-
 kernel/trace/ring_buffer.c           | 354 +++++++++++++++++----------
 kernel/trace/ring_buffer_benchmark.c |   2 +-
 kernel/trace/trace.c                 |  92 +++----
 kernel/trace/trace.h                 |   1 -
 5 files changed, 269 insertions(+), 184 deletions(-)


base-commit: db2ddb87143519e20a95aa36c60b36107b736a58
-- 
2.55.0.691.gc56d675ccc-goog

[PATCH v4 1/9] ring-buffer: Free cpu_buffer->free_page with subbuf_order

[Vincent Donnefort]

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.

Fixes: f9b94daa542a ("ring-buffer: Set new size of the ring buffer sub page")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Vincent Donnefort <vdonnefort@google.com>

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 2667992f0aa2..a3d28b2e2c94 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);
 
 	kfree(cpu_buffer);
 }
-- 
2.55.0.691.gc56d675ccc-goog

[PATCH v4 2/9] ring-buffer: Hold cpu_buffer::lock when resizing a subbuf

[Vincent Donnefort]

Because, ring_buffer_subbuf_order_set() can clear cpu_buffer->free_page,
hold cpu_buffer->lock to prevent races with
ring_buffer_alloc_read_page() and ring_buffer_free_read_page().

Fixes: 8e7b58c27b3c ("ring-buffer: Just update the subbuffers when changing their allocation order")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Vincent Donnefort <vdonnefort@google.com>

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index a3d28b2e2c94..ec4f5a0c93e8 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);
 
 		rb_head_page_activate(cpu_buffer);
 
-- 
2.55.0.691.gc56d675ccc-goog

[PATCH v4 3/9] ring-buffer: Fix subbuf resize race with ring buffer readers

[Vincent Donnefort]

trace_buffer subbuf_size is read lockless in ring_buffer_read_page() and
ring_buffer_read_start(), while it can simultaneously be resized with
ring_buffer_subbuf_order_set().

Instead of trace_buffer::subbuf_size, use bpage::order in
ring_buffer_read_start() and ring_buffer_read_page().

In ring_buffer_read_start(), even with resize_disabled, there is still a
possibility of a race with a buffer modification. Hold the trace_buffer
mutex to synchronise with any pending ring buffer order modification.

trace_buffer::subbuf_size is now actually useless, remove it. Also,
create accessors rb_subbuf_capacity() and rb_page_capacity() which
return the actual size available for storing events, while
rb_subbuf_size() returns the actual subbuf page-size.

Fixes: f9b94daa542a ("ring-buffer: Set new size of the ring buffer sub page")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Vincent Donnefort <vdonnefort@google.com>

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index ec4f5a0c93e8..97449423d3a6 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -391,6 +391,17 @@ static __always_inline unsigned int rb_page_size(struct buffer_page *bpage)
 	return rb_data_page_size(bpage->page);
 }
 
+/**
+ * rb_page_capacity - Get the capacity of a buffer page
+ * @bpage:	The buffer page
+ *
+ * Return: The maximum size available for events in the given buffer page.
+ */
+static __always_inline unsigned int rb_page_capacity(struct buffer_page *bpage)
+{
+	return (PAGE_SIZE << bpage->order) - BUF_PAGE_HDR_SIZE;
+}
+
 static void free_buffer_page(struct buffer_page *bpage)
 {
 	/* Range pages are not to be freed */
@@ -586,11 +597,42 @@ struct trace_buffer {
 
 	struct ring_buffer_meta		*meta;
 
-	unsigned int			subbuf_size;
 	unsigned int			subbuf_order;
 	unsigned int			max_data_size;
 };
 
+static __always_inline unsigned int rb_subbuf_size(struct trace_buffer *buffer)
+{
+	return PAGE_SIZE << buffer->subbuf_order;
+}
+
+/**
+ * rb_subbuf_capacity - Get the capacity of a subbuffer
+ * @buffer:	A trace buffer
+ *
+ * Unsafe to use without holding trace_buffer::mutex or with resizing enabled.
+ * Consider rb_page_capacity() instead.
+ *
+ * Return: The maximum size available for events in a trace buffer subbuffer.
+ */
+static __always_inline unsigned int rb_subbuf_capacity(struct trace_buffer *buffer)
+{
+	return rb_subbuf_size(buffer) - BUF_PAGE_HDR_SIZE;
+}
+
+/**
+ * rb_subbuf_start - Get the start address of a subbuffer
+ * @buffer:	A trace buffer
+ * @addr:	An address of an event on a subbuffer
+ *
+ * Return: The start of the subbuffer for where @addr sits
+ */
+static __always_inline
+unsigned long rb_subbuf_start(struct trace_buffer *buffer, unsigned long addr)
+{
+	return addr & ~((unsigned long)(rb_subbuf_size(buffer) - 1));
+}
+
 struct ring_buffer_iter {
 	struct ring_buffer_per_cpu	*cpu_buffer;
 	unsigned long			head;
@@ -630,7 +672,7 @@ int ring_buffer_print_page_header(struct trace_buffer *buffer, struct trace_seq
 	trace_seq_printf(s, "\tfield: char data;\t"
 			 "offset:%u;\tsize:%u;\tsigned:%u;\n",
 			 (unsigned int)offsetof(typeof(field), data),
-			 (unsigned int)(buffer ? buffer->subbuf_size :
+			 (unsigned int)(buffer ? rb_subbuf_capacity(buffer) :
 						 PAGE_SIZE - BUF_PAGE_HDR_SIZE),
 			 (unsigned int)is_signed_type(char));
 
@@ -1620,7 +1662,7 @@ rb_range_align_subbuf(unsigned long addr, int subbuf_size, int nr_subbufs)
  */
 static void *rb_range_meta(struct trace_buffer *buffer, int nr_pages, int cpu)
 {
-	int subbuf_size = buffer->subbuf_size + BUF_PAGE_HDR_SIZE;
+	int subbuf_size = rb_subbuf_size(buffer);
 	struct ring_buffer_cpu_meta *meta;
 	struct ring_buffer_meta *bmeta;
 	unsigned long ptr;
@@ -2432,8 +2474,8 @@ static int __rb_allocate_pages(struct ring_buffer_per_cpu *cpu_buffer,
 			bpage->id = i + 1;
 			cpu_buffer->subbuf_ids[i + 1] = bpage;
 		} else {
-			int order = cpu_buffer->buffer->subbuf_order;
-			bpage->page = alloc_cpu_data(cpu_buffer->cpu, order);
+			bpage->page = alloc_cpu_data(cpu_buffer->cpu,
+						     cpu_buffer->buffer->subbuf_order);
 			if (!bpage->page)
 				goto free_pages;
 		}
@@ -2556,8 +2598,7 @@ rb_allocate_cpu_buffer(struct trace_buffer *buffer, long nr_pages, int cpu)
 		bpage->range = 1;
 		cpu_buffer->subbuf_ids[0] = bpage;
 	} else {
-		int order = cpu_buffer->buffer->subbuf_order;
-		bpage->page = alloc_cpu_data(cpu, order);
+		bpage->page = alloc_cpu_data(cpu, bpage->order);
 		if (!bpage->page)
 			goto fail_free_reader;
 	}
@@ -2731,10 +2772,9 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
 
 	buffer->subbuf_order = order;
 	subbuf_size = (PAGE_SIZE << order);
-	buffer->subbuf_size = subbuf_size - BUF_PAGE_HDR_SIZE;
 
 	/* Max payload is buffer page size - header (8bytes) */
-	buffer->max_data_size = buffer->subbuf_size - (sizeof(u32) * 2);
+	buffer->max_data_size = rb_subbuf_capacity(buffer) - (sizeof(u32) * 2);
 
 	buffer->flags = flags;
 	buffer->clock = trace_clock_local;
@@ -2818,9 +2858,8 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
 		if (nr_pages < 2)
 			goto fail_free_buffers;
 	} else {
-
 		/* need at least two pages */
-		nr_pages = DIV_ROUND_UP(size, buffer->subbuf_size);
+		nr_pages = DIV_ROUND_UP(size, rb_subbuf_capacity(buffer));
 		if (nr_pages < 2)
 			nr_pages = 2;
 	}
@@ -3203,7 +3242,7 @@ static void update_pages_handler(struct work_struct *work)
  * @size: the new size.
  * @cpu_id: the cpu buffer to resize
  *
- * Minimum size is 2 * buffer->subbuf_size.
+ * Minimum size is 2 * rb_subbuf_capacity(buffer).
  *
  * Returns 0 on success and < 0 on failure.
  */
@@ -3225,12 +3264,6 @@ int ring_buffer_resize(struct trace_buffer *buffer, unsigned long size,
 	    !cpumask_test_cpu(cpu_id, buffer->cpumask))
 		return 0;
 
-	nr_pages = DIV_ROUND_UP(size, buffer->subbuf_size);
-
-	/* we need a minimum of two pages */
-	if (nr_pages < 2)
-		nr_pages = 2;
-
 	/*
 	 * Keep CPUs from coming online while resizing to synchronize
 	 * with new per CPU buffers being created.
@@ -3241,6 +3274,12 @@ int ring_buffer_resize(struct trace_buffer *buffer, unsigned long size,
 	mutex_lock(&buffer->mutex);
 	atomic_inc(&buffer->resizing);
 
+	nr_pages = DIV_ROUND_UP(size, rb_subbuf_capacity(buffer));
+
+	/* we need a minimum of two pages */
+	if (nr_pages < 2)
+		nr_pages = 2;
+
 	if (cpu_id == RING_BUFFER_ALL_CPUS) {
 		/*
 		 * Don't succeed if resizing is disabled, as a reader might be
@@ -3513,7 +3552,7 @@ rb_event_index(struct ring_buffer_per_cpu *cpu_buffer, struct ring_buffer_event
 {
 	unsigned long addr = (unsigned long)event;
 
-	addr &= (PAGE_SIZE << cpu_buffer->buffer->subbuf_order) - 1;
+	addr &= (unsigned long)rb_subbuf_size(cpu_buffer->buffer) - 1;
 
 	return addr - BUF_PAGE_HDR_SIZE;
 }
@@ -3755,8 +3794,8 @@ static inline void
 rb_reset_tail(struct ring_buffer_per_cpu *cpu_buffer,
 	      unsigned long tail, struct rb_event_info *info)
 {
-	unsigned long bsize = READ_ONCE(cpu_buffer->buffer->subbuf_size);
 	struct buffer_page *tail_page = info->tail_page;
+	unsigned long bsize = rb_page_capacity(tail_page);
 	struct ring_buffer_event *event;
 	unsigned long length = info->length;
 
@@ -4101,8 +4140,7 @@ rb_try_to_discard(struct ring_buffer_per_cpu *cpu_buffer,
 
 	new_index = rb_event_index(cpu_buffer, event);
 	old_index = new_index + rb_event_ts_length(event);
-	addr = (unsigned long)event;
-	addr &= ~((PAGE_SIZE << cpu_buffer->buffer->subbuf_order) - 1);
+	addr = rb_subbuf_start(cpu_buffer->buffer, (unsigned long)event);
 
 	bpage = READ_ONCE(cpu_buffer->tail_page);
 
@@ -4767,7 +4805,7 @@ __rb_reserve_next(struct ring_buffer_per_cpu *cpu_buffer,
 	tail = write - info->length;
 
 	/* See if we shot pass the end of this buffer page */
-	if (unlikely(write > cpu_buffer->buffer->subbuf_size)) {
+	if (unlikely(write > rb_page_capacity(tail_page))) {
 		check_buffer(cpu_buffer, info, CHECK_FULL_PAGE);
 		return rb_move_tail(cpu_buffer, tail, info);
 	}
@@ -5012,7 +5050,7 @@ rb_decrement_entry(struct ring_buffer_per_cpu *cpu_buffer,
 	struct buffer_page *bpage = cpu_buffer->commit_page;
 	struct buffer_page *start;
 
-	addr &= ~((PAGE_SIZE << cpu_buffer->buffer->subbuf_order) - 1);
+	addr = rb_subbuf_start(cpu_buffer->buffer, addr);
 
 	/* Do the likely case first */
 	if (likely(bpage->page == (void *)addr)) {
@@ -5799,7 +5837,6 @@ static struct buffer_page *
 __rb_get_reader_page(struct ring_buffer_per_cpu *cpu_buffer)
 {
 	int max_loops = cpu_buffer->ring_meta ? cpu_buffer->nr_pages : 3;
-	unsigned long bsize = READ_ONCE(cpu_buffer->buffer->subbuf_size);
 	struct buffer_page *reader = NULL;
 	unsigned long overwrite;
 	unsigned long flags;
@@ -5947,7 +5984,7 @@ __rb_get_reader_page(struct ring_buffer_per_cpu *cpu_buffer)
 #define USECS_WAIT	1000000
         for (nr_loops = 0; nr_loops < USECS_WAIT; nr_loops++) {
 		/* If the write is past the end of page, a writer is still updating it */
-		if (likely(!reader || rb_page_write(reader) <= bsize))
+		if (likely(!reader || rb_page_write(reader) <= rb_page_capacity(reader)))
 			break;
 
 		udelay(1);
@@ -6380,36 +6417,44 @@ EXPORT_SYMBOL_GPL(ring_buffer_consume);
 struct ring_buffer_iter *
 ring_buffer_read_start(struct trace_buffer *buffer, int cpu, gfp_t flags)
 {
+	struct ring_buffer_iter *iter __free(kfree) = kzalloc_obj(*iter, flags);
 	struct ring_buffer_per_cpu *cpu_buffer;
-	struct ring_buffer_iter *iter;
+
+	if (!iter)
+		return NULL;
 
 	if (!cpumask_test_cpu(cpu, buffer->cpumask))
 		return NULL;
 
-	iter = kzalloc_obj(*iter, flags);
-	if (!iter)
-		return NULL;
-
-	/* Holds the entire event: data and meta data */
-	iter->event_size = buffer->subbuf_size;
-	iter->event = kmalloc(iter->event_size, flags);
-	if (!iter->event) {
-		kfree(iter);
-		return NULL;
-	}
-
 	cpu_buffer = buffer->buffers[cpu];
 
-	iter->cpu_buffer = cpu_buffer;
+	/*
+	 * Only KDB is using GFP_ATOMIC, for the others, lock the buffer to
+	 * prevent concurrent resizing.
+	 */
+	if (gfpflags_allow_blocking(flags))
+		mutex_lock(&buffer->mutex);
 
 	atomic_inc(&cpu_buffer->resize_disabled);
 
+	if (gfpflags_allow_blocking(flags))
+		mutex_unlock(&buffer->mutex);
+
+	/* Holds the entire event: data and meta data. */
+	iter->event_size = rb_page_capacity(READ_ONCE(cpu_buffer->reader_page));
+	iter->event = kmalloc(iter->event_size, flags);
+	if (!iter->event) {
+		atomic_dec(&cpu_buffer->resize_disabled);
+		return NULL;
+	}
+	iter->cpu_buffer = cpu_buffer;
+
 	guard(raw_spinlock_irqsave)(&cpu_buffer->reader_lock);
 	arch_spin_lock(&cpu_buffer->lock);
 	rb_iter_reset(iter);
 	arch_spin_unlock(&cpu_buffer->lock);
 
-	return iter;
+	return_ptr(iter);
 }
 EXPORT_SYMBOL_GPL(ring_buffer_read_start);
 
@@ -6463,7 +6508,7 @@ unsigned long ring_buffer_size(struct trace_buffer *buffer, int cpu)
 	if (!cpumask_test_cpu(cpu, buffer->cpumask))
 		return 0;
 
-	return buffer->subbuf_size * buffer->buffers[cpu]->nr_pages;
+	return rb_subbuf_capacity(buffer) * buffer->buffers[cpu]->nr_pages;
 }
 EXPORT_SYMBOL_GPL(ring_buffer_size);
 
@@ -7094,15 +7139,15 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
 	if (!data_page || !data_page->data)
 		return -1;
 
-	if (data_page->order != buffer->subbuf_order)
-		return -1;
-
 	dpage = data_page->data;
 	if (!dpage)
 		return -1;
 
 	guard(raw_spinlock_irqsave)(&cpu_buffer->reader_lock);
 
+	if (data_page->order != cpu_buffer->reader_page->order)
+		return -1;
+
 	reader = rb_get_reader_page(cpu_buffer);
 	if (!reader)
 		return -1;
@@ -7228,7 +7273,7 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
 		 * missed events, then record it there.
 		 */
 		if (missed_events > 0 &&
-		    buffer->subbuf_size - size >= sizeof(missed_events)) {
+		    rb_page_capacity(reader) - size >= sizeof(missed_events)) {
 			memcpy(&dpage->data[size], &missed_events,
 			       sizeof(missed_events));
 			local_add(RB_MISSED_STORED, &dpage->commit);
@@ -7248,8 +7293,8 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
 	/*
 	 * This page may be off to user land. Zero it out here.
 	 */
-	if (size < buffer->subbuf_size)
-		memset(&dpage->data[size], 0, buffer->subbuf_size - size);
+	if (size < rb_page_capacity(reader))
+		memset(&dpage->data[size], 0, rb_page_capacity(reader) - size);
 
 	return read;
 }
@@ -7275,7 +7320,7 @@ EXPORT_SYMBOL_GPL(ring_buffer_read_page_data);
  */
 int ring_buffer_subbuf_size_get(struct trace_buffer *buffer)
 {
-	return buffer->subbuf_size + BUF_PAGE_HDR_SIZE;
+	return rb_subbuf_size(buffer);
 }
 EXPORT_SYMBOL_GPL(ring_buffer_subbuf_size_get);
 
@@ -7320,7 +7365,8 @@ int ring_buffer_subbuf_order_set(struct trace_buffer *buffer, int order)
 {
 	struct ring_buffer_per_cpu *cpu_buffer;
 	struct buffer_page *bpage, *tmp;
-	int old_order, old_size;
+	unsigned int old_capacity;
+	int old_order;
 	int nr_pages;
 	int psize;
 	int err;
@@ -7329,9 +7375,6 @@ int ring_buffer_subbuf_order_set(struct trace_buffer *buffer, int order)
 	if (!buffer || order < 0)
 		return -EINVAL;
 
-	if (buffer->subbuf_order == order)
-		return 0;
-
 	psize = (1 << order) * PAGE_SIZE;
 	if (psize <= BUF_PAGE_HDR_SIZE)
 		return -EINVAL;
@@ -7340,18 +7383,21 @@ int ring_buffer_subbuf_order_set(struct trace_buffer *buffer, int order)
 	if (psize > RB_WRITE_MASK + 1)
 		return -EINVAL;
 
-	old_order = buffer->subbuf_order;
-	old_size = buffer->subbuf_size;
-
 	/* prevent another thread from changing buffer sizes */
 	guard(mutex)(&buffer->mutex);
+
+	old_order = buffer->subbuf_order;
+	if (old_order == order)
+		return 0;
+
+	old_capacity = rb_subbuf_capacity(buffer);
+
 	atomic_inc(&buffer->record_disabled);
 
 	/* Make sure all commits have finished */
 	synchronize_rcu();
 
 	buffer->subbuf_order = order;
-	buffer->subbuf_size = psize - BUF_PAGE_HDR_SIZE;
 
 	/* Make sure all new buffers are allocated, before deleting the old ones */
 	for_each_buffer_cpu(buffer, cpu) {
@@ -7367,8 +7413,8 @@ int ring_buffer_subbuf_order_set(struct trace_buffer *buffer, int order)
 		}
 
 		/* Update the number of pages to match the new size */
-		nr_pages = old_size * buffer->buffers[cpu]->nr_pages;
-		nr_pages = DIV_ROUND_UP(nr_pages, buffer->subbuf_size);
+		nr_pages = old_capacity * buffer->buffers[cpu]->nr_pages;
+		nr_pages = DIV_ROUND_UP(nr_pages, rb_subbuf_capacity(buffer));
 
 		/* we need a minimum of two pages */
 		if (nr_pages < 2)
@@ -7456,7 +7502,6 @@ int ring_buffer_subbuf_order_set(struct trace_buffer *buffer, int order)
 
 error:
 	buffer->subbuf_order = old_order;
-	buffer->subbuf_size = old_size;
 
 	atomic_dec(&buffer->record_disabled);
 
@@ -7534,7 +7579,7 @@ static void rb_setup_ids_meta_page(struct ring_buffer_per_cpu *cpu_buffer,
 
 	meta->meta_struct_len = sizeof(*meta);
 	meta->nr_subbufs = nr_subbufs;
-	meta->subbuf_size = cpu_buffer->buffer->subbuf_size + BUF_PAGE_HDR_SIZE;
+	meta->subbuf_size = rb_subbuf_size(cpu_buffer->buffer);
 	meta->meta_page_size = meta->subbuf_size;
 
 	rb_update_meta_page(cpu_buffer);
@@ -7896,7 +7941,7 @@ int ring_buffer_map_get_reader(struct trace_buffer *buffer, int cpu)
 			 * missed events, then record it there.
 			 */
 			commit = rb_page_size(reader);
-			if (buffer->subbuf_size - commit >= sizeof(missed_events)) {
+			if (rb_subbuf_capacity(buffer) - commit >= sizeof(missed_events)) {
 				memcpy(&dpage->data[commit], &missed_events,
 				       sizeof(missed_events));
 				local_add(RB_MISSED_STORED, &dpage->commit);
@@ -7928,7 +7973,7 @@ int ring_buffer_map_get_reader(struct trace_buffer *buffer, int cpu)
 out:
 	/* Some archs do not have data cache coherency between kernel and user-space */
 	flush_kernel_vmap_range(cpu_buffer->reader_page->page,
-				buffer->subbuf_size + BUF_PAGE_HDR_SIZE);
+				rb_subbuf_size(buffer));
 
 	rb_update_meta_page(cpu_buffer);
 
-- 
2.55.0.691.gc56d675ccc-goog

[PATCH v4 4/9] ring-buffer: Fix subbuf resize race in ring_buffer_alloc_read_page()

[Vincent Donnefort]

ring_buffer_alloc_read_page() is racy with ring_buffer_subbuf_order_set,
it can allocate a reader page with an outdated order. This isn't a big
issue, the user can still re-allocate a new reader page and try again.

However, what is more problematic is if the value of subbuf_order
changes in the middle of ring_buffer_alloc_read_page(). In that case,
bpage->order might not match the actual allocated memory.

Use bpage->order for the allocation to prevent this race.

Fixes: bce761d75745 ("ring-buffer: Read and write to ring buffers with custom sub buffer size")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Vincent Donnefort <vdonnefort@google.com>

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 97449423d3a6..94552a433228 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -7018,7 +7018,7 @@ ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu)
 	if (bpage->data) {
 		rb_init_data_page(bpage->data);
 	} else {
-		bpage->data = alloc_cpu_data(cpu, cpu_buffer->buffer->subbuf_order);
+		bpage->data = alloc_cpu_data(cpu, bpage->order);
 		if (!bpage->data) {
 			kfree(bpage);
 			return ERR_PTR(-ENOMEM);
-- 
2.55.0.691.gc56d675ccc-goog

[PATCH v4 5/9] tracing: Fix subbuf resize races in trace_pipe_raw readers

[Vincent Donnefort]

Concurrent subbuffer resizes may crash trace_pipe_raw readers or leak
uninitialized memory to userspace due to stale size values.

Modify ring_buffer_alloc_read_page() to let it handle the resizing of
a previous buffer_data_read_page if necessary and add a new
ring_buffer_read_page_size() which enable ring-buffer users to not use
the racy ring_buffer_subbuf_size_get(). This makes the spare_size member
of ftrace_buffer_info redundant.

Use those functions in trace_pipe_raw readers and handle in both the
case where the subbuf order is modified in the middle of the read.

Fixes: bce761d75745 ("ring-buffer: Read and write to ring buffers with custom sub buffer size")
Signed-off-by: Vincent Donnefort <vdonnefort@google.com>

diff --git a/include/linux/ring_buffer.h b/include/linux/ring_buffer.h
index 0670742b2d60..fafbeb327037 100644
--- a/include/linux/ring_buffer.h
+++ b/include/linux/ring_buffer.h
@@ -219,13 +219,15 @@ size_t ring_buffer_nr_dirty_pages(struct trace_buffer *buffer, int cpu);
 
 struct buffer_data_read_page;
 struct buffer_data_read_page *
-ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu);
+ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu,
+			    struct buffer_data_read_page *prev);
 void ring_buffer_free_read_page(struct trace_buffer *buffer, int cpu,
 				struct buffer_data_read_page *page);
 int ring_buffer_read_page(struct trace_buffer *buffer,
 			  struct buffer_data_read_page *data_page,
 			  size_t len, int cpu, int full);
 void *ring_buffer_read_page_data(struct buffer_data_read_page *page);
+unsigned int ring_buffer_read_page_size(struct buffer_data_read_page *page);
 
 struct trace_seq;
 
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 94552a433228..f62d6853ee5c 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -6988,22 +6988,34 @@ EXPORT_SYMBOL_GPL(ring_buffer_swap_cpu);
  * Returns:
  *  The page allocated, or ERR_PTR
  */
-struct buffer_data_read_page *
-ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu)
+struct buffer_data_read_page *ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu,
+							  struct buffer_data_read_page *prev)
 {
+	struct buffer_data_read_page *bpage = prev;
 	struct ring_buffer_per_cpu *cpu_buffer;
-	struct buffer_data_read_page *bpage = NULL;
 	unsigned long flags;
+	unsigned int order;
 
 	if (!cpumask_test_cpu(cpu, buffer->cpumask))
 		return ERR_PTR(-ENODEV);
 
-	bpage = kzalloc_obj(*bpage);
-	if (!bpage)
-		return ERR_PTR(-ENOMEM);
-
-	bpage->order = buffer->subbuf_order;
+	order = buffer->subbuf_order;
 	cpu_buffer = buffer->buffers[cpu];
+
+	if (!bpage) {
+		bpage = kzalloc_obj(*bpage);
+		if (!bpage)
+			return ERR_PTR(-ENOMEM);
+	} else {
+		if (bpage->order == order)
+			return bpage;
+
+		free_pages((unsigned long)bpage->data, bpage->order);
+		bpage->data = NULL;
+	}
+
+	bpage->order = order;
+
 	local_irq_save(flags);
 	arch_spin_lock(&cpu_buffer->lock);
 
@@ -7020,7 +7032,9 @@ ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu)
 	} else {
 		bpage->data = alloc_cpu_data(cpu, bpage->order);
 		if (!bpage->data) {
-			kfree(bpage);
+			if (!prev)
+				kfree(bpage);
+
 			return ERR_PTR(-ENOMEM);
 		}
 	}
@@ -7041,13 +7055,22 @@ void ring_buffer_free_read_page(struct trace_buffer *buffer, int cpu,
 				struct buffer_data_read_page *data_page)
 {
 	struct ring_buffer_per_cpu *cpu_buffer;
-	struct buffer_data_page *dpage = data_page->data;
-	struct page *page = virt_to_page(dpage);
+	struct buffer_data_page *dpage;
 	unsigned long flags;
+	struct page *page;
 
 	if (!buffer || !buffer->buffers || !buffer->buffers[cpu])
 		return;
 
+	if (!data_page)
+		return;
+
+	dpage = data_page->data;
+	if (!dpage)
+		goto out;
+
+	page = virt_to_page(dpage);
+
 	cpu_buffer = buffer->buffers[cpu];
 
 	/*
@@ -7312,6 +7335,18 @@ void *ring_buffer_read_page_data(struct buffer_data_read_page *page)
 }
 EXPORT_SYMBOL_GPL(ring_buffer_read_page_data);
 
+/**
+ * ring_buffer_read_page_size - get size of the read page.
+ * @page:  the page to get the size from
+ *
+ * Returns size of the page in bytes.
+ */
+unsigned int ring_buffer_read_page_size(struct buffer_data_read_page *page)
+{
+	return PAGE_SIZE << page->order;
+}
+EXPORT_SYMBOL_GPL(ring_buffer_read_page_size);
+
 /**
  * ring_buffer_subbuf_size_get - get size of the sub buffer.
  * @buffer: the buffer to get the sub buffer size from
diff --git a/kernel/trace/ring_buffer_benchmark.c b/kernel/trace/ring_buffer_benchmark.c
index 593e3b59e42e..39b7dee21ed3 100644
--- a/kernel/trace/ring_buffer_benchmark.c
+++ b/kernel/trace/ring_buffer_benchmark.c
@@ -114,7 +114,7 @@ static enum event_status read_page(int cpu)
 	int inc;
 	int i;
 
-	bpage = ring_buffer_alloc_read_page(buffer, cpu);
+	bpage = ring_buffer_alloc_read_page(buffer, cpu, NULL);
 	if (IS_ERR(bpage))
 		return EVENT_DROPPED;
 
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 395238b2b715..0409d20a168b 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -7080,8 +7080,8 @@ ssize_t tracing_buffers_read(struct file *filp, char __user *ubuf,
 {
 	struct ftrace_buffer_info *info = filp->private_data;
 	struct trace_iterator *iter = &info->iter;
-	void *trace_data;
-	int page_size;
+	void *trace_data, *prev_spare;
+	unsigned int spare_size;
 	ssize_t ret = 0;
 	ssize_t size;
 
@@ -7091,36 +7091,31 @@ ssize_t tracing_buffers_read(struct file *filp, char __user *ubuf,
 	if (iter->snapshot && tracer_uses_snapshot(iter->tr->current_trace))
 		return -EBUSY;
 
-	page_size = ring_buffer_subbuf_size_get(iter->array_buffer->buffer);
+again:
+	prev_spare = info->spare;
+	if (prev_spare) {
+		spare_size = ring_buffer_read_page_size(info->spare);
 
-	/* Make sure the spare matches the current sub buffer size */
-	if (info->spare) {
-		if (page_size != info->spare_size) {
-			ring_buffer_free_read_page(iter->array_buffer->buffer,
-						   info->spare_cpu, info->spare);
-			info->spare = NULL;
-		}
+		/* Do we have previous read data to read? */
+		if (info->read < spare_size)
+			goto read;
 	}
 
-	if (!info->spare) {
-		info->spare = ring_buffer_alloc_read_page(iter->array_buffer->buffer,
-							  iter->cpu_file);
-		if (IS_ERR(info->spare)) {
-			ret = PTR_ERR(info->spare);
-			info->spare = NULL;
-		} else {
-			info->spare_cpu = iter->cpu_file;
-			info->spare_size = page_size;
-		}
-	}
-	if (!info->spare)
+	info->read = 0;
+
+	/* Make sure the read page order is aligned with the current buffer subbuf order */
+	info->spare = ring_buffer_alloc_read_page(iter->array_buffer->buffer, iter->cpu_file,
+						  prev_spare);
+	if (IS_ERR(info->spare)) {
+		ret = PTR_ERR(info->spare);
+		info->spare = NULL;
+		ring_buffer_free_read_page(iter->array_buffer->buffer, info->spare_cpu, prev_spare);
 		return ret;
+	}
 
-	/* Do we have previous read data to read? */
-	if (info->read < page_size)
-		goto read;
+	spare_size = ring_buffer_read_page_size(info->spare);
+	info->spare_cpu = iter->cpu_file;
 
- again:
 	trace_access_lock(iter->cpu_file);
 	ret = ring_buffer_read_page(iter->array_buffer->buffer,
 				    info->spare,
@@ -7129,6 +7124,10 @@ ssize_t tracing_buffers_read(struct file *filp, char __user *ubuf,
 	trace_access_unlock(iter->cpu_file);
 
 	if (ret < 0) {
+		/* Did we race with ring_buffer_subbuf_order_set ? */
+		if (spare_size != ring_buffer_subbuf_size_get(iter->array_buffer->buffer))
+			goto again;
+
 		if (trace_empty(iter) && !iter->closed) {
 			if (update_last_data_if_empty(iter->tr))
 				return 0;
@@ -7142,12 +7141,12 @@ ssize_t tracing_buffers_read(struct file *filp, char __user *ubuf,
 
 			goto again;
 		}
+
 		return 0;
 	}
 
-	info->read = 0;
  read:
-	size = page_size - info->read;
+	size = spare_size - info->read;
 	if (size > count)
 		size = count;
 	trace_data = ring_buffer_read_page_data(info->spare);
@@ -7268,23 +7267,12 @@ ssize_t tracing_buffers_splice_read(struct file *file, loff_t *ppos,
 	};
 	struct buffer_ref *ref;
 	bool woken = false;
-	int page_size;
 	int entries, i;
 	ssize_t ret = 0;
 
 	if (iter->snapshot && tracer_uses_snapshot(iter->tr->current_trace))
 		return -EBUSY;
 
-	page_size = ring_buffer_subbuf_size_get(iter->array_buffer->buffer);
-	if (*ppos & (page_size - 1))
-		return -EINVAL;
-
-	if (len & (page_size - 1)) {
-		if (len < page_size)
-			return -EINVAL;
-		len &= (~(page_size - 1));
-	}
-
 	if (splice_grow_spd(pipe, &spd))
 		return -ENOMEM;
 
@@ -7292,9 +7280,10 @@ ssize_t tracing_buffers_splice_read(struct file *file, loff_t *ppos,
 	trace_access_lock(iter->cpu_file);
 	entries = ring_buffer_entries_cpu(iter->array_buffer->buffer, iter->cpu_file);
 
-	for (i = 0; i < spd.nr_pages_max && len && entries; i++, len -= page_size) {
+	for (i = 0; i < spd.nr_pages_max && len && entries; i++) {
+		unsigned int page_size;
 		struct page *page;
-		int r;
+		int r = -EINVAL;
 
 		ref = kzalloc_obj(*ref);
 		if (!ref) {
@@ -7304,7 +7293,7 @@ ssize_t tracing_buffers_splice_read(struct file *file, loff_t *ppos,
 
 		refcount_set(&ref->refcount, 1);
 		ref->buffer = iter->array_buffer->buffer;
-		ref->page = ring_buffer_alloc_read_page(ref->buffer, iter->cpu_file);
+		ref->page = ring_buffer_alloc_read_page(ref->buffer, iter->cpu_file, NULL);
 		if (IS_ERR(ref->page)) {
 			ret = PTR_ERR(ref->page);
 			ref->page = NULL;
@@ -7313,11 +7302,21 @@ ssize_t tracing_buffers_splice_read(struct file *file, loff_t *ppos,
 		}
 		ref->cpu = iter->cpu_file;
 
-		r = ring_buffer_read_page(ref->buffer, ref->page,
-					  len, iter->cpu_file, 1);
+		page_size = ring_buffer_read_page_size(ref->page);
+
+		if (IS_ALIGNED(*ppos, page_size) && len >= page_size) {
+			r = ring_buffer_read_page(ref->buffer, ref->page, len, iter->cpu_file, 1);
+		} else if (!i) {
+			/*
+			 * If the first iteration fails this is an invalid userspace input.
+			 * Otherwise, this is because the subbuf order has been modified. Do not
+			 * report an error and finish the read.
+			 */
+			ret = -EINVAL;
+		}
+
 		if (r < 0) {
-			ring_buffer_free_read_page(ref->buffer, ref->cpu,
-						   ref->page);
+			ring_buffer_free_read_page(ref->buffer, ref->cpu, ref->page);
 			kfree(ref);
 			break;
 		}
@@ -7330,6 +7329,7 @@ ssize_t tracing_buffers_splice_read(struct file *file, loff_t *ppos,
 		spd.partial[i].private = (unsigned long)ref;
 		spd.nr_pages++;
 		*ppos += page_size;
+		len -= page_size;
 
 		entries = ring_buffer_entries_cpu(iter->array_buffer->buffer, iter->cpu_file);
 	}
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index bf77331f56a4..981e87b0f5a9 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -748,7 +748,6 @@ struct ftrace_buffer_info {
 	struct trace_iterator	iter;
 	void			*spare;
 	unsigned int		spare_cpu;
-	unsigned int		spare_size;
 	unsigned int		read;
 };
 
-- 
2.55.0.691.gc56d675ccc-goog

[PATCH v4 6/9] ring-buffer: Dynamically calculate max_data_size

[Vincent Donnefort]

The ring buffer order can be dynamically modified and temporarily
disables writing to do so. It is therefore safe to use the updated value
to calculate the maximum event size which can be written onto the ring
buffer.

However, notice it is hardly making any difference for trace_marker
because of the TRACE_MARKER_MAX_SIZE limit. For an 8KiB subbuf size,
trace_marker can take 4096 characters while it can 'only' take 4054
bytes for smaller subbufs.

Fixes: f9b94daa542a ("ring-buffer: Set new size of the ring buffer sub page")
Signed-off-by: Vincent Donnefort <vdonnefort@google.com>

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index f62d6853ee5c..64bf4ac853f5 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -598,7 +598,6 @@ struct trace_buffer {
 	struct ring_buffer_meta		*meta;
 
 	unsigned int			subbuf_order;
-	unsigned int			max_data_size;
 };
 
 static __always_inline unsigned int rb_subbuf_size(struct trace_buffer *buffer)
@@ -620,6 +619,23 @@ static __always_inline unsigned int rb_subbuf_capacity(struct trace_buffer *buff
 	return rb_subbuf_size(buffer) - BUF_PAGE_HDR_SIZE;
 }
 
+/**
+ * rb_subbuf_max_data_size - Get the maximum payload size of a single event
+ * @buffer:	A trace buffer
+ *
+ * Return: The maximum data payload size that can be stored in a single event.
+ */
+static __always_inline unsigned int rb_subbuf_max_data_size(struct trace_buffer *buffer)
+{
+	struct ring_buffer_event *event;
+
+	/*
+	 * surely rb_subbuf_capacity() is bigger than
+	 * RINGBUF_TYPE_DATA_TYPE_LEN_MAX (see ring_buffer_event_length).
+	 */
+	return rb_subbuf_capacity(buffer) - RB_EVNT_HDR_SIZE - sizeof(event->array[0]);
+}
+
 /**
  * rb_subbuf_start - Get the start address of a subbuffer
  * @buffer:	A trace buffer
@@ -2773,9 +2789,6 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
 	buffer->subbuf_order = order;
 	subbuf_size = (PAGE_SIZE << order);
 
-	/* Max payload is buffer page size - header (8bytes) */
-	buffer->max_data_size = rb_subbuf_capacity(buffer) - (sizeof(u32) * 2);
-
 	buffer->flags = flags;
 	buffer->clock = trace_clock_local;
 	buffer->reader_lock_key = key;
@@ -4941,7 +4954,7 @@ rb_reserve_next_event(struct trace_buffer *buffer,
 	if (ring_buffer_time_stamp_abs(cpu_buffer->buffer)) {
 		add_ts_default = RB_ADD_STAMP_ABSOLUTE;
 		info.length += RB_LEN_TIME_EXTEND;
-		if (info.length > cpu_buffer->buffer->max_data_size)
+		if (info.length > rb_subbuf_max_data_size(cpu_buffer->buffer))
 			goto out_fail;
 	} else {
 		add_ts_default = RB_ADD_STAMP_NONE;
@@ -5016,7 +5029,7 @@ ring_buffer_lock_reserve(struct trace_buffer *buffer, unsigned long length)
 	if (unlikely(atomic_read(&cpu_buffer->record_disabled)))
 		goto out;
 
-	if (unlikely(length > buffer->max_data_size))
+	if (unlikely(length > rb_subbuf_max_data_size(buffer)))
 		goto out;
 
 	if (unlikely(trace_recursive_lock(cpu_buffer)))
@@ -5163,7 +5176,7 @@ int ring_buffer_write(struct trace_buffer *buffer,
 	if (atomic_read(&cpu_buffer->record_disabled))
 		return -EBUSY;
 
-	if (length > buffer->max_data_size)
+	if (length > rb_subbuf_max_data_size(buffer))
 		return -EBUSY;
 
 	if (unlikely(trace_recursive_lock(cpu_buffer)))
@@ -6522,8 +6535,9 @@ unsigned long ring_buffer_max_event_size(struct trace_buffer *buffer)
 {
 	/* If abs timestamp is requested, events have a timestamp too */
 	if (ring_buffer_time_stamp_abs(buffer))
-		return buffer->max_data_size - RB_LEN_TIME_EXTEND;
-	return buffer->max_data_size;
+		return rb_subbuf_max_data_size(buffer) - RB_LEN_TIME_EXTEND;
+
+	return rb_subbuf_max_data_size(buffer);
 }
 EXPORT_SYMBOL_GPL(ring_buffer_max_event_size);
 
@@ -7976,7 +7990,7 @@ int ring_buffer_map_get_reader(struct trace_buffer *buffer, int cpu)
 			 * missed events, then record it there.
 			 */
 			commit = rb_page_size(reader);
-			if (rb_subbuf_capacity(buffer) - commit >= sizeof(missed_events)) {
+			if (rb_page_capacity(reader) - commit >= sizeof(missed_events)) {
 				memcpy(&dpage->data[commit], &missed_events,
 				       sizeof(missed_events));
 				local_add(RB_MISSED_STORED, &dpage->commit);
-- 
2.55.0.691.gc56d675ccc-goog

[PATCH v4 7/9] ring-buffer: Remove trace_buffer::cpus

[Vincent Donnefort]

The 'cpus' field in struct trace_buffer became useless in commit
8e7b58c27b3c ("ring-buffer: Just update the subbuffers when changing their
allocation order"). Remove it

Fixes: 8e7b58c27b3c ("ring-buffer: Just update the subbuffers when changing their allocation order")
Signed-off-by: Vincent Donnefort <vdonnefort@google.com>

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 64bf4ac853f5..45aa10f0b17e 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -572,7 +572,6 @@ struct ring_buffer_per_cpu {
 
 struct trace_buffer {
 	unsigned			flags;
-	int				cpus;
 	atomic_t			record_disabled;
 	atomic_t			resizing;
 	cpumask_var_t			cpumask;
@@ -2796,7 +2795,6 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
 	init_irq_work(&buffer->irq_work.work, rb_wake_up_waiters);
 	init_waitqueue_head(&buffer->irq_work.waiters);
 
-	buffer->cpus = nr_cpu_ids;
 
 	bsize = sizeof(void *) * nr_cpu_ids;
 	buffer->buffers = kzalloc(ALIGN(bsize, cache_line_size()),
-- 
2.55.0.691.gc56d675ccc-goog

[PATCH v4 8/9] ring-buffer: Remove ring_buffer_per_cpu::mapped

[Vincent Donnefort]

ring_buffer_per_cpu::mapped tracks if a ring-buffer is either mapped by
user-space or if it is a persistent buffer. We already have user_mapped
for the former and ring_meta for the latter. Get rid of mapped and
instead create rb_is_static(). A static ring-buffer cannot be resized,
swapped or have its pages extracted.

Signed-off-by: Vincent Donnefort <vdonnefort@google.com>

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 45aa10f0b17e..990a904cefe8 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -514,7 +514,7 @@ struct ring_buffer_per_cpu {
 	int				cpu;
 	atomic_t			record_disabled;
 	atomic_t			resize_disabled;
-	struct trace_buffer	*buffer;
+	struct trace_buffer		*buffer;
 	raw_spinlock_t			reader_lock;	/* serialize readers */
 	arch_spinlock_t			lock;
 	struct lock_class_key		lock_key;
@@ -552,7 +552,6 @@ struct ring_buffer_per_cpu {
 	/* pages removed since last reset */
 	unsigned long			pages_removed;
 
-	unsigned int			mapped;
 	unsigned int			user_mapped;	/* user space mapping */
 	struct mutex			mapping_lock;
 	struct buffer_page		**subbuf_ids;	/* ID to subbuf VA */
@@ -648,6 +647,11 @@ unsigned long rb_subbuf_start(struct trace_buffer *buffer, unsigned long addr)
 	return addr & ~((unsigned long)(rb_subbuf_size(buffer) - 1));
 }
 
+static bool rb_is_static(struct ring_buffer_per_cpu *cpu_buffer)
+{
+	return cpu_buffer->user_mapped || cpu_buffer->remote || cpu_buffer->ring_meta;
+}
+
 struct ring_buffer_iter {
 	struct ring_buffer_per_cpu	*cpu_buffer;
 	unsigned long			head;
@@ -2578,7 +2582,6 @@ rb_allocate_cpu_buffer(struct trace_buffer *buffer, long nr_pages, int cpu)
 		 * Range mapped buffers have the same restrictions as memory
 		 * mapped ones do.
 		 */
-		cpu_buffer->mapped = 1;
 		cpu_buffer->ring_meta = rb_range_meta(buffer, nr_pages, cpu);
 		bpage->page = rb_range_buffer(cpu_buffer, 0);
 		if (!bpage->page)
@@ -6667,12 +6670,11 @@ rb_reset_cpu(struct ring_buffer_per_cpu *cpu_buffer)
 	rb_head_page_activate(cpu_buffer);
 	cpu_buffer->pages_removed = 0;
 
-	if (cpu_buffer->mapped) {
-		rb_update_meta_page(cpu_buffer);
-		if (cpu_buffer->ring_meta) {
-			struct ring_buffer_cpu_meta *meta = cpu_buffer->ring_meta;
-			meta->commit_buffer = meta->head_buffer;
-		}
+	rb_update_meta_page(cpu_buffer);
+	if (cpu_buffer->ring_meta) {
+		struct ring_buffer_cpu_meta *meta = cpu_buffer->ring_meta;
+
+		meta->commit_buffer = meta->head_buffer;
 	}
 }
 
@@ -6921,8 +6923,8 @@ int ring_buffer_swap_cpu(struct trace_buffer *buffer_a,
 	cpu_buffer_a = buffer_a->buffers[cpu];
 	cpu_buffer_b = buffer_b->buffers[cpu];
 
-	/* It's up to the callers to not try to swap mapped buffers */
-	if (WARN_ON_ONCE(cpu_buffer_a->mapped || cpu_buffer_b->mapped))
+	/* It's up to the callers to not try to swap static buffers */
+	if (WARN_ON_ONCE(rb_is_static(cpu_buffer_a) || rb_is_static(cpu_buffer_b)))
 		return -EBUSY;
 
 	/* At least make sure the two buffers are somewhat the same */
@@ -7157,7 +7159,6 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
 	unsigned int size;
 	unsigned int read;
 	u64 save_timestamp;
-	bool force_memcpy;
 
 	if (!cpumask_test_cpu(cpu, buffer->cpumask))
 		return -1;
@@ -7196,8 +7197,6 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
 	/* Check if any events were dropped */
 	missed_events = cpu_buffer->lost_events;
 
-	force_memcpy = cpu_buffer->mapped || cpu_buffer->remote;
-
 	/*
 	 * If this page has been partially read or
 	 * if len is not big enough to read the rest of the page or
@@ -7207,7 +7206,7 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
 	 */
 	if (read || (len < (size - read)) ||
 	    cpu_buffer->reader_page == cpu_buffer->commit_page ||
-	    force_memcpy) {
+	    rb_is_static(cpu_buffer)) {
 		struct buffer_data_page *rpage = cpu_buffer->reader_page->page;
 		unsigned int rpos = read;
 		unsigned int pos = 0;
@@ -7668,11 +7667,7 @@ static int __rb_inc_dec_mapped(struct ring_buffer_per_cpu *cpu_buffer,
 
 	lockdep_assert_held(&cpu_buffer->mapping_lock);
 
-	/* mapped is always greater or equal to user_mapped */
-	if (WARN_ON(cpu_buffer->mapped < cpu_buffer->user_mapped))
-		return -EINVAL;
-
-	if (inc && cpu_buffer->mapped == UINT_MAX)
+	if (inc && cpu_buffer->user_mapped == UINT_MAX)
 		return -EBUSY;
 
 	if (WARN_ON(!inc && cpu_buffer->user_mapped == 0))
@@ -7681,13 +7676,10 @@ static int __rb_inc_dec_mapped(struct ring_buffer_per_cpu *cpu_buffer,
 	mutex_lock(&cpu_buffer->buffer->mutex);
 	raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags);
 
-	if (inc) {
+	if (inc)
 		cpu_buffer->user_mapped++;
-		cpu_buffer->mapped++;
-	} else {
+	else
 		cpu_buffer->user_mapped--;
-		cpu_buffer->mapped--;
-	}
 
 	raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);
 	mutex_unlock(&cpu_buffer->buffer->mutex);
@@ -7859,7 +7851,6 @@ int ring_buffer_map(struct trace_buffer *buffer, int cpu,
 	if (!err) {
 		raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags);
 		/* This is the first time it is mapped by user */
-		cpu_buffer->mapped++;
 		cpu_buffer->user_mapped = 1;
 		raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);
 	} else {
@@ -7916,8 +7907,6 @@ int ring_buffer_unmap(struct trace_buffer *buffer, int cpu)
 	raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags);
 
 	/* This is the last user space mapping */
-	if (!WARN_ON_ONCE(cpu_buffer->mapped < cpu_buffer->user_mapped))
-		cpu_buffer->mapped--;
 	cpu_buffer->user_mapped = 0;
 
 	raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);
-- 
2.55.0.691.gc56d675ccc-goog

[PATCH v4 9/9] ring-buffer: Make nr_pages unsigned int

[Vincent Donnefort]

nr_pages is an int or unsigned int almost everywhere already. Also, all
the meta-data ring_buffer_desc, ring_buffer_cpu_meta and
trace_buffer_meta allowing to share information about the ring buffer
are already capping this value to 32-bits.

Make ring_buffer_per_cpu::nr_pages unsigned and align all the users to
it. As a side effect, this makes ring_buffer_per_cpu slightly smaller.

Signed-off-by: Vincent Donnefort <vdonnefort@google.com>

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 990a904cefe8..2fccb950e593 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -519,7 +519,7 @@ struct ring_buffer_per_cpu {
 	arch_spinlock_t			lock;
 	struct lock_class_key		lock_key;
 	struct buffer_data_page		*free_page;
-	unsigned long			nr_pages;
+	unsigned int			nr_pages;
 	unsigned int			current_context;
 	struct list_head		*pages;
 	/* pages generation counter, incremented when the list changes */
@@ -561,7 +561,7 @@ struct ring_buffer_per_cpu {
 	struct ring_buffer_remote	*remote;
 
 	/* ring buffer pages to update, > 0 to add, < 0 to remove */
-	long				nr_pages_to_update;
+	int				nr_pages_to_update;
 	struct list_head		new_pages; /* new pages to add */
 	struct work_struct		update_pages_work;
 	struct completion		update_done;
@@ -1669,7 +1669,7 @@ static void rb_check_pages(struct ring_buffer_per_cpu *cpu_buffer)
  * This is used to help find the next per cpu subbuffer within a mapped range.
  */
 static unsigned long
-rb_range_align_subbuf(unsigned long addr, int subbuf_size, int nr_subbufs)
+rb_range_align_subbuf(unsigned long addr, int subbuf_size, unsigned int nr_subbufs)
 {
 	addr += sizeof(struct ring_buffer_cpu_meta) +
 		sizeof(int) * nr_subbufs;
@@ -1679,13 +1679,13 @@ rb_range_align_subbuf(unsigned long addr, int subbuf_size, int nr_subbufs)
 /*
  * Return the ring_buffer_meta for a given @cpu.
  */
-static void *rb_range_meta(struct trace_buffer *buffer, int nr_pages, int cpu)
+static void *rb_range_meta(struct trace_buffer *buffer, unsigned int nr_pages, int cpu)
 {
 	int subbuf_size = rb_subbuf_size(buffer);
 	struct ring_buffer_cpu_meta *meta;
 	struct ring_buffer_meta *bmeta;
+	unsigned int nr_subbufs;
 	unsigned long ptr;
-	int nr_subbufs;
 
 	bmeta = buffer->meta;
 	if (!bmeta)
@@ -1840,7 +1840,7 @@ static bool rb_meta_init(struct trace_buffer *buffer, int scratch_size)
  * must be the same.
  */
 static bool rb_cpu_meta_valid(struct ring_buffer_cpu_meta *meta, int cpu,
-			      struct trace_buffer *buffer, int nr_pages,
+			      struct trace_buffer *buffer, unsigned int nr_pages,
 			      unsigned long *subbuf_mask)
 {
 	int subbuf_size = PAGE_SIZE;
@@ -2231,7 +2231,7 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
 	}
 }
 
-static void rb_range_meta_init(struct trace_buffer *buffer, int nr_pages, int scratch_size)
+static void rb_range_meta_init(struct trace_buffer *buffer, unsigned int nr_pages, int scratch_size)
 {
 	struct ring_buffer_cpu_meta *meta;
 	unsigned long *subbuf_mask;
@@ -2331,8 +2331,8 @@ static int rbm_show(struct seq_file *m, void *v)
 			   rb_meta_subbuf_idx(meta, (void *)meta->head_buffer));
 		seq_printf(m, "commit_buffer: %d\n",
 			   rb_meta_subbuf_idx(meta, (void *)meta->commit_buffer));
-		seq_printf(m, "subbuf_size:   %d\n", meta->subbuf_size);
-		seq_printf(m, "nr_subbufs:    %d\n", meta->nr_subbufs);
+		seq_printf(m, "subbuf_size:   %u\n", meta->subbuf_size);
+		seq_printf(m, "nr_subbufs:    %u\n", meta->nr_subbufs);
 		return 0;
 	}
 
@@ -2417,7 +2417,7 @@ static void *ring_buffer_desc_page(struct ring_buffer_desc *desc, unsigned int p
 }
 
 static int __rb_allocate_pages(struct ring_buffer_per_cpu *cpu_buffer,
-		long nr_pages, struct list_head *pages)
+			       unsigned int nr_pages, struct list_head *pages)
 {
 	struct trace_buffer *buffer = cpu_buffer->buffer;
 	struct ring_buffer_cpu_meta *meta = NULL;
@@ -2520,7 +2520,7 @@ static int __rb_allocate_pages(struct ring_buffer_per_cpu *cpu_buffer,
 }
 
 static int rb_allocate_pages(struct ring_buffer_per_cpu *cpu_buffer,
-			     unsigned long nr_pages)
+			     unsigned int nr_pages)
 {
 	LIST_HEAD(pages);
 
@@ -2545,7 +2545,7 @@ static int rb_allocate_pages(struct ring_buffer_per_cpu *cpu_buffer,
 }
 
 static struct ring_buffer_per_cpu *
-rb_allocate_cpu_buffer(struct trace_buffer *buffer, long nr_pages, int cpu)
+rb_allocate_cpu_buffer(struct trace_buffer *buffer, unsigned int nr_pages, int cpu)
 {
 	struct ring_buffer_per_cpu *cpu_buffer __free(kfree) =
 		alloc_cpu_buffer(cpu);
@@ -2773,7 +2773,7 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
 					 struct ring_buffer_remote *remote)
 {
 	struct trace_buffer *buffer __free(kfree) = NULL;
-	long nr_pages;
+	unsigned int nr_pages;
 	int subbuf_size;
 	int bsize;
 	int cpu;
@@ -3037,12 +3037,12 @@ static inline unsigned long rb_page_write(struct buffer_page *bpage)
 }
 
 static bool
-rb_remove_pages(struct ring_buffer_per_cpu *cpu_buffer, unsigned long nr_pages)
+rb_remove_pages(struct ring_buffer_per_cpu *cpu_buffer, unsigned int nr_pages)
 {
 	struct list_head *tail_page, *to_remove, *next_page;
 	struct buffer_page *to_remove_page, *tmp_iter_page;
 	struct buffer_page *last_page, *first_page;
-	unsigned long nr_removed;
+	unsigned int nr_removed;
 	unsigned long head_bit;
 	int page_entries;
 
@@ -3264,7 +3264,7 @@ int ring_buffer_resize(struct trace_buffer *buffer, unsigned long size,
 			int cpu_id)
 {
 	struct ring_buffer_per_cpu *cpu_buffer;
-	unsigned long nr_pages;
+	unsigned int nr_pages;
 	int cpu, err;
 
 	/*
@@ -6522,7 +6522,7 @@ unsigned long ring_buffer_size(struct trace_buffer *buffer, int cpu)
 	if (!cpumask_test_cpu(cpu, buffer->cpumask))
 		return 0;
 
-	return rb_subbuf_capacity(buffer) * buffer->buffers[cpu]->nr_pages;
+	return (unsigned long)rb_subbuf_capacity(buffer) * buffer->buffers[cpu]->nr_pages;
 }
 EXPORT_SYMBOL_GPL(ring_buffer_size);
 
@@ -7412,8 +7412,8 @@ int ring_buffer_subbuf_order_set(struct trace_buffer *buffer, int order)
 	struct ring_buffer_per_cpu *cpu_buffer;
 	struct buffer_page *bpage, *tmp;
 	unsigned int old_capacity;
+	unsigned int nr_pages;
 	int old_order;
-	int nr_pages;
 	int psize;
 	int err;
 	int cpu;
@@ -7459,8 +7459,9 @@ int ring_buffer_subbuf_order_set(struct trace_buffer *buffer, int order)
 		}
 
 		/* Update the number of pages to match the new size */
-		nr_pages = old_capacity * buffer->buffers[cpu]->nr_pages;
-		nr_pages = DIV_ROUND_UP(nr_pages, rb_subbuf_capacity(buffer));
+		nr_pages = DIV_ROUND_UP(
+			(unsigned long)old_capacity * buffer->buffers[cpu]->nr_pages,
+			rb_subbuf_capacity(buffer));
 
 		/* we need a minimum of two pages */
 		if (nr_pages < 2)
@@ -7702,8 +7703,8 @@ static int __rb_inc_dec_mapped(struct ring_buffer_per_cpu *cpu_buffer,
 static int __rb_map_vma(struct ring_buffer_per_cpu *cpu_buffer,
 			struct vm_area_struct *vma)
 {
-	unsigned long nr_subbufs, nr_pages, nr_vma_pages, pgoff = vma->vm_pgoff;
-	unsigned int subbuf_pages, subbuf_order;
+	unsigned long nr_pages, nr_vma_pages, pgoff = vma->vm_pgoff;
+	unsigned int nr_subbufs, subbuf_pages, subbuf_order;
 	struct page **pages __free(kfree) = NULL;
 	int p = 0, s = 0;
 	int err;
@@ -8033,9 +8034,9 @@ static void rb_cpu_sync(void *data)
 int trace_rb_cpu_prepare(unsigned int cpu, struct hlist_node *node)
 {
 	struct trace_buffer *buffer;
-	long nr_pages_same;
+	unsigned int nr_pages;
+	int nr_pages_same;
 	int cpu_i;
-	unsigned long nr_pages;
 
 	buffer = container_of(node, struct trace_buffer, node);
 	if (cpumask_test_cpu(cpu, buffer->cpumask))
-- 
2.55.0.691.gc56d675ccc-goog

Re: [PATCH v4 4/9] ring-buffer: Fix subbuf resize race in ring_buffer_alloc_read_page()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch is an incomplete fix for a race condition in `ring_buffer_alloc_read_page()`. While it ensures newly allocated pages use `bpage->order`, it entirely misses the branch where `cpu_buffer->free_page` is reused. This allows an undersized cached page to be paired with a newer, larger `bpage->order`, leading to heap buffer overflows during reads and out-of-bounds frees when the page is released.
--

--- Patch [4]: [PATCH v4 4/9] ring-buffer: Fix subbuf resize race in ring_buffer_alloc_read_page() ---
commit 28754c82212e5979b25cf6fac815748ff021a277
Author: Vincent Donnefort <vdonnefort@google.com>

ring-buffer: Fix subbuf resize race in ring_buffer_alloc_read_page()

ring_buffer_alloc_read_page() is racy with ring_buffer_subbuf_order_set,
it can allocate a reader page with an outdated order. This isn't a big
issue, the user can still re-allocate a new reader page and try again.

However, what is more problematic is if the value of subbuf_order
changes in the middle of ring_buffer_alloc_read_page(). In that case,
bpage->order might not match the actual allocated memory.

Use bpage->order for the allocation to prevent this race.

> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 97449423d3a68..94552a433228d 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -7018,7 +7018,7 @@ ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu)
>  	if (bpage->data) {
>  		rb_init_data_page(bpage->data);

[Severity: High]
This is a pre-existing issue, but does this path leave bpage->order
mismatched with the actual allocation size of cpu_buffer->free_page?

If ring_buffer_subbuf_order_set() updates buffer->subbuf_order to a larger
size before clearing cpu_buffer->free_page, ring_buffer_alloc_read_page()
might read the new larger order into bpage->order, but then claim an older,
smaller cpu_buffer->free_page:

kernel/trace/ring_buffer.c:ring_buffer_alloc_read_page() {
    ...
	bpage->order = order;
    ...
	if (cpu_buffer->free_page) {
		bpage->data = cpu_buffer->free_page;
		cpu_buffer->free_page = NULL;
	}
    ...
}

Could this lead to an overflow of dpage->data when reading events?

kernel/trace/ring_buffer.c:ring_buffer_read_page() {
    ...
			memcpy(dpage->data + pos, rpage->data + rpos, event_size);
    ...
}

It also appears this could cause an out-of-bounds free of bpage->data when
the page is released, since it frees using the mismatched bpage->order
instead of the page's actual allocation order.

Should we also verify the cached page's actual order matches bpage->order
before claiming it?

>  	} else {
> -		bpage->data = alloc_cpu_data(cpu, cpu_buffer->buffer->subbuf_order);
> +		bpage->data = alloc_cpu_data(cpu, bpage->order);
>  		if (!bpage->data) {
>  			kfree(bpage);
>  			return ERR_PTR(-ENOMEM);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812153311.2328812-1-vdonnefort@google.com?part=4

Re: [PATCH v4 2/9] ring-buffer: Hold cpu_buffer::lock when resizing a subbuf

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The patch attempts to fix a race with `cpu_buffer->free_page` by adding an `arch_spin_lock(&cpu_buffer->lock)`, but it leaves a severe Time-Of-Check to Time-Of-Use (TOCTOU) race unaddressed. `buffer->subbuf_order` and `buffer->subbuf_size` are updated globally outside of any per-CPU lock. If a reader (like `tracing_buffers_splice_read`) calls `ring_buffer_alloc_read_page()` concurrently, it locklessly reads the NEW `buffer->subbuf_order` but then retrieves the OLD, incorrectly-sized `free_page` from the locked cache. When `ring_buffer_read_page()` uses this page, it copies data up to the NEW `buffer->subbuf_size`, leading to a heap buffer overflow if the new order is larger. Similarly, `ring_buffer_free_read_page()` validates `data_page->order` locklessly; if the global order changes just after the check, it inserts a new-order page into `free_page`, which `ring_buffer_subbuf_order_set()` will later free using the `old_order`, corrupting the page allocator.
- [High] The patch adds an unconditional `arch_spin_lock(&cpu_buffer->lock)` to `ring_buffer_subbuf_order_set()`. This creates an NMI deadlock window. If an NMI (e.g., from a watchdog or an Oops triggering `ftrace_dump_on_oops`) fires on the same CPU while this lock is held, the NMI handler may attempt to read the ring buffer. The read path (via `ring_buffer_peek()` -> `rb_buffer_peek()` -> `__rb_get_reader_page()`) unconditionally acquires the same `cpu_buffer->lock` using `arch_spin_lock()` without an NMI-safe trylock fallback. Because the interrupted context holds the lock, the NMI handler will spin forever, causing a hard lockup.
--

--- Patch [2]: [PATCH v4 2/9] ring-buffer: Hold cpu_buffer::lock when resizing a subbuf ---
commit 1db68f92346fd21f55d54529fb97b920f02a0136
Author: Vincent Donnefort <vdonnefort@google.com>

ring-buffer: Hold cpu_buffer::lock when resizing a subbuf

This commit adds an arch_spin_lock() around the clearing of
cpu_buffer->free_page in ring_buffer_subbuf_order_set(). This aims to
prevent races with ring_buffer_alloc_read_page() and
ring_buffer_free_read_page() during sub-buffer resizing.

> 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);

[Severity: Critical]
This is a pre-existing issue, but is there a Time-Of-Check to Time-Of-Use
(TOCTOU) race between the global order updates and this per-CPU lock?

Earlier in ring_buffer_subbuf_order_set(), buffer->subbuf_order and
buffer->subbuf_size are updated globally outside of any per-CPU lock.

If a reader like tracing_buffers_splice_read() calls
ring_buffer_alloc_read_page() concurrently, can it locklessly read the new
buffer->subbuf_order, but then retrieve the old, incorrectly-sized
free_page from the locked cache?

When ring_buffer_read_page() uses this page, wouldn't it copy data up to
the new buffer->subbuf_size, leading to a heap buffer overflow if the new
order is larger?

Similarly, ring_buffer_free_read_page() validates data_page->order against
buffer->subbuf_order locklessly. If the global order changes right after
the check, could it insert a new-order page into free_page, which
ring_buffer_subbuf_order_set() will later free using the old order,
corrupting the page allocator?

[Severity: High]
This is also a pre-existing issue, but does adding this unconditional
arch_spin_lock() widen an NMI deadlock window?

If an NMI (such as a watchdog or an Oops triggering ftrace_dump_on_oops)
fires on the same CPU while this lock is held, the NMI handler may attempt
to read the ring buffer.

The read path via __rb_get_reader_page() unconditionally acquires the same
cpu_buffer->lock using arch_spin_lock() without an NMI-safe trylock
fallback.

Because the interrupted context holds the lock, will the NMI handler spin
forever, causing a hard lockup?

>  		old_free_data_page = cpu_buffer->free_page;
>  		cpu_buffer->free_page = NULL;
> +		arch_spin_unlock(&cpu_buffer->lock);
>  
>  		rb_head_page_activate(cpu_buffer);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812153311.2328812-1-vdonnefort@google.com?part=2

Re: [PATCH v4 5/9] tracing: Fix subbuf resize races in trace_pipe_raw readers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] A logic error in the retry loop of `tracing_buffers_read()` bypasses `ring_buffer_read_page()` on retry, leaking uninitialized or stale kernel memory to userspace.

Pre-existing issues:
- [Critical] A race condition during subbuffer resize causes `cpu_buffer->free_page` to retain an old allocation size while `buffer->subbuf_order` reflects the new order, leading to a kernel buffer overflow.
--

--- Patch [5]: [PATCH v4 5/9] tracing: Fix subbuf resize races in trace_pipe_raw readers ---
commit c5c1a16fabb91c8f580283f14e5e76cc1ccb99b0
Author: Vincent Donnefort <vdonnefort@google.com>

tracing: Fix subbuf resize races in trace_pipe_raw readers

Concurrent subbuffer resizes may crash trace_pipe_raw readers or leak
uninitialized memory to userspace due to stale size values.

> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 94552a433228d..f62d6853ee5c0 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c

[ ... ]

> @@ -6988,22 +6988,34 @@ EXPORT_SYMBOL_GPL(ring_buffer_swap_cpu);
>   * Returns:
>   *  The page allocated, or ERR_PTR
>   */
> -struct buffer_data_read_page *
> -ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu)
> +struct buffer_data_read_page *ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu,
> +							  struct buffer_data_read_page *prev)
>  {
> +	struct buffer_data_read_page *bpage = prev;
>  	struct ring_buffer_per_cpu *cpu_buffer;
> -	struct buffer_data_read_page *bpage = NULL;
>  	unsigned long flags;
> +	unsigned int order;
>  
>  	if (!cpumask_test_cpu(cpu, buffer->cpumask))
>  		return ERR_PTR(-ENODEV);
>  
> -	bpage = kzalloc_obj(*bpage);
> -	if (!bpage)
> -		return ERR_PTR(-ENOMEM);
> -
> -	bpage->order = buffer->subbuf_order;
> +	order = buffer->subbuf_order;
>  	cpu_buffer = buffer->buffers[cpu];
> +
> +	if (!bpage) {
> +		bpage = kzalloc_obj(*bpage);
> +		if (!bpage)
> +			return ERR_PTR(-ENOMEM);
> +	} else {
> +		if (bpage->order == order)
> +			return bpage;
> +
> +		free_pages((unsigned long)bpage->data, bpage->order);
> +		bpage->data = NULL;
> +	}
> +
> +	bpage->order = order;
> +
>  	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;
>  	}

[Severity: Critical]
This is a pre-existing issue, but can this lead to an out-of-bounds write
during a subbuffer resize?

In ring_buffer_alloc_read_page(), the newly read buffer->subbuf_order is
assigned to a page stolen from cpu_buffer->free_page without checking if
their size orders match.

If a concurrent call to ring_buffer_subbuf_order_set() updates
buffer->subbuf_order locklessly, and this function is called before
cpu_buffer->free_page is cleared, it will steal the old free_page but assign
it the new size order.

Will this malformed page be swapped into the ring buffer, allowing writers
to write past its actual bounds?

[ ... ]

> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 395238b2b715c..0409d20a168bb 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c

[ ... ]

> @@ -7080,8 +7080,8 @@ ssize_t tracing_buffers_read(struct file *filp, char __user *ubuf,
>  {
>  	struct ftrace_buffer_info *info = filp->private_data;
>  	struct trace_iterator *iter = &info->iter;
> -	void *trace_data;
> -	int page_size;
> +	void *trace_data, *prev_spare;
> +	unsigned int spare_size;
>  	ssize_t ret = 0;
>  	ssize_t size;
>  
> @@ -7091,36 +7091,31 @@ ssize_t tracing_buffers_read(struct file *filp, char __user *ubuf,
>  	if (iter->snapshot && tracer_uses_snapshot(iter->tr->current_trace))
>  		return -EBUSY;
>  
> -	page_size = ring_buffer_subbuf_size_get(iter->array_buffer->buffer);
> +again:
> +	prev_spare = info->spare;
> +	if (prev_spare) {
> +		spare_size = ring_buffer_read_page_size(info->spare);
>  
> -	/* Make sure the spare matches the current sub buffer size */
> -	if (info->spare) {
> -		if (page_size != info->spare_size) {
> -			ring_buffer_free_read_page(iter->array_buffer->buffer,
> -						   info->spare_cpu, info->spare);
> -			info->spare = NULL;
> -		}
> +		/* Do we have previous read data to read? */
> +		if (info->read < spare_size)
> +			goto read;
>  	}

[Severity: High]
Can the retry path bypass the actual read function and copy stale data
to userspace?

When reading from an empty trace_pipe_raw file, ring_buffer_read_page()
returns < 0. The code then blocks in wait_on_pipe(). Once woken up, it
executes a goto again.

At the again label, info->read is 0 and info->spare is already allocated,
so the condition info->read < spare_size will evaluate to true.

Does this cause execution to jump directly to the read label, bypassing
ring_buffer_read_page(), and copy the uninitialized or stale info->spare
page to userspace?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812153311.2328812-1-vdonnefort@google.com?part=5

Re: [PATCH v4 9/9] ring-buffer: Make nr_pages unsigned int

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Integer truncation of `nr_pages_to_update` allows a massive buffer resize to bypass memory allocation checks, directly triggering an out-of-bounds page removal loop causing a critical kernel Use-After-Free (UAF).
- [High] Multiplication overflow in `subbuf_size * nr_pages` evaluates as an `unsigned int` when allocating huge persistent ring buffers, destroying trace data isolation.
--

--- Patch [9]: [PATCH v4 9/9] ring-buffer: Make nr_pages unsigned int ---
commit cb1365ebdbbed7daa3a685ae1d6be7a4167052dd
Author: Vincent Donnefort <vdonnefort@google.com>
Date:   Wed Aug 12 15:33:10 2026 +0000

    ring-buffer: Make nr_pages unsigned int
    
    nr_pages is an int or unsigned int almost everywhere already. Also, all
    the meta-data ring_buffer_desc, ring_buffer_cpu_meta and
    trace_buffer_meta allowing to share information about the ring buffer
    are already capping this value to 32-bits.
    
    Make ring_buffer_per_cpu::nr_pages unsigned and align all the users to
    it. As a side effect, this makes ring_buffer_per_cpu slightly smaller.

> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 990a904cefe85..2fccb950e593c 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
[ ... ]
> @@ -561,7 +561,7 @@ struct ring_buffer_per_cpu {
>  	struct ring_buffer_remote	*remote;
>  
>  	/* ring buffer pages to update, > 0 to add, < 0 to remove */
> -	long				nr_pages_to_update;
> +	int				nr_pages_to_update;
>  	struct list_head		new_pages; /* new pages to add */
>  	struct work_struct		update_pages_work;
>  	struct completion		update_done;

[Severity: Critical]
Could this type change lead to an out-of-bounds page removal regression?

In ring_buffer_resize(), size is an unsigned long and can be quite large,
resulting in an unsigned int nr_pages near the maximum 32-bit value. When
calculating nr_pages_to_update:

ring_buffer_resize() {
    ...
    cpu_buffer->nr_pages_to_update = nr_pages - cpu_buffer->nr_pages;
    if (cpu_buffer->nr_pages_to_update <= 0)
        continue;
    ...
}

If nr_pages is very large (e.g. 4294967286), subtracting the current nr_pages
(e.g. 10) results in an unsigned value that wraps into a small negative number
when stored in the 32-bit int nr_pages_to_update (e.g. -20).

This would bypass the allocation checks and trigger rb_remove_pages() to
remove more pages than exist in the buffer, causing it to loop past the end
of the list and free pages that are still in use.

[ ... ]
> @@ -2773,7 +2773,7 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
>  					 struct ring_buffer_remote *remote)
>  {
>  	struct trace_buffer *buffer __free(kfree) = NULL;
> -	long nr_pages;
> +	unsigned int nr_pages;
>  	int subbuf_size;
>  	int bsize;
>  	int cpu;

[Severity: High]
Will this type change cause a multiplication overflow when allocating large
persistent ring buffers?

Later in alloc_buffer(), the pointer is advanced for the next CPU's buffer:

alloc_buffer() {
    ...
    ptr += subbuf_size * nr_pages;
    ...
}

Because subbuf_size is an int and nr_pages is now an unsigned int, the
multiplication is evaluated as a 32-bit unsigned integer. 

If the buffer is larger than 4GB per CPU, this will overflow before being
added to ptr, placing the next CPU's buffer inside the previous CPU's buffer.

Previously, when nr_pages was a long, this math was safely performed as 64-bit
on 64-bit systems.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812153311.2328812-1-vdonnefort@google.com?part=9

Re: [PATCH v4 1/9] ring-buffer: Free cpu_buffer->free_page with subbuf_order

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Concurrent execution of sub-buffer order resizing (`ring_buffer_subbuf_order_set`) and read operations (`ring_buffer_alloc_read_page`, `ring_buffer_read_page`) can lead to out-of-bounds writes, use-after-free, and page allocator corruption due to mismatched synchronization.
--

--- Patch [1]: [PATCH v4 1/9] ring-buffer: Free cpu_buffer->free_page with subbuf_order ---
commit 4100674474d6bd293fb872fecb17fc781ca58502
Author: Vincent Donnefort <vdonnefort@google.com>

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 isn't a bug introduced by this patch, but is it possible for a race
condition between resizing and reading to lead to memory corruption?

If thread A changes the sub-buffer size via
/sys/kernel/tracing/buffer_subbuf_size_kb, ring_buffer_subbuf_order_set()
updates buffer->subbuf_order locklessly globally before tearing down the CPU
buffers and clearing their free_page.

If thread B concurrently reads /sys/kernel/tracing/per_cpu/cpu0/trace_pipe_raw
via tracing_buffers_read(), it observes the new order and calls
ring_buffer_alloc_read_page().

Because ring_buffer_subbuf_order_set() hasn't yet cleared
cpu_buffer->free_page for this CPU (they use distinct locks), thread B pops
the old page from free_page but tags it with the new order:

kernel/trace/ring_buffer.c:ring_buffer_alloc_read_page() {
    ...
    bpage->order = buffer->subbuf_order;
    ...
    if (cpu_buffer->free_page) {
        bpage->data = cpu_buffer->free_page;
        cpu_buffer->free_page = NULL;
    }
    ...
}

Thread B then calls ring_buffer_read_page(). The lockless check passes
because both match the new order:

kernel/trace/ring_buffer.c:ring_buffer_read_page() {
    ...
    if (data_page->order != buffer->subbuf_order)
        return -1;
    ...
    reader->page = data_page->data;
}

Could this allow the undersized old page to be swapped into the ring buffer
as the reader_page, leading to an out-of-bounds write when a writer attempts
to write large events? 

It seems it could also cause page allocator corruption when a smaller page
is later passed to free_pages() with a larger order.

>  	kfree(cpu_buffer);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812153311.2328812-1-vdonnefort@google.com?part=1

Re: [PATCH v4 3/9] ring-buffer: Fix subbuf resize race with ring buffer readers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Use-After-Free dereference of `reader_page` in `ring_buffer_read_start()` when bypassing buffer mutex.

Pre-existing issues:
- [High] 32-bit integer overflow in ring buffer capacity calculation truncates large trace buffers.
- [High] Out-of-bounds kernel memory write and VMAP flush in `ring_buffer_map_get_reader()` due to global capacity TOCTOU race.
--

--- Patch [3]: [PATCH v4 3/9] ring-buffer: Fix subbuf resize race with ring buffer readers ---
commit 9618f28d9a6939c645e7579fa798c42557eb075c
Author: Vincent Donnefort <vdonnefort@google.com>

ring-buffer: Fix subbuf resize race with ring buffer readers

This commit changes the trace_buffer to use bpage order instead of
trace_buffer::subbuf_size in ring_buffer_read_start() and
ring_buffer_read_page(). It also removes trace_buffer::subbuf_size and
introduces capacity accessors.

> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index ec4f5a0c93e85..97449423d3a68 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
[ ... ]
> @@ -6380,36 +6417,44 @@ EXPORT_SYMBOL_GPL(ring_buffer_consume);
>  struct ring_buffer_iter *
>  ring_buffer_read_start(struct trace_buffer *buffer, int cpu, gfp_t flags)
>  {
> +	struct ring_buffer_iter *iter __free(kfree) = kzalloc_obj(*iter, flags);
>  	struct ring_buffer_per_cpu *cpu_buffer;
> -	struct ring_buffer_iter *iter;
> -
> -	if (!cpumask_test_cpu(cpu, buffer->cpumask))
> -		return NULL;
>  
> -	iter = kzalloc_obj(*iter, flags);
>  	if (!iter)
>  		return NULL;
>  
> -	/* Holds the entire event: data and meta data */
> -	iter->event_size = buffer->subbuf_size;
> -	iter->event = kmalloc(iter->event_size, flags);
> -	if (!iter->event) {
> -		kfree(iter);
> +	if (!cpumask_test_cpu(cpu, buffer->cpumask))
>  		return NULL;
> -	}
>  
>  	cpu_buffer = buffer->buffers[cpu];
>  
> -	iter->cpu_buffer = cpu_buffer;
> +	/*
> +	 * Only KDB is using GFP_ATOMIC, for the others, lock the buffer to
> +	 * prevent concurrent resizing.
> +	 */
> +	if (gfpflags_allow_blocking(flags))
> +		mutex_lock(&buffer->mutex);
>  
>  	atomic_inc(&cpu_buffer->resize_disabled);
>  
> +	if (gfpflags_allow_blocking(flags))
> +		mutex_unlock(&buffer->mutex);
> +
> +	/* Holds the entire event: data and meta data. */
> +	iter->event_size = rb_page_capacity(READ_ONCE(cpu_buffer->reader_page));

[Severity: High]
Can this result in a use-after-free dereference of the reader page?

When ring_buffer_read_start() is called with GFP_ATOMIC, it skips acquiring
buffer->mutex and only increments cpu_buffer->resize_disabled.

If a concurrent ring_buffer_subbuf_order_set() has already passed its
resize_disabled check, it will proceed to allocate new pages and free the old
reader page.

A thread executing ring_buffer_read_start() could load the old reader page
pointer just before it is freed by the other CPU, dereferencing freed memory
when accessing the order field.

> +	iter->event = kmalloc(iter->event_size, flags);
> +	if (!iter->event) {
> +		atomic_dec(&cpu_buffer->resize_disabled);
> +		return NULL;
> +	}
[ ... ]
> @@ -7367,8 +7413,8 @@ int ring_buffer_subbuf_order_set(struct trace_buffer *buffer, int order)
>  		}
>  
>  		/* Update the number of pages to match the new size */
> -		nr_pages = old_size * buffer->buffers[cpu]->nr_pages;
> -		nr_pages = DIV_ROUND_UP(nr_pages, buffer->subbuf_size);
> +		nr_pages = old_capacity * buffer->buffers[cpu]->nr_pages;

[Severity: High]
This is a pre-existing issue, but can this 32-bit integer capacity calculation
truncate large trace buffers?

For buffers larger than 4GB, the multiplication of old_capacity and
buffer->buffers[cpu]->nr_pages can exceed 32 bits. Because nr_pages is a
32-bit signed integer, the result is truncated, and the buffer is shrunk to a
tiny fraction of its intended size during a resize operation.

> +		nr_pages = DIV_ROUND_UP(nr_pages, rb_subbuf_capacity(buffer));
>  
>  		/* we need a minimum of two pages */
>  		if (nr_pages < 2)
[ ... ]
> @@ -7896,7 +7941,7 @@ int ring_buffer_map_get_reader(struct trace_buffer *buffer, int cpu)
>  			 * missed events, then record it there.
>  			 */
>  			commit = rb_page_size(reader);
> -			if (buffer->subbuf_size - commit >= sizeof(missed_events)) {
> +			if (rb_subbuf_capacity(buffer) - commit >= sizeof(missed_events)) {

[Severity: High]
This is a pre-existing issue, but does this create a time-of-check to
time-of-use race leading to an out-of-bounds kernel memory write and
VMAP flush?

During a sub-buffer order resize, ring_buffer_subbuf_order_set() modifies
buffer->subbuf_order globally before verifying if resize_disabled is set. If
the buffer is currently mapped, the resize aborts and restores the old order.

In that race window, a concurrent ring_buffer_map_get_reader() call will see
the new, larger capacity via rb_subbuf_capacity(buffer) here. It could then
write missed_events past the end of the old, smaller reader page, and
subsequently flush memory beyond the mapped VMAP range.

>  				memcpy(&dpage->data[commit], &missed_events,
>  				       sizeof(missed_events));
>  				local_add(RB_MISSED_STORED, &dpage->commit);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812153311.2328812-1-vdonnefort@google.com?part=3

Re: [PATCH v4 6/9] ring-buffer: Dynamically calculate max_data_size

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Concurrent execution of ring_buffer_alloc_read_page() and ring_buffer_subbuf_order_set() leads to buddy allocator corruption and potential OOB memory access due to a mismatched page order when taking cpu_buffer->free_page.
--

--- Patch [6]: [PATCH v4 6/9] ring-buffer: Dynamically calculate max_data_size ---
commit a7794ac711440e6d89d8e099e4a88707ad083ed7
Author: Vincent Donnefort <vdonnefort@google.com>

ring-buffer: Dynamically calculate max_data_size

The ring buffer order can be dynamically modified and temporarily
disables writing to do so. It is therefore safe to use the updated value
to calculate the maximum event size which can be written onto the ring
buffer.

> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index f62d6853ee5c0..64bf4ac853f57 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -598,7 +598,6 @@ struct trace_buffer {
>  	struct ring_buffer_meta		*meta;
>  
>  	unsigned int			subbuf_order;
> -	unsigned int			max_data_size;
>  };
>  
>  static __always_inline unsigned int rb_subbuf_size(struct trace_buffer *buffer)

[ ... ]

[Severity: Critical]
This is a pre-existing issue, but can concurrent execution of
ring_buffer_alloc_read_page() and ring_buffer_subbuf_order_set() lead to buddy
allocator corruption and potential out-of-bounds memory access?

When a user reads /sys/kernel/tracing/per_cpu/cpuX/trace_pipe_raw, it calls
ring_buffer_alloc_read_page(). If another thread concurrently resizes the ring
buffer via /sys/kernel/tracing/buffer_subbuf_order, it calls
ring_buffer_subbuf_order_set().

In ring_buffer_subbuf_order_set(), buffer->subbuf_order is updated locklessly
with respect to the reader:

kernel/trace/ring_buffer.c:ring_buffer_subbuf_order_set() {
    ...
	buffer->subbuf_order = order;

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

Before ring_buffer_subbuf_order_set() reaches its second loop to clear
cpu_buffer->free_page, ring_buffer_alloc_read_page() can fetch the new
order, grab the old page from cpu_buffer->free_page, and assign the new
order to bpage->order:

kernel/trace/ring_buffer.c:ring_buffer_alloc_read_page() {
    ...
	if (cpu_buffer->free_page) {
		bpage->data = cpu_buffer->free_page;
		cpu_buffer->free_page = NULL;
	}
    ...
}

When this page is later freed or swapped into the ring buffer, could this
cause memory corruption because writers believe the subbuffer size is the
new order, leading them to overwrite past the old order page?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812153311.2328812-1-vdonnefort@google.com?part=6

Re: [PATCH v4 1/9] ring-buffer: Free cpu_buffer->free_page with subbuf_order

[Vincent Donnefort]

On Wed, Aug 12, 2026 at 03:50:37PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [Critical] Concurrent execution of sub-buffer order resizing (`ring_buffer_subbuf_order_set`) and read operations (`ring_buffer_alloc_read_page`, `ring_buffer_read_page`) can lead to out-of-bounds writes, use-after-free, and page allocator corruption due to mismatched synchronization.
> --
> 
> --- Patch [1]: [PATCH v4 1/9] ring-buffer: Free cpu_buffer->free_page with subbuf_order ---
> commit 4100674474d6bd293fb872fecb17fc781ca58502
> Author: Vincent Donnefort <vdonnefort@google.com>
> 
> 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 isn't a bug introduced by this patch, but is it possible for a race
> condition between resizing and reading to lead to memory corruption?
> 
> If thread A changes the sub-buffer size via
> /sys/kernel/tracing/buffer_subbuf_size_kb, ring_buffer_subbuf_order_set()
> updates buffer->subbuf_order locklessly globally before tearing down the CPU
> buffers and clearing their free_page.
> 
> If thread B concurrently reads /sys/kernel/tracing/per_cpu/cpu0/trace_pipe_raw
> via tracing_buffers_read(), it observes the new order and calls
> ring_buffer_alloc_read_page().
> 
> Because ring_buffer_subbuf_order_set() hasn't yet cleared
> cpu_buffer->free_page for this CPU (they use distinct locks), thread B pops
> the old page from free_page but tags it with the new order:
> 
> kernel/trace/ring_buffer.c:ring_buffer_alloc_read_page() {
>     ...
>     bpage->order = buffer->subbuf_order;
>     ...
>     if (cpu_buffer->free_page) {
>         bpage->data = cpu_buffer->free_page;
>         cpu_buffer->free_page = NULL;
>     }
>     ...
> }
> 
> Thread B then calls ring_buffer_read_page(). The lockless check passes
> because both match the new order:
> 
> kernel/trace/ring_buffer.c:ring_buffer_read_page() {
>     ...
>     if (data_page->order != buffer->subbuf_order)
>         return -1;
>     ...
>     reader->page = data_page->data;
> }
> 
> Could this allow the undersized old page to be swapped into the ring buffer
> as the reader_page, leading to an out-of-bounds write when a writer attempts
> to write large events? 
> 
> It seems it could also cause page allocator corruption when a smaller page
> is later passed to free_pages() with a larger order.
> 
> >  	kfree(cpu_buffer);
> >  }
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260812153311.2328812-1-vdonnefort@google.com?part=1

All that is hopefully fixed in one of the later patch of the series

-- 
Vincent

Re: [PATCH v4 2/9] ring-buffer: Hold cpu_buffer::lock when resizing a subbuf

[Vincent Donnefort]

On Wed, Aug 12, 2026 at 03:46:58PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [Critical] The patch attempts to fix a race with `cpu_buffer->free_page` by adding an `arch_spin_lock(&cpu_buffer->lock)`, but it leaves a severe Time-Of-Check to Time-Of-Use (TOCTOU) race unaddressed. `buffer->subbuf_order` and `buffer->subbuf_size` are updated globally outside of any per-CPU lock. If a reader (like `tracing_buffers_splice_read`) calls `ring_buffer_alloc_read_page()` concurrently, it locklessly reads the NEW `buffer->subbuf_order` but then retrieves the OLD, incorrectly-sized `free_page` from the locked cache. When `ring_buffer_read_page()` uses this page, it copies data up to the NEW `buffer->subbuf_size`, leading to a heap buffer overflow if the new order is larger. Similarly, `ring_buffer_free_read_page()` validates `data_page->order` locklessly; if the global order changes just after the check, it inserts a new-order page into `free_page`, which `ring_buffer_subbuf_order_set()` will later free using the `old_order`, corrupting the page allocator.
> - [High] The patch adds an unconditional `arch_spin_lock(&cpu_buffer->lock)` to `ring_buffer_subbuf_order_set()`. This creates an NMI deadlock window. If an NMI (e.g., from a watchdog or an Oops triggering `ftrace_dump_on_oops`) fires on the same CPU while this lock is held, the NMI handler may attempt to read the ring buffer. The read path (via `ring_buffer_peek()` -> `rb_buffer_peek()` -> `__rb_get_reader_page()`) unconditionally acquires the same `cpu_buffer->lock` using `arch_spin_lock()` without an NMI-safe trylock fallback. Because the interrupted context holds the lock, the NMI handler will spin forever, causing a hard lockup.
> --
> 
> --- Patch [2]: [PATCH v4 2/9] ring-buffer: Hold cpu_buffer::lock when resizing a subbuf ---
> commit 1db68f92346fd21f55d54529fb97b920f02a0136
> Author: Vincent Donnefort <vdonnefort@google.com>
> 
> ring-buffer: Hold cpu_buffer::lock when resizing a subbuf
> 
> This commit adds an arch_spin_lock() around the clearing of
> cpu_buffer->free_page in ring_buffer_subbuf_order_set(). This aims to
> prevent races with ring_buffer_alloc_read_page() and
> ring_buffer_free_read_page() during sub-buffer resizing.
> 
> > 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);
> 
> [Severity: Critical]
> This is a pre-existing issue, but is there a Time-Of-Check to Time-Of-Use
> (TOCTOU) race between the global order updates and this per-CPU lock?
> 
> Earlier in ring_buffer_subbuf_order_set(), buffer->subbuf_order and
> buffer->subbuf_size are updated globally outside of any per-CPU lock.
> 
> If a reader like tracing_buffers_splice_read() calls
> ring_buffer_alloc_read_page() concurrently, can it locklessly read the new
> buffer->subbuf_order, but then retrieve the old, incorrectly-sized
> free_page from the locked cache?
> 
> When ring_buffer_read_page() uses this page, wouldn't it copy data up to
> the new buffer->subbuf_size, leading to a heap buffer overflow if the new
> order is larger?
> 
> Similarly, ring_buffer_free_read_page() validates data_page->order against
> buffer->subbuf_order locklessly. If the global order changes right after
> the check, could it insert a new-order page into free_page, which
> ring_buffer_subbuf_order_set() will later free using the old order,
> corrupting the page allocator?

This is hopefully fixed in one of the following patch of this series.

> 
> [Severity: High]
> This is also a pre-existing issue, but does adding this unconditional
> arch_spin_lock() widen an NMI deadlock window?
> 
> If an NMI (such as a watchdog or an Oops triggering ftrace_dump_on_oops)
> fires on the same CPU while this lock is held, the NMI handler may attempt
> to read the ring buffer.
> 
> The read path via __rb_get_reader_page() unconditionally acquires the same
> cpu_buffer->lock using arch_spin_lock() without an NMI-safe trylock
> fallback.

This is under raw_spin_lock_irqsave(). I believe the panic path will first try
to issue normal IPIs.

-- 
Vincent

> 
> Because the interrupted context holds the lock, will the NMI handler spin
> forever, causing a hard lockup?
> 
> >  		old_free_data_page = cpu_buffer->free_page;
> >  		cpu_buffer->free_page = NULL;
> > +		arch_spin_unlock(&cpu_buffer->lock);
> >  
> >  		rb_head_page_activate(cpu_buffer);
> >
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260812153311.2328812-1-vdonnefort@google.com?part=2

Re: [PATCH v4 3/9] ring-buffer: Fix subbuf resize race with ring buffer readers

[Vincent Donnefort]

On Wed, Aug 12, 2026 at 03:53:12PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> 
> New issues:
> - [High] Use-After-Free dereference of `reader_page` in `ring_buffer_read_start()` when bypassing buffer mutex.
> 
> Pre-existing issues:
> - [High] 32-bit integer overflow in ring buffer capacity calculation truncates large trace buffers.
> - [High] Out-of-bounds kernel memory write and VMAP flush in `ring_buffer_map_get_reader()` due to global capacity TOCTOU race.
> --
> 
> --- Patch [3]: [PATCH v4 3/9] ring-buffer: Fix subbuf resize race with ring buffer readers ---
> commit 9618f28d9a6939c645e7579fa798c42557eb075c
> Author: Vincent Donnefort <vdonnefort@google.com>
> 
> ring-buffer: Fix subbuf resize race with ring buffer readers
> 
> This commit changes the trace_buffer to use bpage order instead of
> trace_buffer::subbuf_size in ring_buffer_read_start() and
> ring_buffer_read_page(). It also removes trace_buffer::subbuf_size and
> introduces capacity accessors.
> 
> > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> > index ec4f5a0c93e85..97449423d3a68 100644
> > --- a/kernel/trace/ring_buffer.c
> > +++ b/kernel/trace/ring_buffer.c
> [ ... ]
> > @@ -6380,36 +6417,44 @@ EXPORT_SYMBOL_GPL(ring_buffer_consume);
> >  struct ring_buffer_iter *
> >  ring_buffer_read_start(struct trace_buffer *buffer, int cpu, gfp_t flags)
> >  {
> > +	struct ring_buffer_iter *iter __free(kfree) = kzalloc_obj(*iter, flags);
> >  	struct ring_buffer_per_cpu *cpu_buffer;
> > -	struct ring_buffer_iter *iter;
> > -
> > -	if (!cpumask_test_cpu(cpu, buffer->cpumask))
> > -		return NULL;
> >  
> > -	iter = kzalloc_obj(*iter, flags);
> >  	if (!iter)
> >  		return NULL;
> >  
> > -	/* Holds the entire event: data and meta data */
> > -	iter->event_size = buffer->subbuf_size;
> > -	iter->event = kmalloc(iter->event_size, flags);
> > -	if (!iter->event) {
> > -		kfree(iter);
> > +	if (!cpumask_test_cpu(cpu, buffer->cpumask))
> >  		return NULL;
> > -	}
> >  
> >  	cpu_buffer = buffer->buffers[cpu];
> >  
> > -	iter->cpu_buffer = cpu_buffer;
> > +	/*
> > +	 * Only KDB is using GFP_ATOMIC, for the others, lock the buffer to
> > +	 * prevent concurrent resizing.
> > +	 */
> > +	if (gfpflags_allow_blocking(flags))
> > +		mutex_lock(&buffer->mutex);
> >  
> >  	atomic_inc(&cpu_buffer->resize_disabled);
> >  
> > +	if (gfpflags_allow_blocking(flags))
> > +		mutex_unlock(&buffer->mutex);
> > +
> > +	/* Holds the entire event: data and meta data. */
> > +	iter->event_size = rb_page_capacity(READ_ONCE(cpu_buffer->reader_page));
> 
> [Severity: High]
> Can this result in a use-after-free dereference of the reader page?
> 
> When ring_buffer_read_start() is called with GFP_ATOMIC, it skips acquiring
> buffer->mutex and only increments cpu_buffer->resize_disabled.
> 
> If a concurrent ring_buffer_subbuf_order_set() has already passed its
> resize_disabled check, it will proceed to allocate new pages and free the old
> reader page.
> 
> A thread executing ring_buffer_read_start() could load the old reader page
> pointer just before it is freed by the other CPU, dereferencing freed memory
> when accessing the order field.

Yep, as the comment says, we only bypass for KDB.

> 
> > +	iter->event = kmalloc(iter->event_size, flags);
> > +	if (!iter->event) {
> > +		atomic_dec(&cpu_buffer->resize_disabled);
> > +		return NULL;
> > +	}
> [ ... ]
> > @@ -7367,8 +7413,8 @@ int ring_buffer_subbuf_order_set(struct trace_buffer *buffer, int order)
> >  		}
> >  
> >  		/* Update the number of pages to match the new size */
> > -		nr_pages = old_size * buffer->buffers[cpu]->nr_pages;
> > -		nr_pages = DIV_ROUND_UP(nr_pages, buffer->subbuf_size);
> > +		nr_pages = old_capacity * buffer->buffers[cpu]->nr_pages;
> 
> [Severity: High]
> This is a pre-existing issue, but can this 32-bit integer capacity calculation
> truncate large trace buffers?
> 
> For buffers larger than 4GB, the multiplication of old_capacity and
> buffer->buffers[cpu]->nr_pages can exceed 32 bits. Because nr_pages is a
> 32-bit signed integer, the result is truncated, and the buffer is shrunk to a
> tiny fraction of its intended size during a resize operation.

This is fixed in a later patch.

> 
> > +		nr_pages = DIV_ROUND_UP(nr_pages, rb_subbuf_capacity(buffer));
> >  
> >  		/* we need a minimum of two pages */
> >  		if (nr_pages < 2)
> [ ... ]
> > @@ -7896,7 +7941,7 @@ int ring_buffer_map_get_reader(struct trace_buffer *buffer, int cpu)
> >  			 * missed events, then record it there.
> >  			 */
> >  			commit = rb_page_size(reader);
> > -			if (buffer->subbuf_size - commit >= sizeof(missed_events)) {
> > +			if (rb_subbuf_capacity(buffer) - commit >= sizeof(missed_events)) {
> 
> [Severity: High]
> This is a pre-existing issue, but does this create a time-of-check to
> time-of-use race leading to an out-of-bounds kernel memory write and
> VMAP flush?

This is fixed in a later patch.

> 
> During a sub-buffer order resize, ring_buffer_subbuf_order_set() modifies
> buffer->subbuf_order globally before verifying if resize_disabled is set. If
> the buffer is currently mapped, the resize aborts and restores the old order.
> 
> In that race window, a concurrent ring_buffer_map_get_reader() call will see
> the new, larger capacity via rb_subbuf_capacity(buffer) here. It could then
> write missed_events past the end of the old, smaller reader page, and
> subsequently flush memory beyond the mapped VMAP range.
> 
> >  				memcpy(&dpage->data[commit], &missed_events,
> >  				       sizeof(missed_events));
> >  				local_add(RB_MISSED_STORED, &dpage->commit);
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260812153311.2328812-1-vdonnefort@google.com?part=3

-- 
Vincent

Re: [PATCH v4 4/9] ring-buffer: Fix subbuf resize race in ring_buffer_alloc_read_page()

[Vincent Donnefort]

On Wed, Aug 12, 2026 at 03:46:53PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The patch is an incomplete fix for a race condition in `ring_buffer_alloc_read_page()`. While it ensures newly allocated pages use `bpage->order`, it entirely misses the branch where `cpu_buffer->free_page` is reused. This allows an undersized cached page to be paired with a newer, larger `bpage->order`, leading to heap buffer overflows during reads and out-of-bounds frees when the page is released.
> --
> 
> --- Patch [4]: [PATCH v4 4/9] ring-buffer: Fix subbuf resize race in ring_buffer_alloc_read_page() ---
> commit 28754c82212e5979b25cf6fac815748ff021a277
> Author: Vincent Donnefort <vdonnefort@google.com>
> 
> ring-buffer: Fix subbuf resize race in ring_buffer_alloc_read_page()
> 
> ring_buffer_alloc_read_page() is racy with ring_buffer_subbuf_order_set,
> it can allocate a reader page with an outdated order. This isn't a big
> issue, the user can still re-allocate a new reader page and try again.
> 
> However, what is more problematic is if the value of subbuf_order
> changes in the middle of ring_buffer_alloc_read_page(). In that case,
> bpage->order might not match the actual allocated memory.
> 
> Use bpage->order for the allocation to prevent this race.
> 
> > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> > index 97449423d3a68..94552a433228d 100644
> > --- a/kernel/trace/ring_buffer.c
> > +++ b/kernel/trace/ring_buffer.c
> > @@ -7018,7 +7018,7 @@ ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu)
> >  	if (bpage->data) {
> >  		rb_init_data_page(bpage->data);
> 
> [Severity: High]
> This is a pre-existing issue, but does this path leave bpage->order
> mismatched with the actual allocation size of cpu_buffer->free_page?
> 
> If ring_buffer_subbuf_order_set() updates buffer->subbuf_order to a larger
> size before clearing cpu_buffer->free_page, ring_buffer_alloc_read_page()
> might read the new larger order into bpage->order, but then claim an older,
> smaller cpu_buffer->free_page:

Not sure I follow here. the whole point is to read subbuf_order only once and
then make sure we always refer to bpage->order and not subbuf_order. 

I believe this is just fixed in the following patch.

> 
> kernel/trace/ring_buffer.c:ring_buffer_alloc_read_page() {
>     ...
> 	bpage->order = order;
>     ...
> 	if (cpu_buffer->free_page) {
> 		bpage->data = cpu_buffer->free_page;
> 		cpu_buffer->free_page = NULL;
> 	}
>     ...
> }
> 
> Could this lead to an overflow of dpage->data when reading events?
> 
> kernel/trace/ring_buffer.c:ring_buffer_read_page() {
>     ...
> 			memcpy(dpage->data + pos, rpage->data + rpos, event_size);
>     ...
> }
> 
> It also appears this could cause an out-of-bounds free of bpage->data when
> the page is released, since it frees using the mismatched bpage->order
> instead of the page's actual allocation order.
> 
> Should we also verify the cached page's actual order matches bpage->order
> before claiming it?
> 
> >  	} else {
> > -		bpage->data = alloc_cpu_data(cpu, cpu_buffer->buffer->subbuf_order);
> > +		bpage->data = alloc_cpu_data(cpu, bpage->order);
> >  		if (!bpage->data) {
> >  			kfree(bpage);
> >  			return ERR_PTR(-ENOMEM);
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260812153311.2328812-1-vdonnefort@google.com?part=4

-- 
Vincent

Re: [PATCH v4 5/9] tracing: Fix subbuf resize races in trace_pipe_raw readers

[Vincent Donnefort]

On Wed, Aug 12, 2026 at 03:47:05PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [High] A logic error in the retry loop of `tracing_buffers_read()` bypasses `ring_buffer_read_page()` on retry, leaking uninitialized or stale kernel memory to userspace.
> 
> Pre-existing issues:
> - [Critical] A race condition during subbuffer resize causes `cpu_buffer->free_page` to retain an old allocation size while `buffer->subbuf_order` reflects the new order, leading to a kernel buffer overflow.
> --
> 
> --- Patch [5]: [PATCH v4 5/9] tracing: Fix subbuf resize races in trace_pipe_raw readers ---
> commit c5c1a16fabb91c8f580283f14e5e76cc1ccb99b0
> Author: Vincent Donnefort <vdonnefort@google.com>
> 
> tracing: Fix subbuf resize races in trace_pipe_raw readers
> 
> Concurrent subbuffer resizes may crash trace_pipe_raw readers or leak
> uninitialized memory to userspace due to stale size values.
> 
> > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> > index 94552a433228d..f62d6853ee5c0 100644
> > --- a/kernel/trace/ring_buffer.c
> > +++ b/kernel/trace/ring_buffer.c
> 
> [ ... ]
> 
> > @@ -6988,22 +6988,34 @@ EXPORT_SYMBOL_GPL(ring_buffer_swap_cpu);
> >   * Returns:
> >   *  The page allocated, or ERR_PTR
> >   */
> > -struct buffer_data_read_page *
> > -ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu)
> > +struct buffer_data_read_page *ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu,
> > +							  struct buffer_data_read_page *prev)
> >  {
> > +	struct buffer_data_read_page *bpage = prev;
> >  	struct ring_buffer_per_cpu *cpu_buffer;
> > -	struct buffer_data_read_page *bpage = NULL;
> >  	unsigned long flags;
> > +	unsigned int order;
> >  
> >  	if (!cpumask_test_cpu(cpu, buffer->cpumask))
> >  		return ERR_PTR(-ENODEV);
> >  
> > -	bpage = kzalloc_obj(*bpage);
> > -	if (!bpage)
> > -		return ERR_PTR(-ENOMEM);
> > -
> > -	bpage->order = buffer->subbuf_order;
> > +	order = buffer->subbuf_order;
> >  	cpu_buffer = buffer->buffers[cpu];
> > +
> > +	if (!bpage) {
> > +		bpage = kzalloc_obj(*bpage);
> > +		if (!bpage)
> > +			return ERR_PTR(-ENOMEM);
> > +	} else {
> > +		if (bpage->order == order)
> > +			return bpage;
> > +
> > +		free_pages((unsigned long)bpage->data, bpage->order);
> > +		bpage->data = NULL;
> > +	}
> > +
> > +	bpage->order = order;
> > +
> >  	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;
> >  	}
> 
> [Severity: Critical]
> This is a pre-existing issue, but can this lead to an out-of-bounds write
> during a subbuffer resize?
> 
> In ring_buffer_alloc_read_page(), the newly read buffer->subbuf_order is
> assigned to a page stolen from cpu_buffer->free_page without checking if
> their size orders match.
> 
> If a concurrent call to ring_buffer_subbuf_order_set() updates
> buffer->subbuf_order locklessly, and this function is called before
> cpu_buffer->free_page is cleared, it will steal the old free_page but assign
> it the new size order.
> 
> Will this malformed page be swapped into the ring buffer, allowing writers
> to write past its actual bounds?
> 
> [ ... ]
> 
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index 395238b2b715c..0409d20a168bb 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> 
> [ ... ]
> 
> > @@ -7080,8 +7080,8 @@ ssize_t tracing_buffers_read(struct file *filp, char __user *ubuf,
> >  {
> >  	struct ftrace_buffer_info *info = filp->private_data;
> >  	struct trace_iterator *iter = &info->iter;
> > -	void *trace_data;
> > -	int page_size;
> > +	void *trace_data, *prev_spare;
> > +	unsigned int spare_size;
> >  	ssize_t ret = 0;
> >  	ssize_t size;
> >  
> > @@ -7091,36 +7091,31 @@ ssize_t tracing_buffers_read(struct file *filp, char __user *ubuf,
> >  	if (iter->snapshot && tracer_uses_snapshot(iter->tr->current_trace))
> >  		return -EBUSY;
> >  
> > -	page_size = ring_buffer_subbuf_size_get(iter->array_buffer->buffer);
> > +again:
> > +	prev_spare = info->spare;
> > +	if (prev_spare) {
> > +		spare_size = ring_buffer_read_page_size(info->spare);
> >  
> > -	/* Make sure the spare matches the current sub buffer size */
> > -	if (info->spare) {
> > -		if (page_size != info->spare_size) {
> > -			ring_buffer_free_read_page(iter->array_buffer->buffer,
> > -						   info->spare_cpu, info->spare);
> > -			info->spare = NULL;
> > -		}
> > +		/* Do we have previous read data to read? */
> > +		if (info->read < spare_size)
> > +			goto read;
> >  	}
> 
> [Severity: High]
> Can the retry path bypass the actual read function and copy stale data
> to userspace?
> 
> When reading from an empty trace_pipe_raw file, ring_buffer_read_page()
> returns < 0. The code then blocks in wait_on_pipe(). Once woken up, it
> executes a goto again.
> 
> At the again label, info->read is 0 and info->spare is already allocated,
> so the condition info->read < spare_size will evaluate to true.
> 
> Does this cause execution to jump directly to the read label, bypassing
> ring_buffer_read_page(), and copy the uninitialized or stale info->spare
> page to userspace?

Arg, this one is real!

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260812153311.2328812-1-vdonnefort@google.com?part=5

-- 
Vincent

Re: [PATCH v4 6/9] ring-buffer: Dynamically calculate max_data_size

[Vincent Donnefort]

On Wed, Aug 12, 2026 at 03:56:42PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [Critical] Concurrent execution of ring_buffer_alloc_read_page() and ring_buffer_subbuf_order_set() leads to buddy allocator corruption and potential OOB memory access due to a mismatched page order when taking cpu_buffer->free_page.
> --
> 
> --- Patch [6]: [PATCH v4 6/9] ring-buffer: Dynamically calculate max_data_size ---
> commit a7794ac711440e6d89d8e099e4a88707ad083ed7
> Author: Vincent Donnefort <vdonnefort@google.com>
> 
> ring-buffer: Dynamically calculate max_data_size
> 
> The ring buffer order can be dynamically modified and temporarily
> disables writing to do so. It is therefore safe to use the updated value
> to calculate the maximum event size which can be written onto the ring
> buffer.
> 
> > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> > index f62d6853ee5c0..64bf4ac853f57 100644
> > --- a/kernel/trace/ring_buffer.c
> > +++ b/kernel/trace/ring_buffer.c
> > @@ -598,7 +598,6 @@ struct trace_buffer {
> >  	struct ring_buffer_meta		*meta;
> >  
> >  	unsigned int			subbuf_order;
> > -	unsigned int			max_data_size;
> >  };
> >  
> >  static __always_inline unsigned int rb_subbuf_size(struct trace_buffer *buffer)
> 
> [ ... ]
> 
> [Severity: Critical]
> This is a pre-existing issue, but can concurrent execution of
> ring_buffer_alloc_read_page() and ring_buffer_subbuf_order_set() lead to buddy
> allocator corruption and potential out-of-bounds memory access?
> 
> When a user reads /sys/kernel/tracing/per_cpu/cpuX/trace_pipe_raw, it calls
> ring_buffer_alloc_read_page(). If another thread concurrently resizes the ring
> buffer via /sys/kernel/tracing/buffer_subbuf_order, it calls
> ring_buffer_subbuf_order_set().
> 
> In ring_buffer_subbuf_order_set(), buffer->subbuf_order is updated locklessly
> with respect to the reader:
> 
> kernel/trace/ring_buffer.c:ring_buffer_subbuf_order_set() {
>     ...
> 	buffer->subbuf_order = order;
> 
> 	/* Make sure all new buffers are allocated, before deleting the old ones */
> 	for_each_buffer_cpu(buffer, cpu) {
>     ...
> }
> 
> Before ring_buffer_subbuf_order_set() reaches its second loop to clear
> cpu_buffer->free_page, ring_buffer_alloc_read_page() can fetch the new
> order, grab the old page from cpu_buffer->free_page, and assign the new
> order to bpage->order:
> 
> kernel/trace/ring_buffer.c:ring_buffer_alloc_read_page() {
>     ...
> 	if (cpu_buffer->free_page) {
> 		bpage->data = cpu_buffer->free_page;
> 		cpu_buffer->free_page = NULL;
> 	}
>     ...
> }
> 
> When this page is later freed or swapped into the ring buffer, could this
> cause memory corruption because writers believe the subbuffer size is the
> new order, leading them to overwrite past the old order page?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260812153311.2328812-1-vdonnefort@google.com?part=6

Ha this is something I was meant to fix as well... we can't just blindly take
cpu_buffer::free_page! 

I think the best is to make cpu_buffer->free_page a buffer_data_read_page so we
have an order associated to it!

-- 
Vincent

Re: [PATCH v4 9/9] ring-buffer: Make nr_pages unsigned int

[Vincent Donnefort]

On Wed, Aug 12, 2026 at 03:47:11PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Critical] Integer truncation of `nr_pages_to_update` allows a massive buffer resize to bypass memory allocation checks, directly triggering an out-of-bounds page removal loop causing a critical kernel Use-After-Free (UAF).
> - [High] Multiplication overflow in `subbuf_size * nr_pages` evaluates as an `unsigned int` when allocating huge persistent ring buffers, destroying trace data isolation.
> --
> 
> --- Patch [9]: [PATCH v4 9/9] ring-buffer: Make nr_pages unsigned int ---
> commit cb1365ebdbbed7daa3a685ae1d6be7a4167052dd
> Author: Vincent Donnefort <vdonnefort@google.com>
> Date:   Wed Aug 12 15:33:10 2026 +0000
> 
>     ring-buffer: Make nr_pages unsigned int
>     
>     nr_pages is an int or unsigned int almost everywhere already. Also, all
>     the meta-data ring_buffer_desc, ring_buffer_cpu_meta and
>     trace_buffer_meta allowing to share information about the ring buffer
>     are already capping this value to 32-bits.
>     
>     Make ring_buffer_per_cpu::nr_pages unsigned and align all the users to
>     it. As a side effect, this makes ring_buffer_per_cpu slightly smaller.
> 
> > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> > index 990a904cefe85..2fccb950e593c 100644
> > --- a/kernel/trace/ring_buffer.c
> > +++ b/kernel/trace/ring_buffer.c
> [ ... ]
> > @@ -561,7 +561,7 @@ struct ring_buffer_per_cpu {
> >  	struct ring_buffer_remote	*remote;
> >  
> >  	/* ring buffer pages to update, > 0 to add, < 0 to remove */
> > -	long				nr_pages_to_update;
> > +	int				nr_pages_to_update;
> >  	struct list_head		new_pages; /* new pages to add */
> >  	struct work_struct		update_pages_work;
> >  	struct completion		update_done;
> 
> [Severity: Critical]
> Could this type change lead to an out-of-bounds page removal regression?
> 
> In ring_buffer_resize(), size is an unsigned long and can be quite large,
> resulting in an unsigned int nr_pages near the maximum 32-bit value. When
> calculating nr_pages_to_update:
> 
> ring_buffer_resize() {
>     ...
>     cpu_buffer->nr_pages_to_update = nr_pages - cpu_buffer->nr_pages;
>     if (cpu_buffer->nr_pages_to_update <= 0)
>         continue;
>     ...
> }
> 
> If nr_pages is very large (e.g. 4294967286), subtracting the current nr_pages
> (e.g. 10) results in an unsigned value that wraps into a small negative number
> when stored in the 32-bit int nr_pages_to_update (e.g. -20).
> 
> This would bypass the allocation checks and trigger rb_remove_pages() to
> remove more pages than exist in the buffer, causing it to loop past the end
> of the list and free pages that are still in use.

That would mean 15 TiB for a single CPU right?

> 
> [ ... ]
> > @@ -2773,7 +2773,7 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
> >  					 struct ring_buffer_remote *remote)
> >  {
> >  	struct trace_buffer *buffer __free(kfree) = NULL;
> > -	long nr_pages;
> > +	unsigned int nr_pages;
> >  	int subbuf_size;
> >  	int bsize;
> >  	int cpu;
> 
> [Severity: High]
> Will this type change cause a multiplication overflow when allocating large
> persistent ring buffers?
> 
> Later in alloc_buffer(), the pointer is advanced for the next CPU's buffer:
> 
> alloc_buffer() {
>     ...
>     ptr += subbuf_size * nr_pages;
>     ...
> }
> 
> Because subbuf_size is an int and nr_pages is now an unsigned int, the
> multiplication is evaluated as a 32-bit unsigned integer. 
> 
> If the buffer is larger than 4GB per CPU, this will overflow before being
> added to ptr, placing the next CPU's buffer inside the previous CPU's buffer.
> 
> Previously, when nr_pages was a long, this math was safely performed as 64-bit
> on 64-bit systems.

This one is worth improving though!

-- 
Vincent

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260812153311.2328812-1-vdonnefort@google.com?part=9

Re: [PATCH v4 3/9] ring-buffer: Fix subbuf resize race with ring buffer readers

[Steven Rostedt]

Hi Roman,

On Wed, 12 Aug 2026 17:44:41 +0100
Vincent Donnefort <vdonnefort@google.com> wrote:

> > [Severity: High]
> > This is a pre-existing issue, but can this 32-bit integer capacity calculation
> > truncate large trace buffers?
> > 
> > For buffers larger than 4GB, the multiplication of old_capacity and
> > buffer->buffers[cpu]->nr_pages can exceed 32 bits. Because nr_pages is a
> > 32-bit signed integer, the result is truncated, and the buffer is shrunk to a
> > tiny fraction of its intended size during a resize operation.  
> 
> This is fixed in a later patch.
> 
> >   
> > > +		nr_pages = DIV_ROUND_UP(nr_pages, rb_subbuf_capacity(buffer));
> > >  
> > >  		/* we need a minimum of two pages */
> > >  		if (nr_pages < 2)  
> > [ ... ]  
> > > @@ -7896,7 +7941,7 @@ int ring_buffer_map_get_reader(struct trace_buffer *buffer, int cpu)
> > >  			 * missed events, then record it there.
> > >  			 */
> > >  			commit = rb_page_size(reader);
> > > -			if (buffer->subbuf_size - commit >= sizeof(missed_events)) {
> > > +			if (rb_subbuf_capacity(buffer) - commit >= sizeof(missed_events)) {  
> > 
> > [Severity: High]
> > This is a pre-existing issue, but does this create a time-of-check to
> > time-of-use race leading to an out-of-bounds kernel memory write and
> > VMAP flush?  
> 
> This is fixed in a later patch.
> 

Is it possible to have Sashiko pull together all the patches so that it
doesn't report bugs that are fixed later in the series? I mean, sending a
patch series to fix a bunch of issues shouldn't trigger Sashiko telling you
about the issues in the early patches where the fix is in that same patch
series later on.

Thanks,

-- Steve