Re: [PATCH v10 2/4] tracing: Fix subbuf resize races with trace_pipe_raw readers

[Steven Rostedt <rostedt@goodmis.org>]

On Fri,  4 Sep 2026 17:44:48 +0100
Vincent Donnefort <vdonnefort@google.com> wrote:

> @@ -7306,25 +7281,37 @@ ssize_t tracing_buffers_splice_read(struct file *file, loff_t *ppos,
>  
>  		refcount_set(&ref->refcount, 1);
>  		ref->buffer = iter->array_buffer->buffer;
> -		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;
> +
> +		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);
> +		} else if (!i) {
> +			/*
> +			 * We failed to read because the length is too small
> +			 * or unaligned. If this is the first iteration, it's
> +			 * an invalid userspace input. Otherwise, this is due
> +			 * to a subbuf order change. Do not report an error
> +			 * and just finish the read.

This isn't quite true. It can be an invalid length and not the first
iteration. If you ask for a length that isn't subbuffer aligned but greater
than one subbuffer in size it will work the first iteration but fail at the
end where it couldn't get a full page.

That is valid but would also trigger this path.

This is the only issue I have with this patch set. I'll just take it as is
now. We can fix the comment later. I want to start testing it and get it to
Linus before the next RC release is out. If it fails the tests, then we can
fix the comment as it will not make the next release.

-- Steve


> +			 */
> +			ret = -EINVAL;
> +		}
> +
>  		if (r < 0) {
> -			ring_buffer_free_read_page(ref->buffer, ref->cpu,
> -						   ref->page);
> +			ring_buffer_free_read_page(ref->buffer, ref->cpu, ref->rpage);
>  			kfree(ref);
>  			break;
>  		}
>  
> -		page = virt_to_page(ring_buffer_read_page_data(ref->page));
> +		page = virt_to_page(ring_buffer_read_page_data(ref->rpage));
>  
>  		spd.pages[i] = page;
>  		spd.partial[i].len = page_size;