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@xxxxxxxxxx> 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@xxxxxxxxxx>
> 
> 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;