[PATCH] ftrace: Add checking nr_subbufs to persistent ring buffer validation

[PATCH] ftrace: Add checking nr_subbufs to persistent ring buffer validation

[Steven Rostedt]

From: Steven Rostedt <rostedt@goodmis.org>

Sashiko reported that the code was using meta->nr_subbufs without making
sure that it matched the nr_pages + 1 on data that was assuming the two
were the same.

Add a check to the persistent ring buffer validation code to make sure
that the saved nr_subbufs matches what we expect.

Fixes: f5b95f1fa2ef3 ("ring-buffer: Validate the persistent meta data subbuf array")
Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/all/20260901164836.D962D1F000E9@smtp.kernel.org/
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/ring_buffer.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index ef66f726e2e2..748d4ae8badf 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -1869,6 +1869,11 @@ static bool rb_cpu_meta_valid(struct ring_buffer_cpu_meta *meta, int cpu,
 		return false;
 	}
 
+	if (meta->nr_subbufs != nr_pages + 1) {
+		pr_info("Ring buffer boot meta [%d] invalid nr_subbufs\n", cpu);
+		return false;
+	}
+
 	buffers_start = meta->first_buffer;
 	buffers_end = meta->first_buffer + (subbuf_size * meta->nr_subbufs);
 
-- 
2.53.0

Re: [PATCH] ftrace: Add checking nr_subbufs to persistent ring buffer validation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Integer overflow in per-CPU ring buffer metadata calculations due to 32-bit arithmetic, leading to memory corruption and false validation rejections for trace buffers larger than 2GB/4GB.
--

commit dffcd0268ea5b9908d6dadacec8d07651bd25ceb
Author: Steven Rostedt <rostedt@goodmis.org>

ftrace: Add checking nr_subbufs to persistent ring buffer validation
    
This commit adds a check to the persistent ring buffer validation logic. It
ensures that the saved nr_subbufs in the metadata matches the expected
nr_pages + 1 to prevent mismatches.

Closes: https://lore.kernel.org/all/20260901164836.D962D1F000E9@smtp.kernel.org/

> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -1856,6 +1856,11 @@ static bool rb_cpu_meta_valid(struct ring_buffer_cpu_meta *meta, int cpu,
>  		return false;
>  	}
>  
> +	if (meta->nr_subbufs != nr_pages + 1) {
> +		pr_info("Ring buffer boot meta [%d] invalid nr_subbufs\n", cpu);
> +		return false;
> +	}
> +
>  	buffers_start = meta->first_buffer;
>  	buffers_end = meta->first_buffer + (subbuf_size * meta->nr_subbufs);

[Severity: High]
This is a pre-existing issue, but could this 32-bit arithmetic overflow for
very large trace buffers? 

In rb_cpu_meta_valid(), subbuf_size is an int and meta->nr_subbufs is a __u32.
If an administrator configures a per-CPU trace buffer larger than 4GB, their
product might truncate before being added to the 64-bit meta->first_buffer.
Would this result in an artificially small buffers_end, causing valid buffers
to falsely fail the bounds check on reboot?

A similar pre-existing issue appears in rb_range_meta(), where both variables
are signed 32-bit ints:

kernel/trace/ring_buffer.c:rb_range_meta() {
    ...
    ptr += subbuf_size * nr_subbufs;
    ...
}

Does this signed multiplication overflow for buffers over 2GB, causing the
ptr to move backward and potentially corrupt memory when written?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903132728.7fb27d34@gandalf.local.home?part=1

Re: [PATCH] ftrace: Add checking nr_subbufs to persistent ring buffer validation

[Vincent Donnefort]

On Thu, Sep 03, 2026 at 05:38:22PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Integer overflow in per-CPU ring buffer metadata calculations due to 32-bit arithmetic, leading to memory corruption and false validation rejections for trace buffers larger than 2GB/4GB.
> --
> 
> commit dffcd0268ea5b9908d6dadacec8d07651bd25ceb
> Author: Steven Rostedt <rostedt@goodmis.org>
> 
> ftrace: Add checking nr_subbufs to persistent ring buffer validation
>     
> This commit adds a check to the persistent ring buffer validation logic. It
> ensures that the saved nr_subbufs in the metadata matches the expected
> nr_pages + 1 to prevent mismatches.
> 
> Closes: https://lore.kernel.org/all/20260901164836.D962D1F000E9@smtp.kernel.org/
> 
> > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> > --- a/kernel/trace/ring_buffer.c
> > +++ b/kernel/trace/ring_buffer.c
> > @@ -1856,6 +1856,11 @@ static bool rb_cpu_meta_valid(struct ring_buffer_cpu_meta *meta, int cpu,
> >  		return false;
> >  	}
> >  
> > +	if (meta->nr_subbufs != nr_pages + 1) {
> > +		pr_info("Ring buffer boot meta [%d] invalid nr_subbufs\n", cpu);
> > +		return false;
> > +	}
> > +
> >  	buffers_start = meta->first_buffer;
> >  	buffers_end = meta->first_buffer + (subbuf_size * meta->nr_subbufs);
> 
> [Severity: High]
> This is a pre-existing issue, but could this 32-bit arithmetic overflow for
> very large trace buffers? 
> 
> In rb_cpu_meta_valid(), subbuf_size is an int and meta->nr_subbufs is a __u32.
> If an administrator configures a per-CPU trace buffer larger than 4GB, their
> product might truncate before being added to the 64-bit meta->first_buffer.
> Would this result in an artificially small buffers_end, causing valid buffers
> to falsely fail the bounds check on reboot?
> 
> A similar pre-existing issue appears in rb_range_meta(), where both variables
> are signed 32-bit ints:
> 
> kernel/trace/ring_buffer.c:rb_range_meta() {
>     ...
>     ptr += subbuf_size * nr_subbufs;
>     ...
> }
> 
> Does this signed multiplication overflow for buffers over 2GB, causing the
> ptr to move backward and potentially corrupt memory when written?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260903132728.7fb27d34@gandalf.local.home?part=1
> 

hahah, yes we know :) 

https://lore.kernel.org/all/apmwYLVIxX-C21aB@google.com/

-- 
Vincent

Re: [PATCH] ftrace: Add checking nr_subbufs to persistent ring buffer validation

[Vincent Donnefort]

On Thu, Sep 03, 2026 at 01:27:28PM -0400, Steven Rostedt wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
> 
> Sashiko reported that the code was using meta->nr_subbufs without making
> sure that it matched the nr_pages + 1 on data that was assuming the two
> were the same.
> 
> Add a check to the persistent ring buffer validation code to make sure
> that the saved nr_subbufs matches what we expect.
> 
> Fixes: f5b95f1fa2ef3 ("ring-buffer: Validate the persistent meta data subbuf array")
> Reported-by: sashiko-bot@kernel.org
> Closes: https://lore.kernel.org/all/20260901164836.D962D1F000E9@smtp.kernel.org/
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

Reviewed-by: Vincent Donnefort <vdonnefort@google.com>

> ---
>  kernel/trace/ring_buffer.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index ef66f726e2e2..748d4ae8badf 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -1869,6 +1869,11 @@ static bool rb_cpu_meta_valid(struct ring_buffer_cpu_meta *meta, int cpu,
>  		return false;
>  	}
>  
> +	if (meta->nr_subbufs != nr_pages + 1) {
> +		pr_info("Ring buffer boot meta [%d] invalid nr_subbufs\n", cpu);
> +		return false;
> +	}
> +
>  	buffers_start = meta->first_buffer;
>  	buffers_end = meta->first_buffer + (subbuf_size * meta->nr_subbufs);
>  
> -- 
> 2.53.0
> 

Re: [PATCH] ftrace: Add checking nr_subbufs to persistent ring buffer validation

[Steven Rostedt]

I just realized I used "ftrace:" and not "ring-buffer:". I'll apply it with
the updated subject.

-- Steve