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

Vincent Donnefort <[email protected]>
Newsgroups org.kernel.vger.linux-trace-kernel,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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 <[email protected]>
> ---
>  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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.