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

[Vincent Donnefort <vdonnefort@google.com>]

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