[PATCH] ring-buffer: Do not allow reader to see unfinished moved head

[PATCH] ring-buffer: Do not allow reader to see unfinished moved head

[Steven Rostedt]

From: Steven Rostedt <rostedt@goodmis.org>

The reader of the ring buffer swaps out a blank sub-buffer with the "head"
of the writable ring buffer so that it has sole access to the sub-buffer
and doesn't race with new writes. To do this, when the writer fills the
ring buffer and needs to overwrite the oldest sub-buffer, it has to mvoe
the HEAD of the buffer that the reader will take next.

While it is moving the head, the reader is prevented from doing the swap.
This is accomplished by the writer changing the HEAD flag of the head page
with the UPDATE flag. It then sets the next sub-buffer to have HEAD flag
set as well.

The cpu_buffer->head_page points to the last place the reader saw the head
page. When it needs to acquire a new page from the ring buffer, it will
start at the cpu_buffer->head_page and search for the sub-buffer with
either the HEAD or UPDATE flag set. It will only do a swap if the
sub-buffer has the HEAD flag set, otherwise it waits for the UPDATE flag
to clear to find the new head page. The problem is that this
cpu_buffer->head_page can be stale, and if an interrupt happens while an
event is being written, and the interrupt writes a lot of events, it can
move the head page several sub-buffers from the original head page. If the
cpu_buffer->head_page ended up between the original head page that has the
UPDATE flag set and the new head page with the HEAD flag set, it can allow
the reader to see the new head page and swap that one out while a commit
is still in progress. This can corrupt the ring buffer.

Have the writer update the cpu_buffer->head_page to keep it pointing to
the old head page before it does the update to make sure that the reader
will always start its search outside the UPDATE and new HEAD sub-buffers.

Cc: stable@vger.kernel.org
Fixes: 77ae365eca89 ("ring-buffer: make lockless")
Closes: https://lore.kernel.org/all/20261105-ring-buffer-head-moved-v2-1-05da40cebccc@debian.org/
Reported-by: Breno Leitao <leitao@debian.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/ring_buffer.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index c64e001c3fc0..0d37f8b529d5 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -1494,7 +1494,13 @@ rb_set_head_page(struct ring_buffer_per_cpu *cpu_buffer)
 	if (RB_WARN_ON(cpu_buffer, rb_list_head(list->prev->next) != list))
 		return NULL;
 
-	page = head = cpu_buffer->head_page;
+	/*
+	 * Make sure that the start of the search of the head_page does not
+	 * start between the writer adding the UPDATE and the new HEAD that it
+	 * moves forward.
+	 */
+	/* Sync with the writer moving the HEAD in rb_handle_head_page() */
+	page = head = smp_load_acquire(&cpu_buffer->head_page);
 	/*
 	 * It is possible that the writer moves the header behind
 	 * where we started, and we miss in one loop.
@@ -3721,6 +3727,15 @@ rb_handle_head_page(struct ring_buffer_per_cpu *cpu_buffer,
 		local_sub(rb_page_commit(next_page), &cpu_buffer->entries_bytes);
 		local_inc(&cpu_buffer->pages_lost);
 
+		/*
+		 * To prevent a stale head_page from being between
+		 * the UPDATE and new HEAD pages, set it here to have
+		 * the reader check always find the update before
+		 * finding the HEAD page.
+		 */
+		/* Sync with the reader setting the head page */
+		smp_store_release(&cpu_buffer->head_page, next_page);
+
 		if (cpu_buffer->ring_meta)
 			rb_update_meta_head(cpu_buffer, next_page);
 		/*
-- 
2.53.0

Re: [PATCH] ring-buffer: Do not allow reader to see unfinished moved head

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] ring-buffer: nested irq in rb_handle_head_page() moves
  head_page past UPDATE
- [Medium] ring-buffer: race on head_page between rb_handle_head_page()
  and reader
--

commit f326a3a8a8353c768b90c13ee6f1ad10ec362a2f
Author: Steven Rostedt <rostedt@goodmis.org>

ring-buffer: Do not allow reader to see unfinished moved head

This commit updates cpu_buffer->head_page in rb_handle_head_page() with
smp_store_release() and reads it in rb_set_head_page() with
smp_load_acquire() so a reader does not start scanning for the head page
between a sub-buffer being updated and the newly moved head sub-buffer.

> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index e760d2fe386fb..cf9e0dc10c88f 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -1494,7 +1494,13 @@ rb_set_head_page(struct ring_buffer_per_cpu *cpu_buffer)
>  	if (RB_WARN_ON(cpu_buffer, rb_list_head(list->prev->next) != list))
>  		return NULL;
>  
> -	page = head = cpu_buffer->head_page;
> +	/*
> +	 * Make sure that the start of the search of the head_page does not
> +	 * start between the writer adding the UPDATE and the new HEAD that it
> +	 * moves forward.
> +	 */
> +	/* Sync with the writer moving the HEAD in rb_handle_head_page() */
> +	page = head = smp_load_acquire(&cpu_buffer->head_page);

[Severity: Medium]
Can the reader's non-atomic update of cpu_buffer->head_page in
__rb_get_reader_page() race with the writer's new store in
rb_handle_head_page() and advance cpu_buffer->head_page past the sub-buffer
marked for update?

Before this patch, only the reader modified cpu_buffer->head_page.  In
__rb_get_reader_page(), after rb_head_page_replace() splices reader_page into
the ring buffer, the reader advances cpu_buffer->head_page assuming it still
points to the replaced reader sub-buffer:

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;
	rb_inc_page(&cpu_buffer->head_page);
    ...
	goto again;
}

If the writer on the traced CPU moves onto the newly spliced reader_page (R),
fills it, and enters rb_handle_head_page() to overwrite the next sub-buffer
(P2) before the reader runs rb_inc_page(), both sides update
cpu_buffer->head_page without synchronization:

CPU0 (reader)                     CPU1 (writer)
__rb_get_reader_page()
  rb_set_head_page()
    head_page = P1
  rb_head_page_replace(P1, R)
  P2->list.prev = &R->list
                                  rb_handle_head_page(..., R, P2)
                                    rb_head_page_set_update(..., P2, R, ...)
                                    smp_store_release(&head_page, P2)
                                    rb_head_page_set_head(..., P3, P2, ...)
  rb_inc_page(&head_page)
    head_page = P3
  goto again
  rb_set_head_page()
    page = head_page (P3)  <-- skips P2 update, swaps P3

When __rb_get_reader_page() loops back to "again", rb_set_head_page() starts
its scan at P3 instead of P2, misses the in-progress update on P2, and swaps
out P3 while the writer is still moving the head page, corrupting the ring
buffer.

> @@ -3719,6 +3725,15 @@ rb_handle_head_page(struct ring_buffer_per_cpu *cpu_buffer,
>  		local_sub(rb_page_commit(next_page), &cpu_buffer->entries_bytes);
>  		local_inc(&cpu_buffer->pages_lost);
>  
> +		/*
> +		 * To prevent a stale head_page from being between
> +		 * the UPDATE and new HEAD pages, set it here to have
> +		 * the reader check always find the update before
> +		 * finding the HEAD page.
> +		 */
> +		/* Sync with the reader setting the head page */
> +		smp_store_release(&cpu_buffer->head_page, next_page);

[Severity: Medium]
Can a nested interrupt that moves the head page across multiple sub-buffers
overwrite cpu_buffer->head_page and still place it past the outer writer's
sub-buffer under update?

When an outer writer in rb_handle_head_page() changes P0->list.next (P1) to
the update state, it gets type == RB_PAGE_HEAD and sets cpu_buffer->head_page
to P1.  If an interrupt then preempts the writer before it restores
P0->list.next to normal and writes enough events to wrap across two
sub-buffers (P1 and P2):

1. On the interrupt's first call to rb_handle_head_page(tail=P0, next=P1),
   rb_head_page_set_update() sees the outer update on P0->list.next and
   returns RB_PAGE_UPDATE.  The interrupt sets P1->list.next (P2) as the new
   head page and advances tail_page to P1, leaving P0->list.next in the
   update state.

2. When the interrupt fills P1 and calls rb_handle_head_page(tail=P1,
   next=P2), P1->list.next has the head flag set, so
   rb_head_page_set_update() succeeds and returns RB_PAGE_HEAD to the
   interrupt:

rb_handle_head_page(..., P0, P1)
  rb_head_page_set_update(..., P1, P0, ...)
  smp_store_release(&head_page, P1)
  <hardirq>
    rb_handle_head_page(..., P0, P1)
      rb_head_page_set_update() -> RB_PAGE_UPDATE
      rb_head_page_set_head(..., P2, P1, ...)
    ...
    rb_handle_head_page(..., P1, P2)
      rb_head_page_set_update() -> RB_PAGE_HEAD
      smp_store_release(&head_page, P2)  <-- moves head_page past P1
      rb_head_page_set_head(..., P3, P2, ...)
      rb_head_page_set_normal(..., P2, P1, ...)

At that point P1 is still in the update state from the outer writer, P3 is
the new head page, and cpu_buffer->head_page has been advanced to P2 (between
P1 and P3).  Furthermore, when the outer writer resumes at
rb_head_page_set_head(..., P2, P1, ...), it transiently sets the head flag on
P2 before resetting it to normal.  A concurrent reader on another CPU calling
rb_set_head_page() starts its scan at P2, never sees P1's update flag, and
can swap out P2 (which is cpu_buffer->tail_page) or P3 while the outer writer
is still in progress.

Also, if the interrupt arrives between rb_head_page_set_update() and
smp_store_release(&cpu_buffer->head_page, next_page), the interrupt sees
RB_PAGE_UPDATE and sets the new head page before cpu_buffer->head_page has
been updated from its stale value.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109032241.55e57d26@fedora?part=1

Re: [PATCH] ring-buffer: Do not allow reader to see unfinished moved head

[Breno Leitao]

Steve,

Thank for the patch,

On Mon, Nov 09, 2026 at 08:39:43AM +0000, sashiko-bot@kernel.org wrote:
> > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> > index e760d2fe386fb..cf9e0dc10c88f 100644
> > --- a/kernel/trace/ring_buffer.c
> > +++ b/kernel/trace/ring_buffer.c
> > @@ -1494,7 +1494,13 @@ rb_set_head_page(struct ring_buffer_per_cpu *cpu_buffer)
> >  	if (RB_WARN_ON(cpu_buffer, rb_list_head(list->prev->next) != list))
> >  		return NULL;
> >  
> > -	page = head = cpu_buffer->head_page;
> > +	/*
> > +	 * Make sure that the start of the search of the head_page does not
> > +	 * start between the writer adding the UPDATE and the new HEAD that it
> > +	 * moves forward.
> > +	 */
> > +	/* Sync with the writer moving the HEAD in rb_handle_head_page() */
> > +	page = head = smp_load_acquire(&cpu_buffer->head_page);
> 
> [Severity: Medium]
> Can the reader's non-atomic update of cpu_buffer->head_page in
> __rb_get_reader_page() race with the writer's new store in
> rb_handle_head_page() and advance cpu_buffer->head_page past the sub-buffer
> marked for update?

I've asked my robot to test this case, and it was still able to
reproduce the race, even with this patch.

Re: [PATCH] ring-buffer: Do not allow reader to see unfinished moved head

[Steven Rostedt]

On Mon, 9 Nov 2026 05:04:50 -0800
Breno Leitao <leitao@debian.org> wrote:

> Steve,
> 
> Thank for the patch,
> 
> On Mon, Nov 09, 2026 at 08:39:43AM +0000, sashiko-bot@kernel.org wrote:
> > > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> > > index e760d2fe386fb..cf9e0dc10c88f 100644
> > > --- a/kernel/trace/ring_buffer.c
> > > +++ b/kernel/trace/ring_buffer.c
> > > @@ -1494,7 +1494,13 @@ rb_set_head_page(struct ring_buffer_per_cpu *cpu_buffer)
> > >  	if (RB_WARN_ON(cpu_buffer, rb_list_head(list->prev->next) != list))
> > >  		return NULL;
> > >  
> > > -	page = head = cpu_buffer->head_page;
> > > +	/*
> > > +	 * Make sure that the start of the search of the head_page does not
> > > +	 * start between the writer adding the UPDATE and the new HEAD that it
> > > +	 * moves forward.
> > > +	 */
> > > +	/* Sync with the writer moving the HEAD in rb_handle_head_page() */
> > > +	page = head = smp_load_acquire(&cpu_buffer->head_page);  
> > 
> > [Severity: Medium]
> > Can the reader's non-atomic update of cpu_buffer->head_page in
> > __rb_get_reader_page() race with the writer's new store in
> > rb_handle_head_page() and advance cpu_buffer->head_page past the sub-buffer
> > marked for update?  
> 
> I've asked my robot to test this case, and it was still able to
> reproduce the race, even with this patch.


Are you still testing with the minimum of 7K? Can you up it to can you
echo 11 into buffer_size_kb to see if it is an issue with three
sub-buffers?

I just want to know if it you are catching issues with something that
looks like this:

                             tail
                              v
   +--HEAD--> subbuf ----> subbuf --+
   |                                |
   +--------------------------------+

Two subbufs should still be fixed but I want to make sure I'm not
looking at two different bugs.

-- Steve