Linux Trace Kernel
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: sashiko-bot@kernel.org
Cc: sashiko-reviews@lists.linux.dev,
	"Vincent Donnefort" <vdonnefort@google.com>,
	linux-trace-kernel@vger.kernel.org
Subject: Re: [PATCH v8 1/3] tracing: Fix subbuf resize races with trace_pipe_raw readers
Date: Wed, 26 Aug 2026 10:37:36 -0400	[thread overview]
Message-ID: <20260826103736.2b55b464@gandalf.local.home> (raw)
In-Reply-To: <20260826095947.4FF8B1F000E9@smtp.kernel.org>

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


  reply	other threads:[~2026-08-26 14:36 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-26  9:45 [PATCH v8 0/3] ring-buffer: Fixes for subbuf resizing and persistent buffers Vincent Donnefort
2026-08-26  9:45 ` [PATCH v8 1/3] tracing: Fix subbuf resize races with trace_pipe_raw readers Vincent Donnefort
2026-08-26  9:59   ` sashiko-bot
2026-08-26 14:37     ` Steven Rostedt [this message]
2026-08-26 16:24       ` Vincent Donnefort
2026-08-26 18:31         ` Steven Rostedt
2026-08-27  6:31           ` Vincent Donnefort
2026-08-27 13:15             ` Steven Rostedt
2026-08-27 16:21               ` Vincent Donnefort
2026-08-27 19:33                 ` Steven Rostedt
2026-08-28  8:24                   ` Vincent Donnefort
2026-08-28  8:36                     ` Steven Rostedt
2026-08-26  9:45 ` [PATCH v8 2/3] ring-buffer: Cap static ring buffer nr_pages Vincent Donnefort
2026-08-26  9:45 ` [PATCH v8 3/3] ring-buffer: Prevent truncation of nr_pages / nr_subbufs Vincent Donnefort
2026-08-26 10:02   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260826103736.2b55b464@gandalf.local.home \
    --to=rostedt@goodmis.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=sashiko-bot@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=vdonnefort@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox