Re: [PATCH v8 1/3] tracing: Fix subbuf resize races with trace_pipe_raw readers

[Vincent Donnefort <vdonnefort@google.com>]

On Wed, Aug 26, 2026 at 10:37:36AM -0400, Steven Rostedt wrote:
> On Wed, 26 Aug 2026 09:59:46 +0000
> sashiko-bot@kernel.org wrote:
> 
> > > 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  
> > [ ... ]
> > > @@ -7183,7 +7218,8 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
> > >  	 */
> > >  	if (read || (len < (size - read)) ||
> > >  	    cpu_buffer->reader_page == cpu_buffer->commit_page ||
> > > -	    rb_is_static(cpu_buffer)) {
> > > +	    rb_is_static(cpu_buffer) ||
> > > +	    data_page->order != reader->order) {
> > >  		struct buffer_data_page *rpage = cpu_buffer->reader_page->page;
> > >  		unsigned int rpos = read;
> > >  		unsigned int pos = 0;  
> > 
> > [Severity: High]
> > Does this new condition break splice reads when the buffer order changes?
> > 
> > When tracing_buffers_splice_read() allocates a read page with the new order
> > and calls ring_buffer_read_page() with full=1, it will now enter this manual
> > copy block because the subbuffer orders no longer match.
> > 
> > However, just a few lines down inside this block in ring_buffer_read_page(),
> > there is the following check:
> > 
> >         if (full &&
> >             (!read || (len < (size - read)) ||
> >              cpu_buffer->reader_page == cpu_buffer->commit_page))
> >                 return -1;
> > 
> > Since the reader page typically hasn't been partially read yet (read == 0),
> > and full == 1 from splice, this check evaluates to true and immediately
> > returns -1 instead of copying the data.
> > 
> > Because tracing_buffers_splice_read() loops and ultimately returns -EAGAIN
> > on failure, will this cause an infinite loop of -EAGAIN for userspace tools
> > trying to read across buffer resizes?
> > 
> 
> 
> This is a legitimate concern. But with an easy solution. Perhaps just add:
> 
> 	if (full &&
> 	    (!read || (len < (size - read)) ||
> -	     cpu_buffer->reader_page == cpu_buffer->commit_page))
> +	     cpu_buffer->reader_page == cpu_buffer->commit_page) &&
> +	    data_page->order == buffer->subbuf_order)
> 		return -1;
> 
> If the user is changing the buffer size at the same time as reading raw
> pages, they get what they deserve! I just don't want to let the kernel go
> into an infinite loop.
> 
> -- Steve
> 

Sounds good. Do you prefer to get a v9 or you fold this into the existing commit?

-- 
Vincent