Re: [PATCH] ring-buffer: Let the reader spin on head page update

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] ring-buffer: stale list.prev dereference in
  rb_handle_head_page() writer loop
--

commit 42f758b28dc946f5aa06647c36c046a615ee80cf
Author: Vincent Donnefort <vdonnefort@google.com>

ring-buffer: Let the reader spin on head page update

This commit introduces the RB_PAGE_HEAD_UPDATE flag to mark newly moved head
page links while a writer update is in progress, converting the final link
to RB_PAGE_HEAD only when the outer writer finishes rb_handle_head_page().

> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 1fd28a72e6ee1..73f31e6719f98 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c

[ ... ]

> @@ -3833,6 +3849,24 @@ rb_handle_head_page(struct ring_buffer_per_cpu *cpu_buffer,
>  			return -1;
>  	}
>  
> +	if (type == RB_PAGE_HEAD) {
> +		struct buffer_page *prev_page;
> +
> +		do {
> +			/* Interrupts might have moved the head further  */
> +			while (!rb_is_head_page(new_head, new_head->list.prev))
> +				rb_inc_page(&new_head);
> +
> +			prev_page = new_head;
> +			rb_dec_page(&prev_page);
> +
> +			/* Try to release the reader */
> +			ret = rb_head_page_set_head(cpu_buffer, new_head, prev_page,
> +						    RB_PAGE_HEAD_UPDATE);
> +
> +		} while (ret != (RB_PAGE_HEAD_UPDATE));
> +	}
> +
>  	return 0;
>  }

[Severity: Medium]
Can reading new_head->list.prev and rb_dec_page(&prev_page) here inspect a
stale predecessor page and spin in rb_handle_head_page()?

Lockless writers cannot rely on the head page's list.prev pointer because
__rb_get_reader_page() and rb_insert_pages() update the forward list.next
pointer via cmpxchg before updating the following page's list.prev pointer:

kernel/trace/ring_buffer.c:__rb_get_reader_page() {
	...
	ret = rb_head_page_replace(reader, cpu_buffer->reader_page);
	if (!ret)
		goto spin;
	...
	rb_list_head(reader->list.next)->prev = &cpu_buffer->reader_page->list;
	...
}

If a writer enters rb_handle_head_page() in the window after a reader swaps
the head page (or after rb_insert_pages() splices in new pages) but before
list.prev is updated, new_head->list.prev still points to the old predecessor:

CPU0 (reader)                     CPU1 (writer)
__rb_get_reader_page()
  rb_head_page_replace(reader,
    cpu_buffer->reader_page)
                                  rb_handle_head_page()
                                    /* tail_page is reader_page->prev,
                                     * next_page is reader_page,
                                     * new_head is reader->next
                                     */
                                    rb_head_page_set_head_update(...,
                                      new_head, next_page, ...)
                                    rb_head_page_set_normal(...,
                                      next_page, tail_page, ...)
                                    rb_is_head_page(new_head,
                                      new_head->list.prev)
                                      /* inspects reader->list.next
                                       * (RB_PAGE_NORMAL) instead of
                                       * reader_page->list.next
                                       * (RB_PAGE_HEAD_UPDATE)
                                       */
                                    rb_inc_page(&new_head)  <-- misses head
  rb_list_head(reader->list.next)->prev =
    &cpu_buffer->reader_page->list;

Because every other link in the ring has RB_PAGE_NORMAL, the inner while loop
wraps around the ring buffer until CPU0 updates list.prev, or deadlocks if the
writer is an NMI that interrupted __rb_get_reader_page() or rb_insert_pages()
on the same CPU.

Since new_head starts as the page right after next_page, tracking prev_page
forward from next_page as new_head advances via rb_inc_page() would avoid
touching list.prev on the writer path.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111160624.1294946-1-vdonnefort@google.com?part=1