Re: [PATCH v1 1/2] tracing/remotes: Account for ring buffer page header in size calculation
From: Steven Rostedt <rostedt@goodmis.org>
Date: 2026-09-09 22:12:02
Also in:
lkml
On Mon, 7 Sep 2026 20:26:42 +0100 Vincent Donnefort [off-list ref] wrote:
trace_buffer_desc_size() and trace_remote_alloc_buffer undercount the required pages because every ring buffer page contains a header (BUF_PAGE_HDR_SIZE). Account for that header to ensure allocated remote ring buffers aren't smaller than requested by the user. While at it, ensure those functions catch nr_pages overflow.
Fixes patches should never have "while at it". That means it belongs as a separate patch. The only "while at it" that is acceptable is white space fixes or other formatting changes along with new code that is not a fix.
quoted hunk ↗ jump to hunk
Fixes: 2e67fabd8b77 ("ring-buffer: Introduce ring-buffer remotes") Signed-off-by: Vincent Donnefort <redacted>diff --git a/include/linux/ring_buffer.h b/include/linux/ring_buffer.h index afc7daa6ee7d..7a1a92f87650 100644 --- a/include/linux/ring_buffer.h +++ b/include/linux/ring_buffer.h@@ -8,6 +8,8 @@ #include <uapi/linux/trace_mmap.h> +#include <linux/ring_buffer_types.h>
Hmm, why is this after the uapi header? It should be part of the linux/ headers.
quoted hunk ↗ jump to hunk
+ struct trace_buffer; struct ring_buffer_iter;@@ -281,9 +283,14 @@ static inline struct ring_buffer_desc *__first_ring_buffer_desc(struct trace_buf static inline size_t trace_buffer_desc_size(size_t buffer_size, unsigned int nr_cpus) { - unsigned int nr_pages = max(DIV_ROUND_UP(buffer_size, PAGE_SIZE), 2UL) + 1; + unsigned long nr_pages = + max(DIV_ROUND_UP(buffer_size, PAGE_SIZE - BUF_PAGE_HDR_SIZE), 2UL) + 1;
This is getting a little unwieldy. Let's break it up:
unsigned long pages_req = DIV_ROUND_UP(buffer_size, PAGE_SIZE - BUF_PAGE_HDR_SIZE);
unsigned long nr_pages = max(pages_req, 2UL) + 1;
Or better yet, let's add a new helper function:
static inline unsigned long calculate_nr_pages(size_t buffer_size)
{
unsigned long req_pages = DIV_ROUND_UP(buffer_size, PAGE_SIZE - BUF_PAGE_HDR_SIZE);
return max(pages_req, 2UL) + 1;
}
Then this could be simply:
unsigned long nr_pages = calculate_nr_pages(buffer_size);
struct ring_buffer_desc *rbdesc; + /* Capped by ring_buffer_desc::nr_page_va */ + if (nr_pages > UINT_MAX) + return SIZE_MAX;
Separate patch.
quoted hunk ↗ jump to hunk
+ return size_add(offsetof(struct trace_buffer_desc, __data), size_mul(nr_cpus, struct_size(rbdesc, page_va, nr_pages))); }diff --git a/kernel/trace/trace_remote.c b/kernel/trace/trace_remote.c index 75fa1ffc4c96..2e0fdbb730b7 100644 --- a/kernel/trace/trace_remote.c +++ b/kernel/trace/trace_remote.c@@ -980,9 +980,12 @@ int trace_remote_alloc_buffer(struct trace_buffer_desc *desc, size_t desc_size, const struct cpumask *cpumask) { size_t min_desc_size = trace_buffer_desc_size(buffer_size, cpumask_weight(cpumask)); - unsigned int nr_pages = max(DIV_ROUND_UP(buffer_size, PAGE_SIZE), 2UL) + 1; struct ring_buffer_desc *rb_desc; int cpu, ret = -ENOMEM; + unsigned int nr_pages; +
+ if (min_desc_size == SIZE_MAX) + return -E2BIG;
Looks like this should be a separate patch too.
quoted hunk ↗ jump to hunk
if (desc_size < min_desc_size) return -EINVAL;@@ -991,6 +994,7 @@ int trace_remote_alloc_buffer(struct trace_buffer_desc *desc, size_t desc_size, desc->struct_len = min_desc_size; rb_desc = __first_ring_buffer_desc(desc); + nr_pages = max(DIV_ROUND_UP(buffer_size, PAGE_SIZE - BUF_PAGE_HDR_SIZE), 2UL) + 1;
And here we can have; nr_pages = calculate_nr_pages(buffer_size); -- Steve
for_each_cpu(cpu, cpumask) {
unsigned int id;