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

[Steven Rostedt <rostedt@goodmis.org>]

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