Re: [PATCH v6 1/2] tracing: Fix subbuf resize races with trace_pipe_raw readers

On Fri, Aug 14, 2026 at 04:01:30PM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The local variable `spare_size` is left uninitialized via `goto read;` in `tracing_buffers_read()`, resulting in a kernel memory leak to userspace.
> --
> 
> --- Patch [1]: [PATCH v6 1/2] tracing: Fix subbuf resize races with trace_pipe_raw readers ---
> commit bdc760fe7b1aa6eb8c8a8b8c38e9fd02d25051b6
> Author: Vincent Donnefort <[email protected]>
> 
> tracing: Fix subbuf resize races with trace_pipe_raw readers
> 
> Concurrent subbuffer resizes may crash trace_pipe_raw readers or leak
> uninitialized memory to userspace due to stale size values. This commit
> modifies ring_buffer_alloc_read_page() to handle resizing and introduces
> ring_buffer_read_page_size().
> 
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index 395238b2b715c..355278083b3fb 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> [ ... ]
> > @@ -7091,36 +7091,21 @@ ssize_t tracing_buffers_read(struct file *filp, char __user *ubuf,
> >  	if (iter->snapshot && tracer_uses_snapshot(iter->tr->current_trace))
> >  		return -EBUSY;
> >  
> > -	page_size = ring_buffer_subbuf_size_get(iter->array_buffer->buffer);
> > -
> > -	/* Make sure the spare matches the current sub buffer size */
> > -	if (info->spare) {
> > -		if (page_size != info->spare_size) {
> > -			ring_buffer_free_read_page(iter->array_buffer->buffer,
> > -						   info->spare_cpu, info->spare);
> > -			info->spare = NULL;
> > -		}
> > -	}
> > +again:
> > +	/* Do we have previous read data to read? */
> > +	if (info->spare && (info->read < ring_buffer_read_page_size(info->spare)))
> > +		goto read;
> >  
> > -	if (!info->spare) {
> > -		info->spare = ring_buffer_alloc_read_page(iter->array_buffer->buffer,
> > -							  iter->cpu_file);
> > -		if (IS_ERR(info->spare)) {
> > -			ret = PTR_ERR(info->spare);
> > -			info->spare = NULL;
> > -		} else {
> > -			info->spare_cpu = iter->cpu_file;
> > -			info->spare_size = page_size;
> > -		}
> > -	}
> > -	if (!info->spare)
> > +	/* Make sure the read page order is aligned with the current buffer subbuf order */
> > +	ret = ring_buffer_alloc_read_page(iter->array_buffer->buffer, iter->cpu_file,
> > +					  &info->spare);
> > +	if (ret < 0)
> >  		return ret;
> >  
> > -	/* Do we have previous read data to read? */
> > -	if (info->read < page_size)
> > -		goto read;
> > +	spare_size = ring_buffer_read_page_size(info->spare);
> > +	info->read = spare_size;
> > +	info->spare_cpu = iter->cpu_file;
> >  
> > - again:
> >  	trace_access_lock(iter->cpu_file);
> >  	ret = ring_buffer_read_page(iter->array_buffer->buffer,
> >  				    info->spare,
> [ ... ]
> > @@ -7146,8 +7133,9 @@ ssize_t tracing_buffers_read(struct file *filp, char __user *ubuf,
> >  	}
> >  
> >  	info->read = 0;
> > +
> >   read:
> > -	size = page_size - info->read;
> > +	size = spare_size - info->read;
> 
> [Severity: High]
> Can this regression leak uninitialized stack memory to userspace?
> 
> If a userspace process does a partial read (consuming fewer bytes than a full
> subbuffer page), the next read syscall will take the goto read; branch at the
> top of tracing_buffers_read().
> 
> When that branch is taken, spare_size is completely uninitialized stack memory.
> 
> Could size end up being a large garbage value (or underflow if spare_size <
> info->read), causing copy_to_user() below to read beyond the bounds of
> info->spare and leak arbitrary kernel memory?

sigh... I hope I will eventually get this right!

	 again:
	-       /* Do we have previous read data to read? */
	-       if (info->spare && (info->read < ring_buffer_read_page_size(info->spare)))
	-               goto read;
	+       if (info->spare) {
	+               spare_size = ring_buffer_read_page_size(info->spare);
	+               /* Do we have previous read data to read? */
	+               if (info->read < spare_size)
	+                       goto read;
	+       }

This should do.

> 
> >  	if (size > count)
> >  		size = count;
> >  	trace_data = ring_buffer_read_page_data(info->spare);
> >  	ret = copy_to_user(ubuf, trace_data + info->read, size);
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

-- 
Vincent