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

[Steven Rostedt <rostedt@goodmis.org>]

On Mon, 17 Aug 2026 14:08:11 +0000
sashiko-bot@kernel.org wrote:

> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index 395238b2b715c..737922b236d46 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c  
> [ ... ]
> > @@ -7128,7 +7116,9 @@ ssize_t tracing_buffers_read(struct file *filp, char __user *ubuf,
> >  				    iter->cpu_file, 0);
> >  	trace_access_unlock(iter->cpu_file);
> >  
> > -	if (ret < 0) {
> > +	if (ret == -EAGAIN) {
> > +		goto again;
> > +	} else if (ret < 0) {  
> 
> [Severity: High]
> If ring_buffer_read_page() returns -EAGAIN because the subbuffer order was
> updated but cpu_buffer->reader_page->order hasn't been updated yet, this
> branches back to the again label.
> 
> Since the loop does not yield the CPU or call cond_resched(), could this cause
> a hard lockup if the reader and the resize thread are on the same CPU?

Hmm, this looks legit. We should at least have a way to break out if it
loops too many times.

> 
> >  		if (trace_empty(iter) && !iter->closed) {
> >  			if (update_last_data_if_empty(iter->tr))
> >  				return 0;  
> [ ... ]

> > -		ref->page = ring_buffer_alloc_read_page(ref->buffer, iter->cpu_file);
> > -		if (IS_ERR(ref->page)) {
> > -			ret = PTR_ERR(ref->page);
> > -			ref->page = NULL;
> > +
> > +new_read_page:
> > +		ret = ring_buffer_alloc_read_page(ref->buffer, iter->cpu_file, &ref->rpage);
> > +		if (ret) {
> >  			kfree(ref);
> >  			break;
> >  		}
> >  		ref->cpu = iter->cpu_file;
> >  
> > -		r = ring_buffer_read_page(ref->buffer, ref->page,
> > -					  len, iter->cpu_file, 1);
> > +		page_size = ring_buffer_read_page_size(ref->rpage);
> > +
> > +		r = -EINVAL;
> > +		if (IS_ALIGNED(*ppos, page_size) && len >= page_size) {
> > +			r = ring_buffer_read_page(ref->buffer, ref->rpage, len, iter->cpu_file, 1);
> > +			if (r == -EAGAIN)
> > +				goto new_read_page;  
> 
> [Severity: High]
> Similar to the read path above, this catches -EAGAIN and loops back to
> new_read_page.
> 
> Because goto new_read_page executes continuously without releasing
> trace_access_lock() or yielding the CPU, can this permanently deadlock the
> system during a resize by preventing the resize thread from acquiring the
> necessary locks?

This also looks legit.

-- Steve

> 
> > +		} else if (!i) {
> > +			/*
> > +			 * If the first iteration fails this is an invalid userspace input.
> > +			 * Otherwise, this is because the subbuf order has been modified. Do not
> > +			 * report an error and finish the read.
> > +			 */
> > +			ret = -EINVAL;
> > +		}  
>