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

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

[Vincent Donnefort]

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

I have managed to reproduce a ring_buffer_read_page() race with

 $ while true; do for i in 8 16 32; do echo $i > /sys/kernel/tracing/buffer_subbuf_size_kb; sleep 0.1; done; done

Changelog:

v9:

  - Fix splice read loop when buffer size changes at the same time (Steven)
  - Allow splice for all static buffers (via memcpy)
  - Rebase on 7.3-rc1

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

  - ring_buffer_read_page() handles gracefully read_page/reader_page
    order mismatch (Steven)

v7 (https://lore.kernel.org/linux-trace-kernel/20260817101533.1558223-1-vdonnefort@google.com/):

  - Match the "static" rb limit with bpage::id bitwidth
  - Cover another 32-bit truncation in rb_range_buffer (Sashiko)
  - Fix uninitialized spare_size (Sashiko)

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

  - New prototype for ring_buffer_alloc_read_page() (Steven)
  - ring_buffer_read_page() to return -EAGAIN (Steven)
  - Keep nr_pages "unsigned long" (Steven)
  - Repase on ring-buffer/next (Drop most of the patches)

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

  - 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 (4):
  ring-buffer: Allow splice reads on static buffers
  tracing: Fix subbuf resize races with trace_pipe_raw readers
  ring-buffer: Cap static ring buffer nr_pages
  ring-buffer: Prevent truncation of nr_pages / nr_subbufs

 include/linux/ring_buffer.h          |   5 +-
 kernel/trace/ring_buffer.c           | 224 +++++++++++++++++----------
 kernel/trace/ring_buffer_benchmark.c |   6 +-
 kernel/trace/trace.c                 |  95 +++++-------
 kernel/trace/trace.h                 |   9 +-
 5 files changed, 196 insertions(+), 143 deletions(-)


base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
-- 
2.55.0.897.gb25b4bd76c-goog

[PATCH v9 1/4] ring-buffer: Allow splice reads on static buffers

[Vincent Donnefort]

ring_buffer_read_page() rejects splice (full=1) reads on static buffers
(that is user-mapped, persistent or remote) because !read check assumes
unread pages must be swapped. However for those buffers we have no other
choice than memcpy the data.

For the memcpy case, only return an error when the writer is still on
the reader page for the splice interface to wait.

Fixes: 117c39200d9d ("ring-buffer: Introducing ring-buffer mapping functions")
Signed-off-by: Vincent Donnefort <vdonnefort@google.com>

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index b0963ac6fd16..84fd4cdd486f 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -7193,15 +7193,8 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
 		unsigned int event_size;
 		unsigned int flags = 0;
 
-		/*
-		 * If a full page is expected, this can still be returned
-		 * if there's been a previous partial read and the
-		 * rest of the page can be read and the commit page is off
-		 * the reader page.
-		 */
-		if (full &&
-		    (!read || (len < (size - read)) ||
-		     cpu_buffer->reader_page == cpu_buffer->commit_page))
+		/* If a full page is requested, it cannot be the commit page */
+		if (full && cpu_buffer->reader_page == cpu_buffer->commit_page)
 			return -1;
 
 		if (len > (size - read))
-- 
2.55.0.897.gb25b4bd76c-goog

[PATCH v9 2/4] 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 handle the resizing of an
existing buffer_data_read_page if necessary and add a new
ring_buffer_read_page_size(). This new function enables ring-buffer
buffer_data_read_page users to not call the racy
ring_buffer_subbuf_size_get(). This makes the spare_size member of
ftrace_buffer_info redundant.

Finally, handle buffer_data_read_page/reader_page order discrepancy in
ring_buffer_read_page(). On a mismatch simply copy manually the data to
the buffer_data_read_page.

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..afc7daa6ee7d 100644
--- a/include/linux/ring_buffer.h
+++ b/include/linux/ring_buffer.h
@@ -218,14 +218,15 @@ bool ring_buffer_time_stamp_abs(struct trace_buffer *buffer);
 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);
+int ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu,
+				struct buffer_data_read_page **rpage);
 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 *rpage);
 
 struct trace_seq;
 
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 84fd4cdd486f..28dd76edfecf 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -330,6 +330,11 @@ struct buffer_data_read_page {
 	struct buffer_data_page	*data;	/* actual data, stored in this page */
 };
 
+static __always_inline unsigned int rb_read_page_capacity(struct buffer_data_read_page *rpage)
+{
+	return (PAGE_SIZE << rpage->order) - BUF_PAGE_HDR_SIZE;
+}
+
 /*
  * Note, the buffer_page list must be first. The buffer pages
  * are allocated in cache lines, which means that each buffer
@@ -6993,56 +6998,78 @@ 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.
+ * @rpage: pointer to pass in an already allocated page (can be NULL)
+ *         and returns the allocated page.
  *
- * 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 @rpage 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
+ *  0 on success, < 0 on error
  */
-struct buffer_data_read_page *
-ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu)
+int ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu,
+				struct buffer_data_read_page **rpage)
 {
 	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);
+		return -ENODEV;
 
-	bpage = kzalloc_obj(*bpage);
-	if (!bpage)
-		return ERR_PTR(-ENOMEM);
+	if (!rpage)
+		return -EINVAL;
 
-	bpage->order = buffer->subbuf_order;
+	order = buffer->subbuf_order;
+
+	if (*rpage) {
+		if ((*rpage)->order == order)
+			return 0;
+
+		/* We can reuse rpage, but we discard the payload */
+		free_pages((unsigned long)(*rpage)->data, (*rpage)->order);
+		(*rpage)->data = NULL;
+	} else {
+		*rpage = kzalloc_obj(**rpage);
+		if (!*rpage)
+			return -ENOMEM;
+	}
+
+	(*rpage)->order = order;
 	cpu_buffer = buffer->buffers[cpu];
+
 	local_irq_save(flags);
 	arch_spin_lock(&cpu_buffer->lock);
 
 	if (cpu_buffer->free_page.data) {
-		*bpage = cpu_buffer->free_page;
+		**rpage = cpu_buffer->free_page;
 		cpu_buffer->free_page.data = NULL;
 	}
 
 	arch_spin_unlock(&cpu_buffer->lock);
 	local_irq_restore(flags);
 
-	if (bpage->data) {
-		rb_init_data_page(bpage->data);
+	if ((*rpage)->data) {
+		rb_init_data_page((*rpage)->data);
 	} else {
-		bpage->data = alloc_cpu_data(cpu, bpage->order);
-		if (!bpage->data) {
-			kfree(bpage);
-			return ERR_PTR(-ENOMEM);
+		(*rpage)->data = alloc_cpu_data(cpu, (*rpage)->order);
+		if (!(*rpage)->data) {
+			kfree(*rpage);
+			*rpage = NULL;
+			return -ENOMEM;
 		}
 	}
 
-	return bpage;
+	return 0;
 }
 EXPORT_SYMBOL_GPL(ring_buffer_alloc_read_page);
 
@@ -7050,21 +7077,30 @@ EXPORT_SYMBOL_GPL(ring_buffer_alloc_read_page);
  * ring_buffer_free_read_page - free an allocated read page
  * @buffer: the buffer the page was allocate for
  * @cpu: the cpu buffer the page came from
- * @data_page: the page to free
+ * @rpage: the buffer_dat_read_page to free
  *
  * Free a page allocated from ring_buffer_alloc_read_page.
  */
 void ring_buffer_free_read_page(struct trace_buffer *buffer, int cpu,
-				struct buffer_data_read_page *data_page)
+				struct buffer_data_read_page *rpage)
 {
 	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 (!rpage)
+		return;
+
+	dpage = rpage->data;
+	if (!dpage)
+		goto out;
+
+	page = virt_to_page(dpage);
+
 	cpu_buffer = buffer->buffers[cpu];
 
 	/*
@@ -7072,14 +7108,14 @@ void ring_buffer_free_read_page(struct trace_buffer *buffer, int cpu,
 	 * is different from the subbuffer order of the buffer -
 	 * we can't reuse it
 	 */
-	if (page_ref_count(page) > 1 || data_page->order != buffer->subbuf_order)
+	if (page_ref_count(page) > 1 || rpage->order != buffer->subbuf_order)
 		goto out;
 
 	local_irq_save(flags);
 	arch_spin_lock(&cpu_buffer->lock);
 
 	if (!cpu_buffer->free_page.data) {
-		cpu_buffer->free_page = *data_page;
+		cpu_buffer->free_page = *rpage;
 		dpage = NULL;
 	}
 
@@ -7087,8 +7123,8 @@ void ring_buffer_free_read_page(struct trace_buffer *buffer, int cpu,
 	local_irq_restore(flags);
 
  out:
-	free_pages((unsigned long)dpage, data_page->order);
-	kfree(data_page);
+	free_pages((unsigned long)dpage, rpage->order);
+	kfree(rpage);
 }
 EXPORT_SYMBOL_GPL(ring_buffer_free_read_page);
 
@@ -7159,10 +7195,9 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
 	if (!dpage)
 		return -1;
 
-	guard(raw_spinlock_irqsave)(&cpu_buffer->reader_lock);
+	len = min_t(size_t, len, rb_read_page_capacity(data_page));
 
-	if (data_page->order != cpu_buffer->reader_page->order)
-		return -1;
+	guard(raw_spinlock_irqsave)(&cpu_buffer->reader_lock);
 
 	reader = rb_get_reader_page(cpu_buffer);
 	if (!reader)
@@ -7177,16 +7212,18 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
 	/* Check if any events were dropped */
 	missed_events = cpu_buffer->lost_events;
 
-	/*
-	 * If this page has been partially read or
-	 * if len is not big enough to read the rest of the page or
-	 * a writer is still on the page, then
-	 * we must copy the data from the page to the buffer.
-	 * Otherwise, we can simply swap the page with the one passed in.
-	 */
+	 /*
+	  * It is not possible to swap the reader page if:
+	  *   - It has been partially read
+	  *   - len is not big enough to read it entirely
+	  *   - A writer is still on it
+	  *   - The ring buffer is static
+	  *   - The order doesn't match
+	  */
 	if (read || (len < (size - read)) ||
 	    cpu_buffer->reader_page == cpu_buffer->commit_page ||
-	    rb_is_static(cpu_buffer)) {
+	    rb_is_static(cpu_buffer) ||
+	    data_page->order != reader->order) {
 		struct buffer_data_page *rpage = cpu_buffer->reader_page->page;
 		unsigned int rpos = read;
 		unsigned int pos = 0;
@@ -7280,7 +7317,7 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
 		 * missed events, then record it there.
 		 */
 		if (missed_events > 0 &&
-		    rb_page_capacity(reader) - size >= sizeof(missed_events)) {
+		    rb_read_page_capacity(data_page) - size >= sizeof(missed_events)) {
 			memcpy(&dpage->data[size], &missed_events,
 			       sizeof(missed_events));
 			local_add(RB_MISSED_STORED, &dpage->commit);
@@ -7300,8 +7337,8 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
 	/*
 	 * This page may be off to user land. Zero it out here.
 	 */
-	if (size < rb_page_capacity(reader))
-		memset(&dpage->data[size], 0, rb_page_capacity(reader) - size);
+	if (size < rb_read_page_capacity(data_page))
+		memset(&dpage->data[size], 0, rb_read_page_capacity(data_page) - size);
 
 	return read;
 }
@@ -7319,6 +7356,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 *rpage)
+{
+	return PAGE_SIZE << rpage->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..c3d34c0e64e2 100644
--- a/kernel/trace/ring_buffer_benchmark.c
+++ b/kernel/trace/ring_buffer_benchmark.c
@@ -104,7 +104,7 @@ static enum event_status read_event(int cpu)
 
 static enum event_status read_page(int cpu)
 {
-	struct buffer_data_read_page *bpage;
+	struct buffer_data_read_page *bpage = NULL;
 	struct ring_buffer_event *event;
 	struct rb_page *rpage;
 	unsigned long commit;
@@ -114,8 +114,8 @@ static enum event_status read_page(int cpu)
 	int inc;
 	int i;
 
-	bpage = ring_buffer_alloc_read_page(buffer, cpu);
-	if (IS_ERR(bpage))
+	ret = ring_buffer_alloc_read_page(buffer, cpu, &bpage);
+	if (ret < 0)
 		return EVENT_DROPPED;
 
 	page_size = ring_buffer_subbuf_size_get(buffer);
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index a946e0183fd1..7ba3856daf44 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -7082,8 +7082,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;
+	unsigned int spare_size;
 	void *trace_data;
-	int page_size;
 	ssize_t ret = 0;
 	ssize_t size;
 
@@ -7093,36 +7093,24 @@ 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);
-
-	/* Make sure the spare matches the current sub buffer size */
+again:
 	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;
-		}
+		spare_size = ring_buffer_read_page_size(info->spare);
+		/* 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 subbuf order */
+	ret = ring_buffer_alloc_read_page(iter->array_buffer->buffer, iter->cpu_file,
+					  &info->spare);
+	if (ret)
 		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,
@@ -7148,8 +7136,9 @@ ssize_t tracing_buffers_read(struct file *filp, char __user *ubuf,
 	}
 
 	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);
@@ -7199,17 +7188,17 @@ int tracing_buffers_release(struct inode *inode, struct file *file)
 }
 
 struct buffer_ref {
-	struct trace_buffer	*buffer;
-	void			*page;
-	int			cpu;
-	refcount_t		refcount;
+	struct trace_buffer		*buffer;
+	struct buffer_data_read_page	*rpage;
+	int				cpu;
+	refcount_t			refcount;
 };
 
 static void buffer_ref_release(struct buffer_ref *ref)
 {
 	if (!refcount_dec_and_test(&ref->refcount))
 		return;
-	ring_buffer_free_read_page(ref->buffer, ref->cpu, ref->page);
+	ring_buffer_free_read_page(ref->buffer, ref->cpu, ref->rpage);
 	kfree(ref);
 }
 
@@ -7270,23 +7259,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;
 
@@ -7294,7 +7272,8 @@ 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;
 
@@ -7306,25 +7285,36 @@ 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);
-		if (IS_ERR(ref->page)) {
-			ret = PTR_ERR(ref->page);
-			ref->page = NULL;
+
+		ret = ring_buffer_alloc_read_page(ref->buffer, iter->cpu_file, &ref->rpage);
+		if (ret) {
 			kfree(ref);
 			break;
 		}
 		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->rpage);
+
+		r = -EINVAL;
+		if (IS_ALIGNED(*ppos, page_size) && len >= page_size) {
+			r = ring_buffer_read_page(ref->buffer, ref->rpage, 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->rpage);
 			kfree(ref);
 			break;
 		}
 
-		page = virt_to_page(ring_buffer_read_page_data(ref->page));
+		page = virt_to_page(ring_buffer_read_page_data(ref->rpage));
 
 		spd.pages[i] = page;
 		spd.partial[i].len = page_size;
@@ -7332,6 +7322,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 74a7a50d1e78..203d098ee14e 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -745,11 +745,10 @@ static inline int tracing_get_cpu(struct inode *inode)
 void tracing_reset_cpu(struct array_buffer *buf, int cpu);
 
 struct ftrace_buffer_info {
-	struct trace_iterator	iter;
-	void			*spare;
-	unsigned int		spare_cpu;
-	unsigned int		spare_size;
-	unsigned int		read;
+	struct trace_iterator		iter;
+	struct buffer_data_read_page	*spare;
+	unsigned int			spare_cpu;
+	unsigned int			read;
 };
 
 /**
-- 
2.55.0.897.gb25b4bd76c-goog

[PATCH v9 3/4] ring-buffer: Cap static ring buffer nr_pages

[Vincent Donnefort]

Static ring buffers (i.e. persistent, user-mapped and remote) rely on
the bpage::id field. The number of pages for those ring buffers must fit
into that variable. Enforce this limit on ring buffer creation or
user-mapping.

While at it, make buffer_page::id 31 bits. This does not change the
struct buffer_page size.

Fixes: be68d63a139b ("ring-buffer: Add ring_buffer_alloc_range()")
Signed-off-by: Vincent Donnefort <vdonnefort@google.com>

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 28dd76edfecf..c4260d6ecdfc 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -350,7 +350,7 @@ struct buffer_page {
 	local_t		 entries;	/* entries on this page */
 	unsigned long	 real_end;	/* real end of data */
 	unsigned	 order;		/* order of the page */
-	u32		 id:30;		/* ID for external mapping */
+	u32		 id:31;		/* ID for external mapping */
 	u32		 range:1;	/* Mapped via a range */
 	struct buffer_data_page *page;	/* Actual data page */
 };
@@ -657,6 +657,15 @@ static bool rb_is_static(struct ring_buffer_per_cpu *cpu_buffer)
 	return cpu_buffer->user_mapped || cpu_buffer->remote || cpu_buffer->ring_meta;
 }
 
+static unsigned long rb_static_max_pages(void)
+{
+	/*
+	 * Static ring buffers are using bpage::id and must account for the
+	 * reader page.
+	 */
+	return (1UL << 31) - 1;
+}
+
 struct ring_buffer_iter {
 	struct ring_buffer_per_cpu	*cpu_buffer;
 	unsigned long			head;
@@ -2842,6 +2851,10 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
 		 */
 		nr_pages = (size - sizeof(struct ring_buffer_cpu_meta)) /
 			(subbuf_size + sizeof(int));
+
+		if (nr_pages > rb_static_max_pages())
+			goto fail_free_buffers;
+
 		/* Need at least two pages plus the reader page */
 		if (nr_pages < 3)
 			goto fail_free_buffers;
@@ -2874,6 +2887,10 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
 		/* The writer is remote. This ring-buffer is read-only */
 		atomic_inc(&buffer->record_disabled);
 		nr_pages = desc->nr_page_va - 1;
+
+		if (nr_pages > rb_static_max_pages())
+			goto fail_free_buffers;
+
 		if (nr_pages < 2)
 			goto fail_free_buffers;
 	} else {
@@ -7836,6 +7853,9 @@ int ring_buffer_map(struct trace_buffer *buffer, int cpu,
 	/* prevent another thread from changing buffer/sub-buffer sizes */
 	guard(mutex)(&buffer->mutex);
 
+	if (cpu_buffer->nr_pages > rb_static_max_pages())
+		return -E2BIG;
+
 	err = rb_alloc_meta_page(cpu_buffer);
 	if (err)
 		return err;
-- 
2.55.0.897.gb25b4bd76c-goog

[PATCH v9 4/4] ring-buffer: Prevent truncation of nr_pages / nr_subbufs

[Vincent Donnefort]

Although ring_buffer_per_cpu::nr_pages is defined as unsigned long, it
is capped to 32-bits in a few places, limiting the operations possible
on a very large buffer. Use `unsigned long` where appropriate and
prevent truncation of values using nr_pages (or nr_subbufs).

While at it, subbuf_size must be at least `unsigned int`.

Note that persistent, remote and user-mapped ring buffers are capping
the number of pages to 31 bits already, making "int" safe in many
places.

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

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index c4260d6ecdfc..ef66f726e2e2 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -1683,7 +1683,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 int subbuf_size, unsigned long nr_subbufs)
 {
 	addr += sizeof(struct ring_buffer_cpu_meta) +
 		sizeof(int) * nr_subbufs;
@@ -1693,13 +1693,12 @@ 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 long nr_pages, int cpu)
 {
-	int subbuf_size = rb_subbuf_size(buffer);
+	unsigned int subbuf_size = rb_subbuf_size(buffer);
 	struct ring_buffer_cpu_meta *meta;
 	struct ring_buffer_meta *bmeta;
-	unsigned long ptr;
-	int nr_subbufs;
+	unsigned long ptr, nr_subbufs;
 
 	bmeta = buffer->meta;
 	if (!bmeta)
@@ -1745,7 +1744,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 int subbuf_size = meta->subbuf_size;
 	unsigned long ptr;
 
 	ptr = (unsigned long)meta;
@@ -1757,11 +1756,11 @@ static void *rb_subbufs_from_meta(struct ring_buffer_cpu_meta *meta)
 /*
  * Return a specific sub-buffer for a given @cpu defined by @idx.
  */
-static void *rb_range_buffer(struct ring_buffer_per_cpu *cpu_buffer, int idx)
+static void *rb_range_buffer(struct ring_buffer_per_cpu *cpu_buffer, unsigned int idx)
 {
 	struct ring_buffer_cpu_meta *meta;
+	unsigned int subbuf_size;
 	unsigned long ptr;
-	int subbuf_size;
 
 	meta = rb_range_meta(cpu_buffer->buffer, 0, cpu_buffer->cpu);
 	if (!meta)
@@ -1777,7 +1776,7 @@ static void *rb_range_buffer(struct ring_buffer_per_cpu *cpu_buffer, int idx)
 
 	ptr = (unsigned long)rb_subbufs_from_meta(meta);
 
-	ptr += subbuf_size * idx;
+	ptr += (unsigned long)subbuf_size * idx;
 	if (ptr + subbuf_size > cpu_buffer->buffer->range_addr_end)
 		return NULL;
 
@@ -1854,13 +1853,13 @@ 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 long 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;
+	unsigned long i;
 
 	if (!subbuf_mask)
 		return false;
@@ -2109,8 +2108,8 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
 	struct buffer_page *head_page, *orig_head, *orig_reader;
 	struct rb_validation_state state = { 0 };
 	bool skip = false;
+	unsigned long i;
 	int ret;
-	int i;
 
 	if (!meta || !meta->head_buffer)
 		return;
@@ -2161,7 +2160,7 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
 		rb_validate_buffer(head_page, cpu_buffer, meta, &state, 0, state.ts);
 	}
 	if (i)
-		pr_info("Ring buffer [%d] rewound %d pages\n", cpu_buffer->cpu, i);
+		pr_info("Ring buffer [%d] rewound %lu pages\n", cpu_buffer->cpu, i);
 
 	/* The last rewound page must be skipped. */
 	if (head_page != orig_head)
@@ -2245,7 +2244,8 @@ 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 long nr_pages, int scratch_size)
 {
 	struct ring_buffer_cpu_meta *meta;
 	unsigned long *subbuf_mask;
@@ -2345,8 +2345,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;
 	}
 
@@ -2431,7 +2431,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 long nr_pages, struct list_head *pages)
 {
 	struct trace_buffer *buffer = cpu_buffer->buffer;
 	struct ring_buffer_cpu_meta *meta = NULL;
@@ -2559,7 +2559,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 long nr_pages, int cpu)
 {
 	struct ring_buffer_per_cpu *cpu_buffer __free(kfree) =
 		alloc_cpu_buffer(cpu);
@@ -2716,8 +2716,8 @@ static void rb_test_inject_invalid_pages(struct trace_buffer *buffer)
 	struct ring_buffer_cpu_meta *meta;
 	struct buffer_data_page *dpage;
 	unsigned long entry_bytes = 0;
+	unsigned int subbuf_size;
 	unsigned long ptr;
-	int subbuf_size;
 	int invalid = 0;
 	int cpu;
 	int i;
@@ -2787,8 +2787,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 int subbuf_size;
+	unsigned long nr_pages;
 	int bsize;
 	int cpu;
 	int ret;
@@ -5875,12 +5875,12 @@ __rb_get_reader_page_from_remote(struct ring_buffer_per_cpu *cpu_buffer)
 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 max_loops = cpu_buffer->ring_meta ? cpu_buffer->nr_pages : 3;
 	struct buffer_page *reader = NULL;
+	unsigned long nr_loops = 0;
 	unsigned long overwrite;
 	unsigned long flags;
 	int missed_events = 0;
-	int nr_loops = 0;
 	bool ret;
 
 	local_irq_save(flags);
@@ -6198,8 +6198,8 @@ rb_iter_peek(struct ring_buffer_iter *iter, u64 *ts)
 	struct trace_buffer *buffer;
 	struct ring_buffer_per_cpu *cpu_buffer;
 	struct ring_buffer_event *event;
-	int nr_loops = 0;
-	int max_loops;
+	unsigned long nr_loops = 0;
+	unsigned long max_loops;
 
 	if (ts)
 		*ts = 0;
@@ -7439,8 +7439,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 long nr_pages;
 	int old_order;
-	int nr_pages;
 	int psize;
 	int err;
 	int cpu;
@@ -7622,10 +7622,10 @@ static void rb_setup_ids_meta_page(struct ring_buffer_per_cpu *cpu_buffer,
 				   struct buffer_page **subbuf_ids)
 {
 	struct trace_buffer_meta *meta = cpu_buffer->meta_page;
-	unsigned int nr_subbufs = cpu_buffer->nr_pages + 1;
+	unsigned long nr_subbufs = cpu_buffer->nr_pages + 1;
 	struct buffer_page *first_subbuf, *subbuf;
-	int cnt = 0;
-	int id = 0;
+	unsigned int cnt = 0;
+	unsigned int id = 0;
 
 	id = rb_page_id(cpu_buffer, cpu_buffer->reader_page, id);
 	subbuf_ids[id++] = cpu_buffer->reader_page;
-- 
2.55.0.897.gb25b4bd76c-goog

Re: [PATCH v9 3/4] ring-buffer: Cap static ring buffer nr_pages

[sashiko-bot]

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

Pre-existing issues:
- [High] Potential integer underflow in nr_pages calculation is not caught by the new static limit on 32-bit systems, allowing bounds-check bypass and memory corruption.
--

commit bfd768f8d8a3d8fc68b36abf088742ebd0904911
Author: Vincent Donnefort <vdonnefort@google.com>

ring-buffer: Cap static ring buffer nr_pages

This commit introduces a limit on the number of pages for static ring
buffers to ensure they fit into the 31-bit bpage::id field. The limit is
enforced on ring buffer creation or user-mapping.

> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c

[ ... ]

> @@ -2842,6 +2851,10 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
>  		 */
>  		nr_pages = (size - sizeof(struct ring_buffer_cpu_meta)) /
>  			(subbuf_size + sizeof(int));
> +
> +		if (nr_pages > rb_static_max_pages())
> +			goto fail_free_buffers;
> +

[Severity: High]
This is a pre-existing issue, but does this new check fully protect 32-bit
systems from an integer underflow here?

If an administrator specifies a small ring buffer size (e.g., < 40 bytes)
via the trace_instance boot parameter, the calculation
size - sizeof(struct ring_buffer_cpu_meta) will underflow.

On 32-bit systems, this underflow results in a huge unsigned value that,
when divided by subbuf_size, yields an nr_pages value around 1,047,552.
This easily passes the new rb_static_max_pages() limit of (1UL << 31) - 1.

Later in alloc_buffer(), the pointer calculation wraps around the 32-bit
address space:

        for (n = 0, ptr = buffers_start; n < nr_cpu_ids; n++) {
                ptr += sizeof(struct ring_buffer_cpu_meta) +
                        sizeof(int) * nr_pages;
                ptr = ALIGN(ptr, subbuf_size);
                ptr += subbuf_size * nr_pages;
        }
        if (ptr > end) {

Because of the wrap around, it bypasses the physical bounds check 
if (ptr > end) and proceeds to rb_range_meta_init(), which then executes 
a negative-length memset and panics the kernel.

While this patch successfully catches the underflow on 64-bit systems
because the massive nr_pages exceeds the 31-bit limit, 32-bit systems
remain exposed. Could we add an explicit check to ensure size is larger
than sizeof(struct ring_buffer_cpu_meta) before the subtraction?

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

Re: [PATCH v9 4/4] ring-buffer: Prevent truncation of nr_pages / nr_subbufs

[sashiko-bot]

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

Pre-existing issues:
- [High] Heap buffer overflow in rb_cpu_meta_valid due to unvalidated meta->nr_subbufs
--

commit 1ffddb5e98b6cd5a5142af30014e0d2a25f757f8
Author: Vincent Donnefort <vdonnefort@google.com>

ring-buffer: Prevent truncation of nr_pages / nr_subbufs

This commit updates the variables tracking ring buffer pages and sub-buffers
from 32-bit types to unsigned long or unsigned int to prevent truncation
on very large buffers.

> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -1854,13 +1853,13 @@ 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 long 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;
> +	unsigned long i;
>  
>  	if (!subbuf_mask)
>  		return false;

[Severity: High]
This is a pre-existing issue, but does this function overflow the
subbuf_mask bitmap?

The size of subbuf_mask is allocated as nr_pages + 1 bits in
rb_range_meta_init(). However, this function unconditionally uses
meta->nr_subbufs from the persistent memory mapped region without
validating it against nr_pages + 1.

If nr_cpu_ids is increased between boots (e.g. by changing the CPU count
or maxcpus= boot parameter), the per-CPU buffer size shrinks, leading to a
smaller nr_pages for the current boot. rb_meta_init() will still accept
the persistent memory as valid because the overall total_size is identical.

Consequently, meta->nr_subbufs from the previous boot will be larger than
the newly allocated subbuf_mask size, which can cause bitmap_clear() and
set_bit() to write out of bounds further down in this function:

    bitmap_clear(subbuf_mask, 0, meta->nr_subbufs);

    for (i = 0; i < meta->nr_subbufs; i++) {
        ...
        set_bit(meta->buffers[i], subbuf_mask);
    }

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

Re: [PATCH v9 0/4] ring-buffer: Fixes for subbuf resizing and persistent buffers

[Steven Rostedt]

On Tue,  1 Sep 2026 16:54:41 +0100
Vincent Donnefort <vdonnefort@google.com> wrote:

> This series addresses multiple issues discovered with the dynamic ring
> buffer resizing.
> 
> I have managed to reproduce a ring_buffer_read_page() race with
> 
>  $ while true; do for i in 8 16 32; do echo $i > /sys/kernel/tracing/buffer_subbuf_size_kb; sleep 0.1; done; done

Did you do anything else while running the above?

I ask because I ran this for several hours and it didn't produce anything.
What bug does it supposed to trigger?

-- Steve

Re: [PATCH v9 0/4] ring-buffer: Fixes for subbuf resizing and persistent buffers

[Vincent Donnefort]

On Thu, Sep 03, 2026 at 10:31:16AM -0400, Steven Rostedt wrote:
> On Tue,  1 Sep 2026 16:54:41 +0100
> Vincent Donnefort <vdonnefort@google.com> wrote:
> 
> > This series addresses multiple issues discovered with the dynamic ring
> > buffer resizing.
> > 
> > I have managed to reproduce a ring_buffer_read_page() race with
> > 
> >  $ while true; do for i in 8 16 32; do echo $i > /sys/kernel/tracing/buffer_subbuf_size_kb; sleep 0.1; done; done
> 
> Did you do anything else while running the above?
> 
> I ask because I ran this for several hours and it didn't produce anything.
> What bug does it supposed to trigger?
> 
> -- Steve

I clearly have been imprecise here.

I had "trace-record -e sched" and I have just verified with a print that it has
run into the reader->order != read_page->order case.

-- 
Vincent

Re: [PATCH v9 2/4] tracing: Fix subbuf resize races with trace_pipe_raw readers

[Steven Rostedt]

On Tue,  1 Sep 2026 16:54:43 +0100
Vincent Donnefort <vdonnefort@google.com> wrote:

> 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 handle the resizing of an
> existing buffer_data_read_page if necessary and add a new
> ring_buffer_read_page_size(). This new function enables ring-buffer
> buffer_data_read_page users to not call the racy
> ring_buffer_subbuf_size_get(). This makes the spare_size member of
> ftrace_buffer_info redundant.
> 
> Finally, handle buffer_data_read_page/reader_page order discrepancy in
> ring_buffer_read_page(). On a mismatch simply copy manually the data to
> the buffer_data_read_page.

Let's add here:

 Link: https://lore.kernel.org/all/20260817140812.2C7D41F00A3A@smtp.kernel.org/

As it has more information about why we came up with this solution.

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



> -struct buffer_data_read_page *
> -ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu)
> +int ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu,
> +				struct buffer_data_read_page **rpage)
>  {
>  	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);
> +		return -ENODEV;
>  
> -	bpage = kzalloc_obj(*bpage);
> -	if (!bpage)
> -		return ERR_PTR(-ENOMEM);
> +	if (!rpage)
> +		return -EINVAL;
>  
> -	bpage->order = buffer->subbuf_order;
> +	order = buffer->subbuf_order;

Hmm, should we add a READ_ONCE() around the subbuf_order? There's no locks
taken here and couldn't we get some inconsistency if things change. I feel
more comfortable knowing that "order" is consistent throughout this
function.

> +
> +	if (*rpage) {
> +		if ((*rpage)->order == order)
> +			return 0;
> +
> +		/* We can reuse rpage, but we discard the payload */
> +		free_pages((unsigned long)(*rpage)->data, (*rpage)->order);
> +		(*rpage)->data = NULL;
> +	} else {
> +		*rpage = kzalloc_obj(**rpage);
> +		if (!*rpage)
> +			return -ENOMEM;
> +	}
> +
> +	(*rpage)->order = order;
>  	cpu_buffer = buffer->buffers[cpu];
> +
>  	local_irq_save(flags);
>  	arch_spin_lock(&cpu_buffer->lock);
>  
>  	if (cpu_buffer->free_page.data) {
> -		*bpage = cpu_buffer->free_page;
> +		**rpage = cpu_buffer->free_page;
>  		cpu_buffer->free_page.data = NULL;
>  	}
>  
>  	arch_spin_unlock(&cpu_buffer->lock);
>  	local_irq_restore(flags);
>  
> -	if (bpage->data) {
> -		rb_init_data_page(bpage->data);
> +	if ((*rpage)->data) {
> +		rb_init_data_page((*rpage)->data);
>  	} else {
> -		bpage->data = alloc_cpu_data(cpu, bpage->order);
> -		if (!bpage->data) {
> -			kfree(bpage);
> -			return ERR_PTR(-ENOMEM);
> +		(*rpage)->data = alloc_cpu_data(cpu, (*rpage)->order);
> +		if (!(*rpage)->data) {
> +			kfree(*rpage);
> +			*rpage = NULL;
> +			return -ENOMEM;
>  		}
>  	}
>  
> -	return bpage;
> +	return 0;
>  }
>  EXPORT_SYMBOL_GPL(ring_buffer_alloc_read_page);
>  
> @@ -7050,21 +7077,30 @@ EXPORT_SYMBOL_GPL(ring_buffer_alloc_read_page);
>   * ring_buffer_free_read_page - free an allocated read page
>   * @buffer: the buffer the page was allocate for
>   * @cpu: the cpu buffer the page came from
> - * @data_page: the page to free
> + * @rpage: the buffer_dat_read_page to free
>   *
>   * Free a page allocated from ring_buffer_alloc_read_page.
>   */
>  void ring_buffer_free_read_page(struct trace_buffer *buffer, int cpu,
> -				struct buffer_data_read_page *data_page)
> +				struct buffer_data_read_page *rpage)
>  {
>  	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 (!rpage)
> +		return;
> +
> +	dpage = rpage->data;
> +	if (!dpage)
> +		goto out;
> +
> +	page = virt_to_page(dpage);
> +
>  	cpu_buffer = buffer->buffers[cpu];
>  
>  	/*
> @@ -7072,14 +7108,14 @@ void ring_buffer_free_read_page(struct trace_buffer *buffer, int cpu,
>  	 * is different from the subbuffer order of the buffer -
>  	 * we can't reuse it
>  	 */
> -	if (page_ref_count(page) > 1 || data_page->order != buffer->subbuf_order)
> +	if (page_ref_count(page) > 1 || rpage->order != buffer->subbuf_order)
>  		goto out;
>  
>  	local_irq_save(flags);
>  	arch_spin_lock(&cpu_buffer->lock);
>  
>  	if (!cpu_buffer->free_page.data) {
> -		cpu_buffer->free_page = *data_page;
> +		cpu_buffer->free_page = *rpage;
>  		dpage = NULL;
>  	}
>  
> @@ -7087,8 +7123,8 @@ void ring_buffer_free_read_page(struct trace_buffer *buffer, int cpu,
>  	local_irq_restore(flags);
>  
>   out:
> -	free_pages((unsigned long)dpage, data_page->order);
> -	kfree(data_page);
> +	free_pages((unsigned long)dpage, rpage->order);
> +	kfree(rpage);
>  }
>  EXPORT_SYMBOL_GPL(ring_buffer_free_read_page);
>  
> @@ -7159,10 +7195,9 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
>  	if (!dpage)
>  		return -1;
>  
> -	guard(raw_spinlock_irqsave)(&cpu_buffer->reader_lock);
> +	len = min_t(size_t, len, rb_read_page_capacity(data_page));
>  
> -	if (data_page->order != cpu_buffer->reader_page->order)
> -		return -1;
> +	guard(raw_spinlock_irqsave)(&cpu_buffer->reader_lock);
>  
>  	reader = rb_get_reader_page(cpu_buffer);
>  	if (!reader)
> @@ -7177,16 +7212,18 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
>  	/* Check if any events were dropped */
>  	missed_events = cpu_buffer->lost_events;
>  
> -	/*
> -	 * If this page has been partially read or
> -	 * if len is not big enough to read the rest of the page or
> -	 * a writer is still on the page, then
> -	 * we must copy the data from the page to the buffer.
> -	 * Otherwise, we can simply swap the page with the one passed in.
> -	 */
> +	 /*
> +	  * It is not possible to swap the reader page if:
> +	  *   - It has been partially read
> +	  *   - len is not big enough to read it entirely
> +	  *   - A writer is still on it
> +	  *   - The ring buffer is static
> +	  *   - The order doesn't match
> +	  */
>  	if (read || (len < (size - read)) ||
>  	    cpu_buffer->reader_page == cpu_buffer->commit_page ||
> -	    rb_is_static(cpu_buffer)) {
> +	    rb_is_static(cpu_buffer) ||
> +	    data_page->order != reader->order) {
>  		struct buffer_data_page *rpage = cpu_buffer->reader_page->page;
>  		unsigned int rpos = read;
>  		unsigned int pos = 0;
> @@ -7280,7 +7317,7 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
>  		 * missed events, then record it there.
>  		 */
>  		if (missed_events > 0 &&
> -		    rb_page_capacity(reader) - size >= sizeof(missed_events)) {
> +		    rb_read_page_capacity(data_page) - size >= sizeof(missed_events)) {
>  			memcpy(&dpage->data[size], &missed_events,
>  			       sizeof(missed_events));
>  			local_add(RB_MISSED_STORED, &dpage->commit);
> @@ -7300,8 +7337,8 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
>  	/*
>  	 * This page may be off to user land. Zero it out here.
>  	 */
> -	if (size < rb_page_capacity(reader))
> -		memset(&dpage->data[size], 0, rb_page_capacity(reader) - size);
> +	if (size < rb_read_page_capacity(data_page))
> +		memset(&dpage->data[size], 0, rb_read_page_capacity(data_page) - size);
>  
>  	return read;
>  }
> @@ -7319,6 +7356,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 *rpage)
> +{
> +	return PAGE_SIZE << rpage->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..c3d34c0e64e2 100644
> --- a/kernel/trace/ring_buffer_benchmark.c
> +++ b/kernel/trace/ring_buffer_benchmark.c
> @@ -104,7 +104,7 @@ static enum event_status read_event(int cpu)
>  
>  static enum event_status read_page(int cpu)
>  {
> -	struct buffer_data_read_page *bpage;
> +	struct buffer_data_read_page *bpage = NULL;
>  	struct ring_buffer_event *event;
>  	struct rb_page *rpage;
>  	unsigned long commit;
> @@ -114,8 +114,8 @@ static enum event_status read_page(int cpu)
>  	int inc;
>  	int i;
>  
> -	bpage = ring_buffer_alloc_read_page(buffer, cpu);
> -	if (IS_ERR(bpage))
> +	ret = ring_buffer_alloc_read_page(buffer, cpu, &bpage);
> +	if (ret < 0)
>  		return EVENT_DROPPED;
>  
>  	page_size = ring_buffer_subbuf_size_get(buffer);
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index a946e0183fd1..7ba3856daf44 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -7082,8 +7082,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;
> +	unsigned int spare_size;
>  	void *trace_data;
> -	int page_size;
>  	ssize_t ret = 0;
>  	ssize_t size;
>  
> @@ -7093,36 +7093,24 @@ 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);
> -
> -	/* Make sure the spare matches the current sub buffer size */
> +again:
>  	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;
> -		}
> +		spare_size = ring_buffer_read_page_size(info->spare);
> +		/* Do we have previous read data to read? */
> +		if (info->read < spare_size)
> +			goto read;
>  	}

I would have ring_buffer_free_read_page() accept a null pointer and then here do:

	spare_size = ring_buffer_read_page_size(info->spare);

 again:
	/* Do we have previous read data to read? */
	if (info->read < spare_size)
		goto read;

As the jump to here below has already calculated the spare_size, why do it again?

Have ring_buffer_read_page_size() be:

unsigned int ring_buffer_read_page_size(struct buffer_data_read_page *rpage)
{
	return rpage ? PAGE_SIZE << rpage->order : 0;
}

Then info->read could not be less than spare_size if there was no spare.

>  
> -	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 subbuf order */

The above comment doesn't really make sense anymore since the user here
should not care about the order. I would nuke it.

> +	ret = ring_buffer_alloc_read_page(iter->array_buffer->buffer, iter->cpu_file,
> +					  &info->spare);
> +	if (ret)
>  		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,
> @@ -7148,8 +7136,9 @@ ssize_t tracing_buffers_read(struct file *filp, char __user *ubuf,
>  	}
>  
>  	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);
> @@ -7199,17 +7188,17 @@ int tracing_buffers_release(struct inode *inode, struct file *file)
>  }
>  
>  struct buffer_ref {
> -	struct trace_buffer	*buffer;
> -	void			*page;
> -	int			cpu;
> -	refcount_t		refcount;
> +	struct trace_buffer		*buffer;
> +	struct buffer_data_read_page	*rpage;
> +	int				cpu;
> +	refcount_t			refcount;
>  };
>  
>  static void buffer_ref_release(struct buffer_ref *ref)
>  {
>  	if (!refcount_dec_and_test(&ref->refcount))
>  		return;
> -	ring_buffer_free_read_page(ref->buffer, ref->cpu, ref->page);
> +	ring_buffer_free_read_page(ref->buffer, ref->cpu, ref->rpage);
>  	kfree(ref);
>  }
>  
> @@ -7270,23 +7259,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));
> -	}

OK, you are removing this so that it is tested in the loop?

> -
>  	if (splice_grow_spd(pipe, &spd))
>  		return -ENOMEM;
>  
> @@ -7294,7 +7272,8 @@ 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++) {

Is there a reason you moved the len -= page_size from here to the end of
the loop?  Basically that has no functional change.

> +		unsigned int page_size;

Was that just to move page_size here?

Let's keep it as-is.

>  		struct page *page;
>  		int r;
>  
> @@ -7306,25 +7285,36 @@ 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);
> -		if (IS_ERR(ref->page)) {
> -			ret = PTR_ERR(ref->page);
> -			ref->page = NULL;
> +
> +		ret = ring_buffer_alloc_read_page(ref->buffer, iter->cpu_file, &ref->rpage);
> +		if (ret) {
>  			kfree(ref);
>  			break;
>  		}
>  		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->rpage);
> +
> +		r = -EINVAL;
> +		if (IS_ALIGNED(*ppos, page_size) && len >= page_size) {
> +			r = ring_buffer_read_page(ref->buffer, ref->rpage, 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;

This is more likely to be true not because the subbuf order was modified,
but also if the length was not a multiple of page_size. From the code you
removed;

-	if (len & (page_size - 1)) {
-		if (len < page_size)
-			return -EINVAL;
-		len &= (~(page_size - 1));
-	}

It would error if len was smaller than page_size but otherwise it would
modify len to be a multiple of page_size.

The overall behavior is the same, but the comment needs to be updated.

-- Steve

> +		}
> +
>  		if (r < 0) {
> -			ring_buffer_free_read_page(ref->buffer, ref->cpu,
> -						   ref->page);
> +			ring_buffer_free_read_page(ref->buffer, ref->cpu, ref->rpage);
>  			kfree(ref);
>  			break;
>  		}
>  
> -		page = virt_to_page(ring_buffer_read_page_data(ref->page));
> +		page = virt_to_page(ring_buffer_read_page_data(ref->rpage));
>  
>  		spd.pages[i] = page;
>  		spd.partial[i].len = page_size;
> @@ -7332,6 +7322,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 74a7a50d1e78..203d098ee14e 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -745,11 +745,10 @@ static inline int tracing_get_cpu(struct inode *inode)
>  void tracing_reset_cpu(struct array_buffer *buf, int cpu);
>  
>  struct ftrace_buffer_info {
> -	struct trace_iterator	iter;
> -	void			*spare;
> -	unsigned int		spare_cpu;
> -	unsigned int		spare_size;
> -	unsigned int		read;
> +	struct trace_iterator		iter;
> +	struct buffer_data_read_page	*spare;
> +	unsigned int			spare_cpu;
> +	unsigned int			read;
>  };
>  
>  /**

Re: [PATCH v9 3/4] ring-buffer: Cap static ring buffer nr_pages

[Steven Rostedt]

On Tue,  1 Sep 2026 16:54:44 +0100
Vincent Donnefort <vdonnefort@google.com> wrote:

> Static ring buffers (i.e. persistent, user-mapped and remote) rely on
> the bpage::id field. The number of pages for those ring buffers must fit
> into that variable. Enforce this limit on ring buffer creation or
> user-mapping.
> 
> While at it, make buffer_page::id 31 bits. This does not change the
> struct buffer_page size.

Let's not add that change to this patch. Especially since this has a fixes
tag to it. That change has nothing to do with the fix.

The reason I had it as 30 to begin with was to reserve a bit in case I
found another reason for it. If 1<<30 is too small for the number of boot
buffer pages, we can always up in another order in the future.

> 
> Fixes: be68d63a139b ("ring-buffer: Add ring_buffer_alloc_range()")
> Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
> 
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 28dd76edfecf..c4260d6ecdfc 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -350,7 +350,7 @@ struct buffer_page {
>  	local_t		 entries;	/* entries on this page */
>  	unsigned long	 real_end;	/* real end of data */
>  	unsigned	 order;		/* order of the page */
> -	u32		 id:30;		/* ID for external mapping */
> +	u32		 id:31;		/* ID for external mapping */
>  	u32		 range:1;	/* Mapped via a range */
>  	struct buffer_data_page *page;	/* Actual data page */
>  };
> @@ -657,6 +657,15 @@ static bool rb_is_static(struct ring_buffer_per_cpu *cpu_buffer)
>  	return cpu_buffer->user_mapped || cpu_buffer->remote || cpu_buffer->ring_meta;
>  }
>  
> +static unsigned long rb_static_max_pages(void)
> +{
> +	/*
> +	 * Static ring buffers are using bpage::id and must account for the
> +	 * reader page.
> +	 */
> +	return (1UL << 31) - 1;
> +}
> +
>  struct ring_buffer_iter {
>  	struct ring_buffer_per_cpu	*cpu_buffer;
>  	unsigned long			head;
> @@ -2842,6 +2851,10 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
>  		 */
>  		nr_pages = (size - sizeof(struct ring_buffer_cpu_meta)) /
>  			(subbuf_size + sizeof(int));
> +
> +		if (nr_pages > rb_static_max_pages())
> +			goto fail_free_buffers;
> +

If you want to add something, we could add to the beginning of this
function:

	/* Prevent ridiculously small sizes */
	if (size < PAGE_SIZE)
		return NULL;

to shut up Sashiko about overflows :-p

-- Steve



>  		/* Need at least two pages plus the reader page */
>  		if (nr_pages < 3)
>  			goto fail_free_buffers;
> @@ -2874,6 +2887,10 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
>  		/* The writer is remote. This ring-buffer is read-only */
>  		atomic_inc(&buffer->record_disabled);
>  		nr_pages = desc->nr_page_va - 1;
> +
> +		if (nr_pages > rb_static_max_pages())
> +			goto fail_free_buffers;
> +
>  		if (nr_pages < 2)
>  			goto fail_free_buffers;
>  	} else {
> @@ -7836,6 +7853,9 @@ int ring_buffer_map(struct trace_buffer *buffer, int cpu,
>  	/* prevent another thread from changing buffer/sub-buffer sizes */
>  	guard(mutex)(&buffer->mutex);
>  
> +	if (cpu_buffer->nr_pages > rb_static_max_pages())
> +		return -E2BIG;
> +
>  	err = rb_alloc_meta_page(cpu_buffer);
>  	if (err)
>  		return err;

Re: [PATCH v9 3/4] ring-buffer: Cap static ring buffer nr_pages

[Vincent Donnefort]

On Thu, Sep 03, 2026 at 12:56:21PM -0400, Steven Rostedt wrote:
> On Tue,  1 Sep 2026 16:54:44 +0100
> Vincent Donnefort <vdonnefort@google.com> wrote:
> 
> > Static ring buffers (i.e. persistent, user-mapped and remote) rely on
> > the bpage::id field. The number of pages for those ring buffers must fit
> > into that variable. Enforce this limit on ring buffer creation or
> > user-mapping.
> > 
> > While at it, make buffer_page::id 31 bits. This does not change the
> > struct buffer_page size.
> 
> Let's not add that change to this patch. Especially since this has a fixes
> tag to it. That change has nothing to do with the fix.
> 
> The reason I had it as 30 to begin with was to reserve a bit in case I
> found another reason for it. If 1<<30 is too small for the number of boot
> buffer pages, we can always up in another order in the future.

ack

> 
> > 
> > Fixes: be68d63a139b ("ring-buffer: Add ring_buffer_alloc_range()")
> > Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
> > 
> > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> > index 28dd76edfecf..c4260d6ecdfc 100644
> > --- a/kernel/trace/ring_buffer.c
> > +++ b/kernel/trace/ring_buffer.c
> > @@ -350,7 +350,7 @@ struct buffer_page {
> >  	local_t		 entries;	/* entries on this page */
> >  	unsigned long	 real_end;	/* real end of data */
> >  	unsigned	 order;		/* order of the page */
> > -	u32		 id:30;		/* ID for external mapping */
> > +	u32		 id:31;		/* ID for external mapping */
> >  	u32		 range:1;	/* Mapped via a range */
> >  	struct buffer_data_page *page;	/* Actual data page */
> >  };
> > @@ -657,6 +657,15 @@ static bool rb_is_static(struct ring_buffer_per_cpu *cpu_buffer)
> >  	return cpu_buffer->user_mapped || cpu_buffer->remote || cpu_buffer->ring_meta;
> >  }
> >  
> > +static unsigned long rb_static_max_pages(void)
> > +{
> > +	/*
> > +	 * Static ring buffers are using bpage::id and must account for the
> > +	 * reader page.
> > +	 */
> > +	return (1UL << 31) - 1;
> > +}
> > +
> >  struct ring_buffer_iter {
> >  	struct ring_buffer_per_cpu	*cpu_buffer;
> >  	unsigned long			head;
> > @@ -2842,6 +2851,10 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
> >  		 */
> >  		nr_pages = (size - sizeof(struct ring_buffer_cpu_meta)) /
> >  			(subbuf_size + sizeof(int));
> > +
> > +		if (nr_pages > rb_static_max_pages())
> > +			goto fail_free_buffers;
> > +
> 
> If you want to add something, we could add to the beginning of this
> function:
> 
> 	/* Prevent ridiculously small sizes */
> 	if (size < PAGE_SIZE)
> 		return NULL;
> 
> to shut up Sashiko about overflows :-p

Yeah, I wasn't sure... It's so pedantic, we have no user with a small size :-(

But that would make sense to fold that into this patch, right?

> 
> -- Steve
> 
> 
> 
> >  		/* Need at least two pages plus the reader page */
> >  		if (nr_pages < 3)
> >  			goto fail_free_buffers;
> > @@ -2874,6 +2887,10 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
> >  		/* The writer is remote. This ring-buffer is read-only */
> >  		atomic_inc(&buffer->record_disabled);
> >  		nr_pages = desc->nr_page_va - 1;
> > +
> > +		if (nr_pages > rb_static_max_pages())
> > +			goto fail_free_buffers;
> > +
> >  		if (nr_pages < 2)
> >  			goto fail_free_buffers;
> >  	} else {
> > @@ -7836,6 +7853,9 @@ int ring_buffer_map(struct trace_buffer *buffer, int cpu,
> >  	/* prevent another thread from changing buffer/sub-buffer sizes */
> >  	guard(mutex)(&buffer->mutex);
> >  
> > +	if (cpu_buffer->nr_pages > rb_static_max_pages())
> > +		return -E2BIG;
> > +
> >  	err = rb_alloc_meta_page(cpu_buffer);
> >  	if (err)
> >  		return err;
> 

Re: [PATCH v9 4/4] ring-buffer: Prevent truncation of nr_pages / nr_subbufs

[Steven Rostedt]

On Tue,  1 Sep 2026 16:54:45 +0100
Vincent Donnefort <vdonnefort@google.com> wrote:

> -static void *rb_range_buffer(struct ring_buffer_per_cpu *cpu_buffer, int idx)
> +static void *rb_range_buffer(struct ring_buffer_per_cpu *cpu_buffer, unsigned int idx)
>  {
>  	struct ring_buffer_cpu_meta *meta;
> +	unsigned int subbuf_size;
>  	unsigned long ptr;
> -	int subbuf_size;
>  
>  	meta = rb_range_meta(cpu_buffer->buffer, 0, cpu_buffer->cpu);
>  	if (!meta)
> @@ -1777,7 +1776,7 @@ static void *rb_range_buffer(struct ring_buffer_per_cpu *cpu_buffer, int idx)
>  
>  	ptr = (unsigned long)rb_subbufs_from_meta(meta);
>  
> -	ptr += subbuf_size * idx;
> +	ptr += (unsigned long)subbuf_size * idx;

Really, it looks like the idx should be typecasted, as it is the number of
subbuffers. Maybe even pass it in as unsigned long?

>  	if (ptr + subbuf_size > cpu_buffer->buffer->range_addr_end)
>  		return NULL;
>  
> @@ -1854,13 +1853,13 @@ 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 long nr_pages,
>  			      unsigned long *subbuf_mask)
>  {
> -	int subbuf_size = PAGE_SIZE;
> +	unsigned long subbuf_size = PAGE_SIZE;

Why the long? Shouldn't it be unsigned int?

>  	unsigned long buffers_start;
>  	unsigned long buffers_end;
> -	int i;
> +	unsigned long i;
>  
>  	if (!subbuf_mask)
>  		return false;
> @@ -2109,8 +2108,8 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
>  	struct buffer_page *head_page, *orig_head, *orig_reader;
>  	struct rb_validation_state state = { 0 };
>  	bool skip = false;
> +	unsigned long i;
>  	int ret;
> -	int i;
>  
>  	if (!meta || !meta->head_buffer)
>  		return;
> @@ -2161,7 +2160,7 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
>  		rb_validate_buffer(head_page, cpu_buffer, meta, &state, 0, state.ts);
>  	}
>  	if (i)
> -		pr_info("Ring buffer [%d] rewound %d pages\n", cpu_buffer->cpu, i);
> +		pr_info("Ring buffer [%d] rewound %lu pages\n", cpu_buffer->cpu, i);
>  
>  	/* The last rewound page must be skipped. */
>  	if (head_page != orig_head)
> @@ -2245,7 +2244,8 @@ 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 long nr_pages, int scratch_size)

static void rb_range_meta_init(struct trace_buffer *buffer, unsigned long nr_pages,
			       int scratch_size)

looks better ;-)

>  {
>  	struct ring_buffer_cpu_meta *meta;
>  	unsigned long *subbuf_mask;
> @@ -2345,8 +2345,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;
>  	}
>  


-- Steve

Re: [PATCH v9 4/4] ring-buffer: Prevent truncation of nr_pages / nr_subbufs

[Steven Rostedt]

On Tue, 01 Sep 2026 16:48:36 +0000
sashiko-bot@kernel.org wrote:

> [Severity: High]
> This is a pre-existing issue, but does this function overflow the
> subbuf_mask bitmap?
> 
> The size of subbuf_mask is allocated as nr_pages + 1 bits in
> rb_range_meta_init(). However, this function unconditionally uses
> meta->nr_subbufs from the persistent memory mapped region without
> validating it against nr_pages + 1.

Sounds like another validation check. Make sure what's in the meta data
matches nr_pages + 1. I'll add this:

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index ef66f726e2e2..748d4ae8badf 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -1869,6 +1869,11 @@ static bool rb_cpu_meta_valid(struct ring_buffer_cpu_meta *meta, int cpu,
 		return false;
 	}
 
+	if (meta->nr_subbufs != nr_pages + 1) {
+		pr_info("Ring buffer boot meta [%d] invalid nr_subbufs\n", cpu);
+		return false;
+	}
+
 	buffers_start = meta->first_buffer;
 	buffers_end = meta->first_buffer + (subbuf_size * meta->nr_subbufs);
 
-- Steve

Re: [PATCH v9 2/4] tracing: Fix subbuf resize races with trace_pipe_raw readers

[Vincent Donnefort]

On Thu, Sep 03, 2026 at 11:48:45AM -0400, Steven Rostedt wrote:
> On Tue,  1 Sep 2026 16:54:43 +0100
> Vincent Donnefort <vdonnefort@google.com> wrote:
> 
> > 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 handle the resizing of an
> > existing buffer_data_read_page if necessary and add a new
> > ring_buffer_read_page_size(). This new function enables ring-buffer
> > buffer_data_read_page users to not call the racy
> > ring_buffer_subbuf_size_get(). This makes the spare_size member of
> > ftrace_buffer_info redundant.
> > 
> > Finally, handle buffer_data_read_page/reader_page order discrepancy in
> > ring_buffer_read_page(). On a mismatch simply copy manually the data to
> > the buffer_data_read_page.
> 
> Let's add here:
> 
>  Link: https://lore.kernel.org/all/20260817140812.2C7D41F00A3A@smtp.kernel.org/
> 
> As it has more information about why we came up with this solution.
> 
> > 
> > Fixes: bce761d75745 ("ring-buffer: Read and write to ring buffers with custom sub buffer size")
> > Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
> 
> 
> 
> > -struct buffer_data_read_page *
> > -ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu)
> > +int ring_buffer_alloc_read_page(struct trace_buffer *buffer, int cpu,
> > +				struct buffer_data_read_page **rpage)
> >  {
> >  	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);
> > +		return -ENODEV;
> >  
> > -	bpage = kzalloc_obj(*bpage);
> > -	if (!bpage)
> > -		return ERR_PTR(-ENOMEM);
> > +	if (!rpage)
> > +		return -EINVAL;
> >  
> > -	bpage->order = buffer->subbuf_order;
> > +	order = buffer->subbuf_order;
> 
> Hmm, should we add a READ_ONCE() around the subbuf_order? There's no locks
> taken here and couldn't we get some inconsistency if things change. I feel
> more comfortable knowing that "order" is consistent throughout this
> function.

Ack, I'll see if there are other lockless readers without READ_ONCE().

> 
> > +
> > +	if (*rpage) {
> > +		if ((*rpage)->order == order)
> > +			return 0;
> > +
> > +		/* We can reuse rpage, but we discard the payload */
> > +		free_pages((unsigned long)(*rpage)->data, (*rpage)->order);
> > +		(*rpage)->data = NULL;
> > +	} else {
> > +		*rpage = kzalloc_obj(**rpage);
> > +		if (!*rpage)
> > +			return -ENOMEM;
> > +	}
> > +
> > +	(*rpage)->order = order;
> >  	cpu_buffer = buffer->buffers[cpu];
> > +
> >  	local_irq_save(flags);
> >  	arch_spin_lock(&cpu_buffer->lock);
> >  
> >  	if (cpu_buffer->free_page.data) {
> > -		*bpage = cpu_buffer->free_page;
> > +		**rpage = cpu_buffer->free_page;
> >  		cpu_buffer->free_page.data = NULL;
> >  	}
> >  
> >  	arch_spin_unlock(&cpu_buffer->lock);
> >  	local_irq_restore(flags);
> >  
> > -	if (bpage->data) {
> > -		rb_init_data_page(bpage->data);
> > +	if ((*rpage)->data) {
> > +		rb_init_data_page((*rpage)->data);
> >  	} else {
> > -		bpage->data = alloc_cpu_data(cpu, bpage->order);
> > -		if (!bpage->data) {
> > -			kfree(bpage);
> > -			return ERR_PTR(-ENOMEM);
> > +		(*rpage)->data = alloc_cpu_data(cpu, (*rpage)->order);
> > +		if (!(*rpage)->data) {
> > +			kfree(*rpage);
> > +			*rpage = NULL;
> > +			return -ENOMEM;
> >  		}
> >  	}
> >  
> > -	return bpage;
> > +	return 0;
> >  }
> >  EXPORT_SYMBOL_GPL(ring_buffer_alloc_read_page);
> >  
> > @@ -7050,21 +7077,30 @@ EXPORT_SYMBOL_GPL(ring_buffer_alloc_read_page);
> >   * ring_buffer_free_read_page - free an allocated read page
> >   * @buffer: the buffer the page was allocate for
> >   * @cpu: the cpu buffer the page came from
> > - * @data_page: the page to free
> > + * @rpage: the buffer_dat_read_page to free
> >   *
> >   * Free a page allocated from ring_buffer_alloc_read_page.
> >   */
> >  void ring_buffer_free_read_page(struct trace_buffer *buffer, int cpu,
> > -				struct buffer_data_read_page *data_page)
> > +				struct buffer_data_read_page *rpage)
> >  {
> >  	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 (!rpage)
> > +		return;
> > +
> > +	dpage = rpage->data;
> > +	if (!dpage)
> > +		goto out;
> > +
> > +	page = virt_to_page(dpage);
> > +
> >  	cpu_buffer = buffer->buffers[cpu];
> >  
> >  	/*
> > @@ -7072,14 +7108,14 @@ void ring_buffer_free_read_page(struct trace_buffer *buffer, int cpu,
> >  	 * is different from the subbuffer order of the buffer -
> >  	 * we can't reuse it
> >  	 */
> > -	if (page_ref_count(page) > 1 || data_page->order != buffer->subbuf_order)
> > +	if (page_ref_count(page) > 1 || rpage->order != buffer->subbuf_order)
> >  		goto out;
> >  
> >  	local_irq_save(flags);
> >  	arch_spin_lock(&cpu_buffer->lock);
> >  
> >  	if (!cpu_buffer->free_page.data) {
> > -		cpu_buffer->free_page = *data_page;
> > +		cpu_buffer->free_page = *rpage;
> >  		dpage = NULL;
> >  	}
> >  
> > @@ -7087,8 +7123,8 @@ void ring_buffer_free_read_page(struct trace_buffer *buffer, int cpu,
> >  	local_irq_restore(flags);
> >  
> >   out:
> > -	free_pages((unsigned long)dpage, data_page->order);
> > -	kfree(data_page);
> > +	free_pages((unsigned long)dpage, rpage->order);
> > +	kfree(rpage);
> >  }
> >  EXPORT_SYMBOL_GPL(ring_buffer_free_read_page);
> >  
> > @@ -7159,10 +7195,9 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
> >  	if (!dpage)
> >  		return -1;
> >  
> > -	guard(raw_spinlock_irqsave)(&cpu_buffer->reader_lock);
> > +	len = min_t(size_t, len, rb_read_page_capacity(data_page));
> >  
> > -	if (data_page->order != cpu_buffer->reader_page->order)
> > -		return -1;
> > +	guard(raw_spinlock_irqsave)(&cpu_buffer->reader_lock);
> >  
> >  	reader = rb_get_reader_page(cpu_buffer);
> >  	if (!reader)
> > @@ -7177,16 +7212,18 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
> >  	/* Check if any events were dropped */
> >  	missed_events = cpu_buffer->lost_events;
> >  
> > -	/*
> > -	 * If this page has been partially read or
> > -	 * if len is not big enough to read the rest of the page or
> > -	 * a writer is still on the page, then
> > -	 * we must copy the data from the page to the buffer.
> > -	 * Otherwise, we can simply swap the page with the one passed in.
> > -	 */
> > +	 /*
> > +	  * It is not possible to swap the reader page if:
> > +	  *   - It has been partially read
> > +	  *   - len is not big enough to read it entirely
> > +	  *   - A writer is still on it
> > +	  *   - The ring buffer is static
> > +	  *   - The order doesn't match
> > +	  */
> >  	if (read || (len < (size - read)) ||
> >  	    cpu_buffer->reader_page == cpu_buffer->commit_page ||
> > -	    rb_is_static(cpu_buffer)) {
> > +	    rb_is_static(cpu_buffer) ||
> > +	    data_page->order != reader->order) {
> >  		struct buffer_data_page *rpage = cpu_buffer->reader_page->page;
> >  		unsigned int rpos = read;
> >  		unsigned int pos = 0;
> > @@ -7280,7 +7317,7 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
> >  		 * missed events, then record it there.
> >  		 */
> >  		if (missed_events > 0 &&
> > -		    rb_page_capacity(reader) - size >= sizeof(missed_events)) {
> > +		    rb_read_page_capacity(data_page) - size >= sizeof(missed_events)) {
> >  			memcpy(&dpage->data[size], &missed_events,
> >  			       sizeof(missed_events));
> >  			local_add(RB_MISSED_STORED, &dpage->commit);
> > @@ -7300,8 +7337,8 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
> >  	/*
> >  	 * This page may be off to user land. Zero it out here.
> >  	 */
> > -	if (size < rb_page_capacity(reader))
> > -		memset(&dpage->data[size], 0, rb_page_capacity(reader) - size);
> > +	if (size < rb_read_page_capacity(data_page))
> > +		memset(&dpage->data[size], 0, rb_read_page_capacity(data_page) - size);
> >  
> >  	return read;
> >  }
> > @@ -7319,6 +7356,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 *rpage)
> > +{
> > +	return PAGE_SIZE << rpage->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..c3d34c0e64e2 100644
> > --- a/kernel/trace/ring_buffer_benchmark.c
> > +++ b/kernel/trace/ring_buffer_benchmark.c
> > @@ -104,7 +104,7 @@ static enum event_status read_event(int cpu)
> >  
> >  static enum event_status read_page(int cpu)
> >  {
> > -	struct buffer_data_read_page *bpage;
> > +	struct buffer_data_read_page *bpage = NULL;
> >  	struct ring_buffer_event *event;
> >  	struct rb_page *rpage;
> >  	unsigned long commit;
> > @@ -114,8 +114,8 @@ static enum event_status read_page(int cpu)
> >  	int inc;
> >  	int i;
> >  
> > -	bpage = ring_buffer_alloc_read_page(buffer, cpu);
> > -	if (IS_ERR(bpage))
> > +	ret = ring_buffer_alloc_read_page(buffer, cpu, &bpage);
> > +	if (ret < 0)
> >  		return EVENT_DROPPED;
> >  
> >  	page_size = ring_buffer_subbuf_size_get(buffer);
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index a946e0183fd1..7ba3856daf44 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -7082,8 +7082,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;
> > +	unsigned int spare_size;
> >  	void *trace_data;
> > -	int page_size;
> >  	ssize_t ret = 0;
> >  	ssize_t size;
> >  
> > @@ -7093,36 +7093,24 @@ 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);
> > -
> > -	/* Make sure the spare matches the current sub buffer size */
> > +again:
> >  	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;
> > -		}
> > +		spare_size = ring_buffer_read_page_size(info->spare);
> > +		/* Do we have previous read data to read? */
> > +		if (info->read < spare_size)
> > +			goto read;
> >  	}
> 
> I would have ring_buffer_free_read_page() accept a null pointer and then here do:
> 
> 	spare_size = ring_buffer_read_page_size(info->spare);
> 
>  again:
> 	/* Do we have previous read data to read? */
> 	if (info->read < spare_size)
> 		goto read;
> 
> As the jump to here below has already calculated the spare_size, why do it again?
> 
> Have ring_buffer_read_page_size() be:
> 
> unsigned int ring_buffer_read_page_size(struct buffer_data_read_page *rpage)
> {
> 	return rpage ? PAGE_SIZE << rpage->order : 0;
> }
> 
> Then info->read could not be less than spare_size if there was no spare.

Sounds good.

> 
> >  
> > -	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 subbuf order */
> 
> The above comment doesn't really make sense anymore since the user here
> should not care about the order. I would nuke it.

ack.

> 
> > +	ret = ring_buffer_alloc_read_page(iter->array_buffer->buffer, iter->cpu_file,
> > +					  &info->spare);
> > +	if (ret)
> >  		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,
> > @@ -7148,8 +7136,9 @@ ssize_t tracing_buffers_read(struct file *filp, char __user *ubuf,
> >  	}
> >  
> >  	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);
> > @@ -7199,17 +7188,17 @@ int tracing_buffers_release(struct inode *inode, struct file *file)
> >  }
> >  
> >  struct buffer_ref {
> > -	struct trace_buffer	*buffer;
> > -	void			*page;
> > -	int			cpu;
> > -	refcount_t		refcount;
> > +	struct trace_buffer		*buffer;
> > +	struct buffer_data_read_page	*rpage;
> > +	int				cpu;
> > +	refcount_t			refcount;
> >  };
> >  
> >  static void buffer_ref_release(struct buffer_ref *ref)
> >  {
> >  	if (!refcount_dec_and_test(&ref->refcount))
> >  		return;
> > -	ring_buffer_free_read_page(ref->buffer, ref->cpu, ref->page);
> > +	ring_buffer_free_read_page(ref->buffer, ref->cpu, ref->rpage);
> >  	kfree(ref);
> >  }
> >  
> > @@ -7270,23 +7259,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));
> > -	}
> 
> OK, you are removing this so that it is tested in the loop?

Yes, page_size can now be modified between iterations.

> 
> > -
> >  	if (splice_grow_spd(pipe, &spd))
> >  		return -ENOMEM;
> >  
> > @@ -7294,7 +7272,8 @@ 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++) {
> 
> Is there a reason you moved the len -= page_size from here to the end of
> the loop?  Basically that has no functional change.

Same reason as above, page_size can be modified and I thought it'd be more clear
to see the declaration within the loop, but yes, that wouldn't change anything.

I'll keep it as-is.

> 
> > +		unsigned int page_size;
> 
> Was that just to move page_size here?
> 
> Let's keep it as-is.
> 
> >  		struct page *page;
> >  		int r;
> >  
> > @@ -7306,25 +7285,36 @@ 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);
> > -		if (IS_ERR(ref->page)) {
> > -			ret = PTR_ERR(ref->page);
> > -			ref->page = NULL;
> > +
> > +		ret = ring_buffer_alloc_read_page(ref->buffer, iter->cpu_file, &ref->rpage);
> > +		if (ret) {
> >  			kfree(ref);
> >  			break;
> >  		}
> >  		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->rpage);
> > +
> > +		r = -EINVAL;
> > +		if (IS_ALIGNED(*ppos, page_size) && len >= page_size) {
> > +			r = ring_buffer_read_page(ref->buffer, ref->rpage, 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;
> 
> This is more likely to be true not because the subbuf order was modified,
> but also if the length was not a multiple of page_size. From the code you
> removed;
> 
> -	if (len & (page_size - 1)) {
> -		if (len < page_size)
> -			return -EINVAL;
> -		len &= (~(page_size - 1));
> -	}
> 
> It would error if len was smaller than page_size but otherwise it would
> modify len to be a multiple of page_size.
> 
> The overall behavior is the same, but the comment needs to be updated.
> 
> -- Steve

I believe "len < page_size" is what I meant by "invalid userspace input", I'll
rephrase it to something more clear. I've just had a hard time myself
understanding that comment.

-- 
Vincent

> 
> > +		}
> > +
> >  		if (r < 0) {
> > -			ring_buffer_free_read_page(ref->buffer, ref->cpu,
> > -						   ref->page);
> > +			ring_buffer_free_read_page(ref->buffer, ref->cpu, ref->rpage);
> >  			kfree(ref);
> >  			break;
> >  		}
> >  
> > -		page = virt_to_page(ring_buffer_read_page_data(ref->page));
> > +		page = virt_to_page(ring_buffer_read_page_data(ref->rpage));
> >  
> >  		spd.pages[i] = page;
> >  		spd.partial[i].len = page_size;
> > @@ -7332,6 +7322,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 74a7a50d1e78..203d098ee14e 100644
> > --- a/kernel/trace/trace.h
> > +++ b/kernel/trace/trace.h
> > @@ -745,11 +745,10 @@ static inline int tracing_get_cpu(struct inode *inode)
> >  void tracing_reset_cpu(struct array_buffer *buf, int cpu);
> >  
> >  struct ftrace_buffer_info {
> > -	struct trace_iterator	iter;
> > -	void			*spare;
> > -	unsigned int		spare_cpu;
> > -	unsigned int		spare_size;
> > -	unsigned int		read;
> > +	struct trace_iterator		iter;
> > +	struct buffer_data_read_page	*spare;
> > +	unsigned int			spare_cpu;
> > +	unsigned int			read;
> >  };
> >  
> >  /**
> 

Re: [PATCH v9 3/4] ring-buffer: Cap static ring buffer nr_pages

[Steven Rostedt]

On Thu, 3 Sep 2026 18:06:51 +0100
Vincent Donnefort <vdonnefort@google.com> wrote:

> Yeah, I wasn't sure... It's so pedantic, we have no user with a small size :-(

Yeah, but perhaps it would make sense if an admin accidentally put in a
very small size (deleted some zeros on the kernel command line?).

> 
> But that would make sense to fold that into this patch, right?

Yes please.

-- Steve

Re: [PATCH v9 4/4] ring-buffer: Prevent truncation of nr_pages / nr_subbufs

[Vincent Donnefort]

On Thu, Sep 03, 2026 at 01:16:01PM -0400, Steven Rostedt wrote:
> On Tue,  1 Sep 2026 16:54:45 +0100
> Vincent Donnefort <vdonnefort@google.com> wrote:
> 
> > -static void *rb_range_buffer(struct ring_buffer_per_cpu *cpu_buffer, int idx)
> > +static void *rb_range_buffer(struct ring_buffer_per_cpu *cpu_buffer, unsigned int idx)
> >  {
> >  	struct ring_buffer_cpu_meta *meta;
> > +	unsigned int subbuf_size;
> >  	unsigned long ptr;
> > -	int subbuf_size;
> >  
> >  	meta = rb_range_meta(cpu_buffer->buffer, 0, cpu_buffer->cpu);
> >  	if (!meta)
> > @@ -1777,7 +1776,7 @@ static void *rb_range_buffer(struct ring_buffer_per_cpu *cpu_buffer, int idx)
> >  
> >  	ptr = (unsigned long)rb_subbufs_from_meta(meta);
> >  
> > -	ptr += subbuf_size * idx;
> > +	ptr += (unsigned long)subbuf_size * idx;
> 
> Really, it looks like the idx should be typecasted, as it is the number of
> subbuffers. Maybe even pass it in as unsigned long?

__rb_allocate_pages() is passing idx as a number pages, so yeah that'd make
more sense, even though a persistent buffer is capped to a 30-bits nr_pages.

> 
> >  	if (ptr + subbuf_size > cpu_buffer->buffer->range_addr_end)
> >  		return NULL;
> >  
> > @@ -1854,13 +1853,13 @@ 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 long nr_pages,
> >  			      unsigned long *subbuf_mask)
> >  {
> > -	int subbuf_size = PAGE_SIZE;
> > +	unsigned long subbuf_size = PAGE_SIZE;
> 
> Why the long? Shouldn't it be unsigned int?

That is to cheat to not have to add a cast in

  buffers_end = meta->first_buffer + (subbuf_size * meta->nr_subbufs);

> 
> >  	unsigned long buffers_start;
> >  	unsigned long buffers_end;
> > -	int i;
> > +	unsigned long i;
> >  
> >  	if (!subbuf_mask)
> >  		return false;
> > @@ -2109,8 +2108,8 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
> >  	struct buffer_page *head_page, *orig_head, *orig_reader;
> >  	struct rb_validation_state state = { 0 };
> >  	bool skip = false;
> > +	unsigned long i;
> >  	int ret;
> > -	int i;
> >  
> >  	if (!meta || !meta->head_buffer)
> >  		return;
> > @@ -2161,7 +2160,7 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
> >  		rb_validate_buffer(head_page, cpu_buffer, meta, &state, 0, state.ts);
> >  	}
> >  	if (i)
> > -		pr_info("Ring buffer [%d] rewound %d pages\n", cpu_buffer->cpu, i);
> > +		pr_info("Ring buffer [%d] rewound %lu pages\n", cpu_buffer->cpu, i);
> >  
> >  	/* The last rewound page must be skipped. */
> >  	if (head_page != orig_head)
> > @@ -2245,7 +2244,8 @@ 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 long nr_pages, int scratch_size)
> 
> static void rb_range_meta_init(struct trace_buffer *buffer, unsigned long nr_pages,
> 			       int scratch_size)
> 
> looks better ;-)

ack

-- 
Vincent

> 
> >  {
> >  	struct ring_buffer_cpu_meta *meta;
> >  	unsigned long *subbuf_mask;
> > @@ -2345,8 +2345,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;
> >  	}
> >  
> 
> 
> -- Steve

Re: [PATCH v9 4/4] ring-buffer: Prevent truncation of nr_pages / nr_subbufs

[Steven Rostedt]

On Thu, 3 Sep 2026 18:37:36 +0100
Vincent Donnefort <vdonnefort@google.com> wrote:

> > >  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 long nr_pages,
> > >  			      unsigned long *subbuf_mask)
> > >  {
> > > -	int subbuf_size = PAGE_SIZE;
> > > +	unsigned long subbuf_size = PAGE_SIZE;  
> > 
> > Why the long? Shouldn't it be unsigned int?  
> 
> That is to cheat to not have to add a cast in
> 
>   buffers_end = meta->first_buffer + (subbuf_size * meta->nr_subbufs);

Hmm, it's only reference once in that function. We could just replace it
with using PAGE_SIZE directly.

	buffers_end = meta->first_buffer + (meta->nr_subbufs * PAGE_SIZE);

-- Steve

Re: [PATCH v9 1/4] ring-buffer: Allow splice reads on static buffers

[Steven Rostedt]

On Tue,  1 Sep 2026 16:54:42 +0100
Vincent Donnefort <vdonnefort@google.com> wrote:

> ring_buffer_read_page() rejects splice (full=1) reads on static buffers
> (that is user-mapped, persistent or remote) because !read check assumes
> unread pages must be swapped. However for those buffers we have no other
> choice than memcpy the data.
> 
> For the memcpy case, only return an error when the writer is still on
> the reader page for the splice interface to wait.
> 
> Fixes: 117c39200d9d ("ring-buffer: Introducing ring-buffer mapping functions")
> Signed-off-by: Vincent Donnefort <vdonnefort@google.com>

I'm going to apply this patch now, so you can drop it from your v10.

-- Steve

Re: [PATCH v9 3/4] ring-buffer: Cap static ring buffer nr_pages

[Vincent Donnefort]

On Thu, Sep 03, 2026 at 12:56:21PM -0400, Steven Rostedt wrote:
> On Tue,  1 Sep 2026 16:54:44 +0100
> Vincent Donnefort <vdonnefort@google.com> wrote:
> 
> > Static ring buffers (i.e. persistent, user-mapped and remote) rely on
> > the bpage::id field. The number of pages for those ring buffers must fit
> > into that variable. Enforce this limit on ring buffer creation or
> > user-mapping.
> > 
> > While at it, make buffer_page::id 31 bits. This does not change the
> > struct buffer_page size.
> 
> Let's not add that change to this patch. Especially since this has a fixes
> tag to it. That change has nothing to do with the fix.
> 
> The reason I had it as 30 to begin with was to reserve a bit in case I
> found another reason for it. If 1<<30 is too small for the number of boot
> buffer pages, we can always up in another order in the future.
> 
> > 
> > Fixes: be68d63a139b ("ring-buffer: Add ring_buffer_alloc_range()")
> > Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
> > 
> > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> > index 28dd76edfecf..c4260d6ecdfc 100644
> > --- a/kernel/trace/ring_buffer.c
> > +++ b/kernel/trace/ring_buffer.c
> > @@ -350,7 +350,7 @@ struct buffer_page {
> >  	local_t		 entries;	/* entries on this page */
> >  	unsigned long	 real_end;	/* real end of data */
> >  	unsigned	 order;		/* order of the page */
> > -	u32		 id:30;		/* ID for external mapping */
> > +	u32		 id:31;		/* ID for external mapping */
> >  	u32		 range:1;	/* Mapped via a range */
> >  	struct buffer_data_page *page;	/* Actual data page */
> >  };
> > @@ -657,6 +657,15 @@ static bool rb_is_static(struct ring_buffer_per_cpu *cpu_buffer)
> >  	return cpu_buffer->user_mapped || cpu_buffer->remote || cpu_buffer->ring_meta;
> >  }
> >  
> > +static unsigned long rb_static_max_pages(void)
> > +{
> > +	/*
> > +	 * Static ring buffers are using bpage::id and must account for the
> > +	 * reader page.
> > +	 */
> > +	return (1UL << 31) - 1;
> > +}
> > +
> >  struct ring_buffer_iter {
> >  	struct ring_buffer_per_cpu	*cpu_buffer;
> >  	unsigned long			head;
> > @@ -2842,6 +2851,10 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
> >  		 */
> >  		nr_pages = (size - sizeof(struct ring_buffer_cpu_meta)) /
> >  			(subbuf_size + sizeof(int));
> > +
> > +		if (nr_pages > rb_static_max_pages())
> > +			goto fail_free_buffers;
> > +
> 
> If you want to add something, we could add to the beginning of this
> function:
> 
> 	/* Prevent ridiculously small sizes */
> 	if (size < PAGE_SIZE)
> 		return NULL;
> 
> to shut up Sashiko about overflows :-p
> 
> -- Steve

tracer_alloc_buffers() uses size of 1 for non-expanded buffers.

I'll test size just before 

  nr_pages = (size - sizeof(struct ring_buffer_cpu_meta)) /
  	(subbuf_size + sizeof(int));

> 
> 
> 
> >  		/* Need at least two pages plus the reader page */
> >  		if (nr_pages < 3)
> >  			goto fail_free_buffers;
> > @@ -2874,6 +2887,10 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
> >  		/* The writer is remote. This ring-buffer is read-only */
> >  		atomic_inc(&buffer->record_disabled);
> >  		nr_pages = desc->nr_page_va - 1;
> > +
> > +		if (nr_pages > rb_static_max_pages())
> > +			goto fail_free_buffers;
> > +
> >  		if (nr_pages < 2)
> >  			goto fail_free_buffers;
> >  	} else {
> > @@ -7836,6 +7853,9 @@ int ring_buffer_map(struct trace_buffer *buffer, int cpu,
> >  	/* prevent another thread from changing buffer/sub-buffer sizes */
> >  	guard(mutex)(&buffer->mutex);
> >  
> > +	if (cpu_buffer->nr_pages > rb_static_max_pages())
> > +		return -E2BIG;
> > +
> >  	err = rb_alloc_meta_page(cpu_buffer);
> >  	if (err)
> >  		return err;
> 

Re: [PATCH v9 3/4] ring-buffer: Cap static ring buffer nr_pages

[Steven Rostedt]

On Fri, 4 Sep 2026 14:04:40 +0100
Vincent Donnefort <vdonnefort@google.com> wrote:

> > If you want to add something, we could add to the beginning of this
> > function:
> > 
> > 	/* Prevent ridiculously small sizes */
> > 	if (size < PAGE_SIZE)
> > 		return NULL;
> > 
> > to shut up Sashiko about overflows :-p
> > 
> > -- Steve  
> 
> tracer_alloc_buffers() uses size of 1 for non-expanded buffers.
> 
> I'll test size just before 
> 
>   nr_pages = (size - sizeof(struct ring_buffer_cpu_meta)) /
>   	(subbuf_size + sizeof(int));
> 

Ack.

-- Steve