Re: [PATCH v6 2/2] ring-buffer: Improve nr_pages type

[Vincent Donnefort <vdonnefort@google.com>]

On Fri, Aug 14, 2026 at 04:48:23PM +0100, Vincent Donnefort wrote:
> If 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.
> 
> Make sure nr_pages is never capped to 32-bits (that includes nr_subbufs)
> and reject a value over 32-bits for the user-mapped, persistent buffer
> and remote buffer cases where the limiting factor is the shared
> meta-data member for the number of pages/subbufs.
> 
> While at it, make sure subbuf_size is 'unsigned int'.
> 
> Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
> ---
>  kernel/trace/ring_buffer.c | 46 ++++++++++++++++++++++++--------------
>  1 file changed, 29 insertions(+), 17 deletions(-)
> 
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index ec13779922ff..ec127e2ad052 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -1669,7 +1669,7 @@ static void rb_check_pages(struct ring_buffer_per_cpu *cpu_buffer)
>   * This is used to help find the next per cpu subbuffer within a mapped range.
>   */
>  static unsigned long
> -rb_range_align_subbuf(unsigned long addr, int subbuf_size, int nr_subbufs)
> +rb_range_align_subbuf(unsigned long addr, unsigned int subbuf_size, unsigned long nr_subbufs)
>  {
>  	addr += sizeof(struct ring_buffer_cpu_meta) +
>  		sizeof(int) * nr_subbufs;
> @@ -1679,13 +1679,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)
> @@ -1731,7 +1730,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;
> @@ -1746,8 +1745,8 @@ static void *rb_subbufs_from_meta(struct ring_buffer_cpu_meta *meta)
>  static void *rb_range_buffer(struct ring_buffer_per_cpu *cpu_buffer, int idx)
>  {
>  	struct ring_buffer_cpu_meta *meta;
> +	unsigned int subbuf_size;
>  	unsigned long ptr;
> -	int subbuf_size;
>  
>  	meta = rb_range_meta(cpu_buffer->buffer, 0, cpu_buffer->cpu);
>  	if (!meta)
> @@ -1840,10 +1839,10 @@ static bool rb_meta_init(struct trace_buffer *buffer, int scratch_size)
>   * must be the same.
>   */
>  static bool rb_cpu_meta_valid(struct ring_buffer_cpu_meta *meta, int cpu,
> -			      struct trace_buffer *buffer, int nr_pages,
> +			      struct trace_buffer *buffer, unsigned long nr_pages,
>  			      unsigned long *subbuf_mask)
>  {
> -	int subbuf_size = PAGE_SIZE;
> +	unsigned int subbuf_size = PAGE_SIZE;
>  	unsigned long buffers_start;
>  	unsigned long buffers_end;
>  	int i;
> @@ -2231,7 +2230,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;
> @@ -2417,7 +2417,7 @@ static void *ring_buffer_desc_page(struct ring_buffer_desc *desc, unsigned int p
>  }
>  
>  static int __rb_allocate_pages(struct ring_buffer_per_cpu *cpu_buffer,
> -		long nr_pages, struct list_head *pages)
> +			       unsigned long nr_pages, struct list_head *pages)
>  {
>  	struct trace_buffer *buffer = cpu_buffer->buffer;
>  	struct ring_buffer_cpu_meta *meta = NULL;
> @@ -2545,7 +2545,7 @@ static int rb_allocate_pages(struct ring_buffer_per_cpu *cpu_buffer,
>  }
>  
>  static struct ring_buffer_per_cpu *
> -rb_allocate_cpu_buffer(struct trace_buffer *buffer, long nr_pages, int cpu)
> +rb_allocate_cpu_buffer(struct trace_buffer *buffer, unsigned long nr_pages, int cpu)
>  {
>  	struct ring_buffer_per_cpu *cpu_buffer __free(kfree) =
>  		alloc_cpu_buffer(cpu);
> @@ -2702,8 +2702,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;
> @@ -2773,8 +2773,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;
> @@ -2837,6 +2837,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));
> +
> +		/* limited by ring_buffer_cpu_meta::nr_subbufs */
> +		if (nr_pages > U32_MAX - 1)
> +			goto fail_free_buffers;
>  		/* Need at least two pages plus the reader page */
>  		if (nr_pages < 3)
>  			goto fail_free_buffers;
> @@ -2869,6 +2873,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;
> +
> +		/* limited by ring_buffer_desc::nr_page_va */
> +		if (nr_pages > U32_MAX - 1)
> +			goto fail_free_buffers;
>  		if (nr_pages < 2)
>  			goto fail_free_buffers;
>  	} else {
> @@ -7421,8 +7429,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;
> @@ -7604,7 +7612,7 @@ 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;
> @@ -7834,6 +7842,10 @@ int ring_buffer_map(struct trace_buffer *buffer, int cpu,
>  	/* prevent another thread from changing buffer/sub-buffer sizes */
>  	guard(mutex)(&buffer->mutex);
>  
> +	/* limited by trace_buffer_meta::nr_subbufs */

And actually I have realised the limiting factor is bpage::id which is only
30-bits.

> +	if (cpu_buffer->nr_pages > U32_MAX - 1)
> +		return -E2BIG;
> +
>  	err = rb_alloc_meta_page(cpu_buffer);
>  	if (err)
>  		return err;
> -- 
> 2.55.0.691.gc56d675ccc-goog
> 

-- 
Vincent