Re: [PATCH v1 3/4] ring-buffer: Order remote page commit with data and timestamp

[Vincent Donnefort <vdonnefort@google.com>]

On Fri, Nov 13, 2026 at 03:34:59PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found no new issues
> in this patch, but noticed 1 related pre-existing issue it does not
> introduce:
> - [Critical] tracing: out-of-bounds write in simple_rb_reserve_next()
>   [1]
> --
> 
> commit b0c88540d75d93c7a85149df8d188a1bfaa9a89e
> Author: Vincent Donnefort <vdonnefort@google.com>
> 
> ring-buffer: Order remote page commit with data and timestamp
> 
> This commit adds memory barriers between page::commit, page::data, and
> page::time_stamp for remote ring-buffers and defers updating read_stamp
> until committed data is present on the reader page.
> 
> > diff --git a/kernel/trace/simple_ring_buffer.c b/kernel/trace/simple_ring_buffer.c
> > index 5f14ba152fba6..2dddaef638dc6 100644
> > --- a/kernel/trace/simple_ring_buffer.c
> > +++ b/kernel/trace/simple_ring_buffer.c
> > @@ -307,6 +307,11 @@ EXPORT_SYMBOL_GPL(simple_ring_buffer_reserve);
> >   */
> >  void simple_ring_buffer_commit(struct simple_rb_per_cpu *cpu_buffer)
> >  {
> > +	/*
> > +	 * Order page::data and page::time_stamp before page::commit. Paired
> > +	 * with __rb_get_reader_page_from_remote().
> > +	 */
> > +	smp_wmb();
> >  	local_set(&cpu_buffer->tail_page->page->commit,
> >  		  cpu_buffer->tail_page->write);
> 
> [Severity: Critical]
> This problem wasn't introduced by this patch, but can
> simple_ring_buffer_reserve() return a pointer whose reservation overflows
> tail->page->data [1]?
> 
> In simple_rb_reserve_next(), when an event exceeds PAGE_SIZE -
> BUF_PAGE_HDR_SIZE, simple_rb_move_tail() rotates to an empty page where
> !tail->entries is true, and event_size is assigned to tail->write without
> checking that it fits on a fresh page:
> 
> kernel/trace/simple_ring_buffer.c:simple_rb_reserve_next() {
> 	...
> 	if (unlikely(write > (PAGE_SIZE - BUF_PAGE_HDR_SIZE)))
> 		tail = simple_rb_move_tail(cpu_buffer);
> 
> 	if (!tail->entries) {
> 		tail->page->time_stamp = timestamp;
> 		time_delta = 0;
> 		ts_ext_size = 0;
> 		write = event_size;
> 		prev_write = 0;
> 	}
> 
> 	tail->write = write;
> 	...
> }
> 
> Writing the caller's payload to the returned event pointer then writes past
> the end of the page buffer and corrupts adjacent memory.
> 
> [1] https://sashiko.dev/#/bug/linux-b2128783-1787-4f13-b863-090db5378e74
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261113152729.837429-1-vdonnefort@google.com?part=3

There's absolutely no existing event that is big enough to a ring buffer page!

-- 
Vincent