[PATCH v5 00/10] ring-buffer: Fixes for subbuf resizing and persistent buffers

[PATCH v5 00/10] ring-buffer: Fixes for subbuf resizing and persistent buffers

[Vincent Donnefort]

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

Changelog:

v5:

  - Reset info->spare_read only when data is in the ring-buffer (Sashiko)
  - Use `unsigned long` for subbuf_size declaration to avoid 32-bit
    truncation. (Sashiko)
  - Make cpu_buffer::free_page a buffer_read_data_page
  - Update kerneldoc for ring_buffer_alloc_read_page()

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

  - 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 (10):
  ring-buffer: Free cpu_buffer::free_page with subbuf_order
  ring-buffer: Hold cpu_buffer::lock when resizing a subbuf
  ring-buffer: Make cpu_buffer::free_page a buffer_data_read_page
  ring-buffer: Fix subbuf resize race with ring buffer readers
  ring-buffer: Fix subbuf resize race with ring_buffer_alloc_read_page()
  tracing: Fix subbuf resize races with 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           | 393 ++++++++++++++++-----------
 kernel/trace/ring_buffer_benchmark.c |   2 +-
 kernel/trace/trace.c                 |  91 ++++---
 kernel/trace/trace.h                 |   1 -
 5 files changed, 291 insertions(+), 200 deletions(-)


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

[PATCH v5 01/10] 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 v5 02/10] 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 v5 03/10] ring-buffer: Make cpu_buffer::free_page a buffer_data_read_page

[Vincent Donnefort]

Discarding a cached reader page after a concurrent ring buffer resize
uses the new global subbuf_order for the free_pages() call. This
mismatched order may crashes the kernel or leaks memory because the cached
page was allocated under the old size.

Save the actual free_page order alongside the page address to ensure we
always refer to the correct value and do not rely on the potentially
stalled cpu_buffer->subbuf_order value. The simplest is to make
free_page a buffer_data_read_page which already covers exactly what we
need: a page address and a page order.

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 ec4f5a0c93e8..b6fa258aafe2 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -507,7 +507,7 @@ struct ring_buffer_per_cpu {
 	raw_spinlock_t			reader_lock;	/* serialize readers */
 	arch_spinlock_t			lock;
 	struct lock_class_key		lock_key;
-	struct buffer_data_page		*free_page;
+	struct buffer_data_read_page	free_page;
 	unsigned long			nr_pages;
 	unsigned int			current_context;
 	struct list_head		*pages;
@@ -2631,7 +2631,7 @@ static void rb_free_cpu_buffer(struct ring_buffer_per_cpu *cpu_buffer)
 		free_buffer_page(bpage);
 	}
 
-	free_pages((unsigned long)cpu_buffer->free_page, cpu_buffer->buffer->subbuf_order);
+	free_pages((unsigned long)cpu_buffer->free_page.data, cpu_buffer->free_page.order);
 
 	kfree(cpu_buffer);
 }
@@ -6962,9 +6962,9 @@ ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu)
 	local_irq_save(flags);
 	arch_spin_lock(&cpu_buffer->lock);
 
-	if (cpu_buffer->free_page) {
-		bpage->data = cpu_buffer->free_page;
-		cpu_buffer->free_page = NULL;
+	if (cpu_buffer->free_page.data) {
+		*bpage = cpu_buffer->free_page;
+		cpu_buffer->free_page.data = NULL;
 	}
 
 	arch_spin_unlock(&cpu_buffer->lock);
@@ -7016,8 +7016,8 @@ void ring_buffer_free_read_page(struct trace_buffer *buffer, int cpu,
 	local_irq_save(flags);
 	arch_spin_lock(&cpu_buffer->lock);
 
-	if (!cpu_buffer->free_page) {
-		cpu_buffer->free_page = dpage;
+	if (!cpu_buffer->free_page.data) {
+		cpu_buffer->free_page = *data_page;
 		dpage = NULL;
 	}
 
@@ -7390,7 +7390,7 @@ int ring_buffer_subbuf_order_set(struct trace_buffer *buffer, int order)
 	}
 
 	for_each_buffer_cpu(buffer, cpu) {
-		struct buffer_data_page *old_free_data_page;
+		struct buffer_data_read_page old_free_data_page;
 		struct list_head old_pages;
 		unsigned long flags;
 
@@ -7433,7 +7433,7 @@ int ring_buffer_subbuf_order_set(struct trace_buffer *buffer, int order)
 
 		arch_spin_lock(&cpu_buffer->lock);
 		old_free_data_page = cpu_buffer->free_page;
-		cpu_buffer->free_page = NULL;
+		cpu_buffer->free_page.data = NULL;
 		arch_spin_unlock(&cpu_buffer->lock);
 
 		rb_head_page_activate(cpu_buffer);
@@ -7445,7 +7445,7 @@ int ring_buffer_subbuf_order_set(struct trace_buffer *buffer, int order)
 			list_del_init(&bpage->list);
 			free_buffer_page(bpage);
 		}
-		free_pages((unsigned long)old_free_data_page, old_order);
+		free_pages((unsigned long)old_free_data_page.data, old_free_data_page.order);
 
 		rb_check_pages(cpu_buffer);
 	}
-- 
2.55.0.691.gc56d675ccc-goog

[PATCH v5 04/10] 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 b6fa258aafe2..ec520c72124e 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 v5 05/10] ring-buffer: Fix subbuf resize race with 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 ec520c72124e..a00ab8a9cbd0 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 v5 06/10] tracing: Fix subbuf resize races with 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 a00ab8a9cbd0..83292d90599e 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -6976,34 +6976,52 @@ EXPORT_SYMBOL_GPL(ring_buffer_swap_cpu);
  * ring_buffer_alloc_read_page - allocate a page to read from buffer
  * @buffer: the buffer to allocate for.
  * @cpu: the cpu buffer to allocate.
+ * @prev: The previous page to be repurposed (can be NULL).
  *
- * This function is used in conjunction with ring_buffer_read_page.
+ * This function is used in conjunction with ring_buffer_read_page().
  * When reading a full page from the ring buffer, these functions
  * can be used to speed up the process. The calling function should
  * allocate a few pages first with this function. Then when it
  * needs to get pages from the ring buffer, it passes the result
- * of this function into ring_buffer_read_page, which will swap
+ * of this function into ring_buffer_read_page(), which will swap
  * the page that was allocated, with the read page of the buffer.
  *
+ * If @prev is provided, and it has a different order than the current
+ * subbuffer order, its payload will be freed and re-allocated. If it
+ * already matches the order, it is simply returned.
+ *
  * 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 ring_buffer_per_cpu *cpu_buffer;
-	struct buffer_data_read_page *bpage = NULL;
+	struct buffer_data_read_page *bpage;
 	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);
+	order = buffer->subbuf_order;
 
-	bpage->order = buffer->subbuf_order;
+	if (prev && prev->order == order) {
+		return prev;
+	} else if (prev) {
+		/* We can reuse prev, but we discard the payload */
+		free_pages((unsigned long)prev->data, prev->order);
+		prev->data = NULL;
+		bpage = prev;
+	} else {
+		bpage = kzalloc_obj(*bpage);
+		if (!bpage)
+			return ERR_PTR(-ENOMEM);
+	}
+
+	bpage->order = order;
 	cpu_buffer = buffer->buffers[cpu];
+
 	local_irq_save(flags);
 	arch_spin_lock(&cpu_buffer->lock);
 
@@ -7020,7 +7038,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 +7061,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 +7341,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..f9399f391ac6 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,30 @@ 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)
+	/* 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->read = spare_size;
+	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 +7123,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 +7140,14 @@ 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 +7268,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 +7281,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 +7294,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 +7303,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 +7330,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 v5 07/10] 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 83292d90599e..d7b3c56e4263 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);
 
@@ -7982,7 +7996,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 v5 08/10] 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 d7b3c56e4263..905b2e6df9ab 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 v5 09/10] 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 905b2e6df9ab..79459c99ce9a 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 */
@@ -7163,7 +7165,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;
@@ -7202,8 +7203,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
@@ -7213,7 +7212,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;
@@ -7674,11 +7673,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))
@@ -7687,13 +7682,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);
@@ -7865,7 +7857,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 {
@@ -7922,8 +7913,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 v5 10/10] 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.

Where necessary, use unsigned long for the size to ensure
multiplications are not truncated to 32-bits.

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

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 79459c99ce9a..d9dcd6d4e7cc 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_read_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, unsigned long 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);
+	unsigned long 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)
@@ -1731,7 +1731,7 @@ static void *rb_range_meta(struct trace_buffer *buffer, int nr_pages, int cpu)
 /* Return the start of subbufs given the meta pointer */
 static void *rb_subbufs_from_meta(struct ring_buffer_cpu_meta *meta)
 {
-	int subbuf_size = meta->subbuf_size;
+	unsigned long subbuf_size = meta->subbuf_size;
 	unsigned long ptr;
 
 	ptr = (unsigned long)meta;
@@ -1746,8 +1746,7 @@ static void *rb_subbufs_from_meta(struct ring_buffer_cpu_meta *meta)
 static void *rb_range_buffer(struct ring_buffer_per_cpu *cpu_buffer, int idx)
 {
 	struct ring_buffer_cpu_meta *meta;
-	unsigned long ptr;
-	int subbuf_size;
+	unsigned long subbuf_size, ptr;
 
 	meta = rb_range_meta(cpu_buffer->buffer, 0, cpu_buffer->cpu);
 	if (!meta)
@@ -1840,10 +1839,10 @@ 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;
+	unsigned long subbuf_size = PAGE_SIZE;
 	unsigned long buffers_start;
 	unsigned long buffers_end;
 	int i;
@@ -2231,7 +2230,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 +2330,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 +2416,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 +2519,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 +2544,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);
@@ -2703,7 +2702,7 @@ static void rb_test_inject_invalid_pages(struct trace_buffer *buffer)
 	struct buffer_data_page *dpage;
 	unsigned long entry_bytes = 0;
 	unsigned long ptr;
-	int subbuf_size;
+	unsigned long subbuf_size;
 	int invalid = 0;
 	int cpu;
 	int i;
@@ -2773,8 +2772,8 @@ 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;
-	int subbuf_size;
+	unsigned long subbuf_size;
+	unsigned int nr_pages;
 	int bsize;
 	int cpu;
 	int ret;
@@ -3037,12 +3036,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 +3263,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;
 
 	/*
@@ -4229,7 +4228,7 @@ rb_set_commit_to_write(struct ring_buffer_per_cpu *cpu_buffer)
 	 * assign the commit to the tail.
 	 */
  again:
-	max_count = cpu_buffer->nr_pages * 100;
+	max_count = cpu_buffer->nr_pages * 100UL;
 
 	while (cpu_buffer->commit_page != READ_ONCE(cpu_buffer->tail_page)) {
 		if (RB_WARN_ON(cpu_buffer, !(--max_count)))
@@ -6522,7 +6521,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);
 
@@ -7418,8 +7417,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;
@@ -7465,8 +7464,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)
@@ -7708,8 +7708,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;
@@ -8039,9 +8039,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 v5 04/10] ring-buffer: Fix subbuf resize race with ring buffer readers

[sashiko-bot]

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

New issues:
- [High] `ring_buffer_read_start()` introduces a Use-After-Free vulnerability for callers using `GFP_ATOMIC`.

Pre-existing issues:
- [High] Race condition and Out-of-Bounds write in `ring_buffer_map_get_reader()` due to evaluating the global `buffer->subbuf_order`.
--

--- Patch [4]: [PATCH v5 04/10] ring-buffer: Fix subbuf resize race with ring buffer readers ---
commit 71a276d1d136c0fb8371689f58251aa6fc99f54c
Author: Vincent Donnefort <vdonnefort@google.com>

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

This commit replaces the lockless read of trace_buffer::subbuf_size with
bpage::order in ring_buffer_read_start() and ring_buffer_read_page() to
prevent races during resizing. It also removes subbuf_size entirely and
introduces new capacity accessors.

> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index b6fa258aafe2c..ec520c72124e5 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)
>  {

[ ... ]

> +	/*
> +	 * 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 cause a use-after-free when called with GFP_ATOMIC?

If this is called with GFP_ATOMIC, it skips taking buffer->mutex and
increments resize_disabled without synchronization. If a concurrent
ring_buffer_subbuf_order_set() evaluated resize_disabled to 0 just before
this increment, it can proceed to replace and free the reader_page.

The lockless dereference of bpage->order inside rb_page_capacity() on the
freed reader_page would then access freed memory. Prior to this patch, it
read the scalar buffer->subbuf_size which was safe from this use-after-free.

[ ... ]

> @@ -7340,18 +7383,21 @@ int ring_buffer_subbuf_order_set(struct trace_buffer *buffer, int order)

[ ... ]

>  	/* 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;

[ ... ]

> @@ -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 can this lead to an out-of-bounds write?

Since ring_buffer_map_get_reader() does not hold buffer->mutex, it can run
concurrently with ring_buffer_subbuf_order_set(). As shown in the hunk above,
ring_buffer_subbuf_order_set() updates the global buffer->subbuf_order before
checking if resize_disabled is set on the CPU buffers.

If ring_buffer_map_get_reader() reads this temporarily inflated global
capacity while processing an older, smaller reader_page, the capacity check
could incorrectly pass. This would allow the following memcpy() to write
missed_events out of bounds.

Should this use rb_page_capacity(reader) instead of the global
rb_subbuf_capacity(buffer), similar to the fix applied to
ring_buffer_read_page()?

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

[Severity: High]
This isn't a bug introduced by this patch, but could this flush more memory
than is actually mapped?

Similar to the check above, if this reads a temporarily inflated global
capacity while flushing an older, smaller reader_page, the flush range
might exceed the actual allocated page bounds.

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

Re: [PATCH v5 06/10] tracing: Fix subbuf resize races with trace_pipe_raw readers

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Data Race / Double Free in tracing_buffers_read with concurrent readers
--

--- Patch [6]: [PATCH v5 06/10] tracing: Fix subbuf resize races with trace_pipe_raw readers ---
commit a4a4fc189c85fb142cc626dac47e73d02ae21aef
Author: Vincent Donnefort <vdonnefort@google.com>

tracing: Fix subbuf resize races with trace_pipe_raw readers

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().

> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index a00ab8a9cbd0c..83292d90599ec 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
[ ... ]
> +struct buffer_data_read_page *ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu,
> +							  struct buffer_data_read_page *prev)
>  {
[ ... ]
> +	if (prev && prev->order == order) {
> +		return prev;
> +	} else if (prev) {
> +		/* We can reuse prev, but we discard the payload */
> +		free_pages((unsigned long)prev->data, prev->order);
> +		prev->data = NULL;
> +		bpage = prev;
> +	} else {
[ ... ]
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 395238b2b715c..f9399f391ac6c 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,
[ ... ]
> +again:
> +	prev_spare = info->spare;
[ ... ]
> -	/* Do we have previous read data to read? */
> -	if (info->read < page_size)
> -		goto read;
> +	/* 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);

[Severity: Critical]
This isn't a bug introduced by this patch, but it looks like a pre-existing
issue with concurrent readers remains. If two threads share a trace_pipe_raw
file descriptor (e.g. via fork or pthreads) and call read() concurrently
while the subbuffer order is being changed, can they trigger a double-free?

Since vfs_read() does not hold f_pos_lock for this file (because
FMODE_ATOMIC_POS is cleared by nonseekable_open()), tracing_buffers_read()
can execute concurrently. If both threads read the same info->spare pointer
into prev_spare:

Thread 1:
    prev_spare = info->spare;

Thread 2:
    prev_spare = info->spare;

When both threads call into ring_buffer_alloc_read_page() with the same
prev pointer, and the subbuffer order has changed, wouldn't both threads
concurrently enter the 'else if (prev)' block and call free_pages() on the
same address without synchronization?

ring_buffer_alloc_read_page() {
    ...
    } else if (prev) {
        free_pages((unsigned long)prev->data, prev->order);
        prev->data = NULL;
    ...
}

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

Re: [PATCH v5 01/10] 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] A data race and locking mismatch between `ring_buffer_subbuf_order_set()`, `ring_buffer_alloc_read_page()`, and `ring_buffer_free_read_page()` allows `cpu_buffer->free_page` to cache a page of an old order while `buffer->subbuf_order` reflects a new larger order. This leads to heap buffer overflow and buddy allocator corruption.
--

--- Patch [1]: [PATCH v5 01/10] ring-buffer: Free cpu_buffer::free_page with subbuf_order ---
commit f550c0bfeadcc22eca47316e1820b4d91f6ef261
Author: Vincent Donnefort <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 is a pre-existing issue, but does this change expose a buddy allocator
corruption if cpu_buffer->free_page contains a page of a smaller order than
buffer->subbuf_order?

A race condition between ring_buffer_subbuf_order_set() and 
ring_buffer_free_read_page() can cause a smaller order page to be stashed in
cpu_buffer->free_page while buffer->subbuf_order reflects a new larger
order.

If a trace reader calls ring_buffer_free_read_page(), it performs a lockless
check of buffer->subbuf_order:

kernel/trace/ring_buffer.c:ring_buffer_free_read_page() {
    ...
	if (page_ref_count(page) > 1 || data_page->order != buffer->subbuf_order)
		goto out;
    ...
}

If a concurrent write to buffer_subbuf_size_kb calls
ring_buffer_subbuf_order_set(), it updates buffer->subbuf_order without
holding cpu_buffer->lock, and clears cpu_buffer->free_page under
reader_lock.

The trace reader then acquires cpu_buffer->lock and stashes the old smaller
page into cpu_buffer->free_page, missing the clearance because of the lock
mismatch (reader_lock vs cpu_buffer->lock):

kernel/trace/ring_buffer.c:ring_buffer_free_read_page() {
    ...
	raw_spin_lock_irqsave(&cpu_buffer->lock, flags);
	if (!cpu_buffer->free_page) {
		cpu_buffer->free_page = dpage;
    ...
}

During teardown, free_pages() here would free the undersized page using the
new larger order, causing buddy allocator corruption. Additionally, if a reader
allocates this stashed page via ring_buffer_read_page(), it is swapped into
the active ring buffer where writers will overflow its bounds.

Can the locking be synchronized to prevent caching an old-order page?

>  
>  	kfree(cpu_buffer);
>  }

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

Re: [PATCH v5 10/10] 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 overflow in ring_buffer_resize() allows an attacker to bypass allocation checks and trigger a massive out-of-bounds ring buffer shrink, leading to a Use-After-Free.
- [Medium] 32-bit shift overflow in __rb_map_vma() truncates the mapped page count for huge ring buffers, causing mmap to fail.
--

--- Patch [10]: [PATCH v5 10/10] ring-buffer: Make nr_pages unsigned int ---
commit 25e7b6bfdf3741f228ceb9dc41c8bbf6e253f525
Author: Vincent Donnefort <vdonnefort@google.com>

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.

> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 79459c99ce9a0..d9dcd6d4e7cc4 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;

[ ... ]

> @@ -3264,7 +3263,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;

[Severity: Critical]
If a huge value is written to buffer_size_kb, could the unsigned
difference between nr_pages and cpu_buffer->nr_pages overflow when
assigned to the now 32-bit signed cpu_buffer->nr_pages_to_update?

Later in this function, we do:

    cpu_buffer->nr_pages_to_update = nr_pages - cpu_buffer->nr_pages;
    if (cpu_buffer->nr_pages_to_update <= 0)
        continue;

If the difference overflows into a negative integer, wouldn't it skip this
allocation limits check but still schedule the worker thread (since it is
non-zero)?

The worker thread calls rb_update_pages():

    success = rb_remove_pages(cpu_buffer, -cpu_buffer->nr_pages_to_update);

This would pass a large positive count to rb_remove_pages(). As it wildly
wraps around the small circular list:

    for (nr_removed = 0; nr_removed < nr_pages; nr_removed++) {
        to_remove = rb_list_head(to_remove)->next;
        head_bit |= (unsigned long)to_remove & RB_PAGE_HEAD;
    }

Could this unlink and double-free actively used pages, leaving ring buffer
structures pointing to freed memory and causing a use-after-free?

>  	int cpu, err;
>  
>  	/*

[ ... ]

> @@ -7708,8 +7708,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;

[Severity: Medium]
Since nr_subbufs is now an unsigned int, could the bitwise shift operation
later in this function silently truncate for exceptionally large ring buffers?

__rb_map_vma() later computes:

    nr_pages = ((nr_subbufs + 1) << subbuf_order) - 1;

Because the operands are now 32-bit, the shift is computed entirely in
32-bit math before assignment to the unsigned long nr_pages. 

If a buffer is large enough to overflow 32-bits, will this cause bounds
checks to fail and mmap to return -EINVAL?

>  	struct page **pages __free(kfree) = NULL;
>  	int p = 0, s = 0;
>  	int err;

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] `ring_buffer_alloc_read_page()` returning a cached page with an obsolete order causes `tracing_buffers_read()` to permanently return EOF to userspace.
--

--- Patch [3]: [PATCH v5 03/10] ring-buffer: Make cpu_buffer::free_page a buffer_data_read_page ---
commit 19f06657067d8dbcf108f6bacc6885d031739166
Author: Vincent Donnefort <vdonnefort@google.com>

    ring-buffer: Make cpu_buffer::free_page a buffer_data_read_page

This commit changes the free_page cache to store the page order alongside
the page address. This fixes a mismatch when a cached reader page is
discarded after a concurrent ring buffer resize changes the global
subbuf_order.

> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index ec4f5a0c93e85..b6fa258aafe2c 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
[ ... ]
> @@ -6962,9 +6962,9 @@ ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu)
>  	local_irq_save(flags);
>  	arch_spin_lock(&cpu_buffer->lock);
>  
> -	if (cpu_buffer->free_page) {
> -		bpage->data = cpu_buffer->free_page;
> -		cpu_buffer->free_page = NULL;
> +	if (cpu_buffer->free_page.data) {
> +		*bpage = cpu_buffer->free_page;
> +		cpu_buffer->free_page.data = NULL;

[Severity: High]
Does this struct assignment cause tracing_buffers_read() to permanently
return EOF to userspace during a concurrent resize?

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

	buffer->subbuf_order = order;

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

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

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

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

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

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

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

This returns -1, which tracing_buffers_read() treats as EOF. Subsequent
reads see info->spare_size == page_size, reuse the bad page, and
permanently return EOF.

>  	}
>  
>  	arch_spin_unlock(&cpu_buffer->lock);

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

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

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Race condition during ring buffer resize leads to mismatched subbuffer order, causing heap overflow and buddy allocator corruption.
--

--- Patch [2]: [PATCH v5 02/10] ring-buffer: Hold cpu_buffer::lock when resizing a subbuf ---
commit b0f06308baef769af600659941453301acb7bea1
Author: Vincent Donnefort <vdonnefort@google.com>

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

This commit adds locking around the clearing of cpu_buffer->free_page in
ring_buffer_subbuf_order_set() to prevent races with readers allocating
or freeing read pages.

> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index a3d28b2e2c94a..ec4f5a0c93e85 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -7431,8 +7431,10 @@ int ring_buffer_subbuf_order_set(struct trace_buffer *buffer, int order)
>  		cpu_buffer->nr_pages = cpu_buffer->nr_pages_to_update;
>  		cpu_buffer->nr_pages_to_update = 0;
>  
> +		arch_spin_lock(&cpu_buffer->lock);
>  		old_free_data_page = cpu_buffer->free_page;
>  		cpu_buffer->free_page = NULL;
> +		arch_spin_unlock(&cpu_buffer->lock);

[Severity: Critical]
This is a pre-existing issue, but does adding this lock fully close the race
window with ring_buffer_alloc_read_page()?

Looking at ring_buffer_subbuf_order_set(), buffer->subbuf_order = order; is
set much earlier in the function, before allocating the new pages for each
CPU.

During this window, a concurrent reader calling ring_buffer_alloc_read_page()
can see the new global order but still claim the old, smaller free_page
from the per-CPU buffer:

ring_buffer_alloc_read_page() {
    bpage->order = buffer->subbuf_order; // gets new, larger order
    ...
    if (cpu_buffer->free_page) {
        bpage->data = cpu_buffer->free_page; // gets old, smaller page
        cpu_buffer->free_page = NULL;
    }
}

Later, if ring_buffer_free_read_page() is called, it will use the mismatched
bpage->order to free the old page, corrupting the buddy allocator.

Similarly, if ring_buffer_read_page() swaps this undersized page into the
ring buffer, producers will overflow the heap.

Should buffer->subbuf_order be updated after the per-CPU free pages are
safely cleared or replaced?

>  
>  		rb_head_page_activate(cpu_buffer);
>

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