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

[sashiko-bot@kernel.org]

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