[PATCH v3] tracing: ring-buffer: serialize shrink with NMI readers

[PATCH v3] tracing: ring-buffer: serialize shrink with NMI readers

[Haoli Zhang]

The syzbot report shows a use-after-free in rb_is_reader_page(), reached
from rb_move_tail(), with the freed page released by rb_remove_pages().
Source inspection suggests that a detached reader page can retain a link
into the range removed during a shrink. This patch addresses that possible
stale-link path and coordinates NMI readers with page removal.

When an NMI reader cannot acquire reader_lock, rb_reader_lock() falls back
to reading the ring buffer without the lock. Gate lockless NMI readers
while rb_remove_pages() removes pages. Drain readers that entered before
the gate before changing the page list. New NMI readers that cannot acquire
reader_lock while the gate is closed skip that CPU. Readers that acquire
reader_lock remain serialized by it. The drain loop calls cond_resched()
while waiting for readers to exit.

During shrink, redirect reader_page->list.prev if it points to a page being
removed. Also detect a stale reader_page->list.next and redirect it after
reconnecting the ring to the surviving successor link, including the
transferred head flag.

When an NMI reader skips a CPU while resize_in_progress is set, treat that
CPU buffer as empty in the empty-query helpers. This lets ftrace_dump_one()
make progress instead of retrying forever when peek/consume also skip the buffer. Entries on a skipped CPU may be omitted from the dump.
Clear event on every consume retry. Cache whether it is padding
before unlocking. This avoids reading event fields after unlock.
Clear timestamps on skipped peek and consume operations.

The NMI fallback increment of record_disabled is intentionally retained.
The existing fallback comment says an unlocked NMI read may corrupt the
buffer and that recording must remain disabled afterward; the existing
unlock path does not undo that disable. The nmi_readers counter is separate
and tracks only the temporary lifetime of a lockless NMI read.

Fixes: 83f40318dab0 ("ring-buffer: Make removal of ring buffer pages atomic")
Reported-by: syzbot+37e1793bdb0061ea3a47@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=37e1793bdb0061ea3a47
Co-developed-by: Jinning Zhang <25031212117@stu.xidian.edu.cn>
Signed-off-by: Jinning Zhang <25031212117@stu.xidian.edu.cn>
Co-developed-by: Bin Yu <byu@xidian.edu.cn>
Signed-off-by: Bin Yu <byu@xidian.edu.cn>
Signed-off-by: Haoli Zhang <26031212468@stu.xidian.edu.cn>

---
v3:
  - Treat skipped empty-buffer queries as empty so NMI trace dumps can progress.
  - Clear the event pointer before each consume retry.
  - Cache padding retry decisions before dropping reader protection.
  - Document that dumps may omit records from CPUs skipped while the gate is set.
  - Call cond_resched() while draining NMI readers in the shrink slow path.
  - Add the Fixes tag.
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 04bb94c..9e52506 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -522,6 +522,8 @@ struct ring_buffer_per_cpu {
 	int				cpu;
 	atomic_t			record_disabled;
 	atomic_t			resize_disabled;
+	atomic_t			resize_in_progress;
+	atomic_t			nmi_readers;
 	struct trace_buffer		*buffer;
 	raw_spinlock_t			reader_lock;	/* serialize readers */
 	arch_spinlock_t			lock;
@@ -2578,6 +2580,8 @@ rb_allocate_cpu_buffer(struct trace_buffer *buffer, unsigned long nr_pages, int
 
 	cpu_buffer->cpu = cpu;
 	cpu_buffer->buffer = buffer;
+	atomic_set(&cpu_buffer->resize_in_progress, 0);
+	atomic_set(&cpu_buffer->nmi_readers, 0);
 	raw_spin_lock_init(&cpu_buffer->reader_lock);
 	lockdep_set_class(&cpu_buffer->reader_lock, buffer->reader_lock_key);
 	cpu_buffer->lock = (arch_spinlock_t)__ARCH_SPIN_LOCK_UNLOCKED;
@@ -3074,10 +3078,18 @@ rb_remove_pages(struct ring_buffer_per_cpu *cpu_buffer, unsigned long nr_pages)
 	struct buffer_page *last_page, *first_page;
 	unsigned long nr_removed;
 	unsigned long head_bit;
+	bool reader_page_next_removed;
 	int page_entries;
 
 	head_bit = 0;
+	reader_page_next_removed = false;
 
+	/* Stop lockless NMI readers before unlinking pages. */
+	atomic_set_release(&cpu_buffer->resize_in_progress, 1);
+	/* Pair with the NMI reader's count increment and gate check. */
+	smp_mb();
+	while (atomic_read_acquire(&cpu_buffer->nmi_readers))
+		cond_resched();
 	raw_spin_lock_irq(&cpu_buffer->reader_lock);
 	atomic_inc(&cpu_buffer->record_disabled);
 	/*
@@ -3106,6 +3118,19 @@ rb_remove_pages(struct ring_buffer_per_cpu *cpu_buffer, unsigned long nr_pages)
 	for (nr_removed = 0; nr_removed < nr_pages; nr_removed++) {
 		to_remove = rb_list_head(to_remove)->next;
 		head_bit |= (unsigned long)to_remove & RB_PAGE_HEAD;
+
+		/*
+		 * The reader page is not part of the ring buffer list, so its
+		 * previous pointer may still point to a page being removed.
+		 * rb_is_reader_page() follows this pointer, so redirect it to a
+		 * page that will remain in the ring buffer.
+		 */
+		if (cpu_buffer->reader_page->list.prev == rb_list_head(to_remove))
+			cpu_buffer->reader_page->list.prev = tail_page;
+		/* The detached reader page can also retain a stale forward link. */
+		if (rb_list_head(cpu_buffer->reader_page->list.next) ==
+		    rb_list_head(to_remove))
+			reader_page_next_removed = true;
 	}
 	/* Read iterators need to reset themselves when some pages removed */
 	cpu_buffer->pages_removed += nr_removed;
@@ -3119,6 +3144,9 @@ rb_remove_pages(struct ring_buffer_per_cpu *cpu_buffer, unsigned long nr_pages)
 	 */
 	tail_page->next = (struct list_head *)((unsigned long)next_page |
 						head_bit);
+	/* Keep the detached reader page's forward link off removed pages too. */
+	if (reader_page_next_removed)
+		cpu_buffer->reader_page->list.next = tail_page->next;
 	next_page = rb_list_head(next_page);
 	next_page->prev = tail_page;
 
@@ -3134,6 +3162,7 @@ rb_remove_pages(struct ring_buffer_per_cpu *cpu_buffer, unsigned long nr_pages)
 	/* pages are removed, resume tracing and then free the pages */
 	atomic_dec(&cpu_buffer->record_disabled);
 	raw_spin_unlock_irq(&cpu_buffer->reader_lock);
+	atomic_set_release(&cpu_buffer->resize_in_progress, 0);
 
 	RB_WARN_ON(cpu_buffer, list_empty(cpu_buffer->pages));
 
@@ -6293,11 +6322,18 @@ rb_iter_peek(struct ring_buffer_iter *iter, u64 *ts)
 }
 EXPORT_SYMBOL_GPL(ring_buffer_iter_peek);
 
-static inline bool rb_reader_lock(struct ring_buffer_per_cpu *cpu_buffer)
+enum rb_reader_lock_state {
+	RB_READER_LOCKED,
+	RB_READER_NMI,
+	RB_READER_SKIP,
+};
+
+static inline enum rb_reader_lock_state
+rb_reader_lock(struct ring_buffer_per_cpu *cpu_buffer)
 {
 	if (likely(!in_nmi())) {
 		raw_spin_lock(&cpu_buffer->reader_lock);
-		return true;
+		return RB_READER_LOCKED;
 	}
 
 	/*
@@ -6310,18 +6346,35 @@ static inline bool rb_reader_lock(struct ring_buffer_per_cpu *cpu_buffer)
 	 * Reading from NMI is a oneshot deal.
 	 */
 	if (raw_spin_trylock(&cpu_buffer->reader_lock))
-		return true;
+		return RB_READER_LOCKED;
+
+	/*
+	 * A resize cannot wait for an NMI that interrupted the CPU doing the
+	 * resize. Track lockless NMI readers so page removal can stop new ones
+	 * and wait for existing readers before changing the page list.
+	 */
+	atomic_inc(&cpu_buffer->nmi_readers);
+	/* Pair with the resize gate before deciding to read without the lock. */
+	smp_mb__after_atomic();
+	if (atomic_read_acquire(&cpu_buffer->resize_in_progress)) {
+		atomic_dec_return_release(&cpu_buffer->nmi_readers);
+		return RB_READER_SKIP;
+	}
 
 	/* Continue without locking, but disable the ring buffer */
 	atomic_inc(&cpu_buffer->record_disabled);
-	return false;
+
+	return RB_READER_NMI;
 }
 
 static inline void
-rb_reader_unlock(struct ring_buffer_per_cpu *cpu_buffer, bool locked)
+rb_reader_unlock(struct ring_buffer_per_cpu *cpu_buffer,
+		 enum rb_reader_lock_state state)
 {
-	if (likely(locked))
+	if (likely(state == RB_READER_LOCKED))
 		raw_spin_unlock(&cpu_buffer->reader_lock);
+	else if (state == RB_READER_NMI)
+		atomic_dec_return_release(&cpu_buffer->nmi_readers);
 }
 
 /**
@@ -6341,21 +6394,30 @@ ring_buffer_peek(struct trace_buffer *buffer, int cpu, u64 *ts,
 	struct ring_buffer_per_cpu *cpu_buffer = buffer->buffers[cpu];
 	struct ring_buffer_event *event;
 	unsigned long flags;
-	bool dolock;
+	enum rb_reader_lock_state state;
+	bool retry;
 
 	if (!cpumask_test_cpu(cpu, buffer->cpumask))
 		return NULL;
 
  again:
+	retry = false;
 	local_irq_save(flags);
-	dolock = rb_reader_lock(cpu_buffer);
-	event = rb_buffer_peek(cpu_buffer, ts, lost_events);
-	if (event && event->type_len == RINGBUF_TYPE_PADDING)
+	state = rb_reader_lock(cpu_buffer);
+	if (state == RB_READER_SKIP) {
+		if (ts)
+			*ts = 0;
+		event = NULL;
+	} else {
+		event = rb_buffer_peek(cpu_buffer, ts, lost_events);
+	}
+	retry = event && event->type_len == RINGBUF_TYPE_PADDING;
+	if (retry)
 		rb_advance_reader(cpu_buffer);
-	rb_reader_unlock(cpu_buffer, dolock);
+	rb_reader_unlock(cpu_buffer, state);
 	local_irq_restore(flags);
 
-	if (event && event->type_len == RINGBUF_TYPE_PADDING)
+	if (retry)
 		goto again;
 
 	return event;
@@ -6416,9 +6478,12 @@ ring_buffer_consume(struct trace_buffer *buffer, int cpu, u64 *ts,
 	struct ring_buffer_per_cpu *cpu_buffer;
 	struct ring_buffer_event *event = NULL;
 	unsigned long flags;
-	bool dolock;
+	enum rb_reader_lock_state state;
+	bool retry;
 
  again:
+	retry = false;
+	event = NULL;
 	/* might be called in atomic */
 	preempt_disable();
 
@@ -6427,21 +6492,25 @@ ring_buffer_consume(struct trace_buffer *buffer, int cpu, u64 *ts,
 
 	cpu_buffer = buffer->buffers[cpu];
 	local_irq_save(flags);
-	dolock = rb_reader_lock(cpu_buffer);
+	state = rb_reader_lock(cpu_buffer);
 
-	event = rb_buffer_peek(cpu_buffer, ts, lost_events);
+	if (state != RB_READER_SKIP)
+		event = rb_buffer_peek(cpu_buffer, ts, lost_events);
+	else if (ts)
+		*ts = 0;
+	retry = event && event->type_len == RINGBUF_TYPE_PADDING;
 	if (event) {
 		cpu_buffer->lost_events = 0;
 		rb_advance_reader(cpu_buffer);
 	}
 
-	rb_reader_unlock(cpu_buffer, dolock);
+	rb_reader_unlock(cpu_buffer, state);
 	local_irq_restore(flags);
 
  out:
 	preempt_enable();
 
-	if (event && event->type_len == RINGBUF_TYPE_PADDING)
+	if (retry)
 		goto again;
 
 	return event;
@@ -6843,7 +6912,7 @@ bool ring_buffer_empty(struct trace_buffer *buffer)
 {
 	struct ring_buffer_per_cpu *cpu_buffer;
 	unsigned long flags;
-	bool dolock;
+	enum rb_reader_lock_state state;
 	bool ret;
 	int cpu;
 
@@ -6851,9 +6920,10 @@ bool ring_buffer_empty(struct trace_buffer *buffer)
 	for_each_buffer_cpu(buffer, cpu) {
 		cpu_buffer = buffer->buffers[cpu];
 		local_irq_save(flags);
-		dolock = rb_reader_lock(cpu_buffer);
-		ret = rb_per_cpu_empty(cpu_buffer);
-		rb_reader_unlock(cpu_buffer, dolock);
+		state = rb_reader_lock(cpu_buffer);
+		/* Do not make an NMI dumper spin on a buffer being resized. */
+		ret = state == RB_READER_SKIP || rb_per_cpu_empty(cpu_buffer);
+		rb_reader_unlock(cpu_buffer, state);
 		local_irq_restore(flags);
 
 		if (!ret)
@@ -6873,7 +6943,7 @@ bool ring_buffer_empty_cpu(struct trace_buffer *buffer, int cpu)
 {
 	struct ring_buffer_per_cpu *cpu_buffer;
 	unsigned long flags;
-	bool dolock;
+	enum rb_reader_lock_state state;
 	bool ret;
 
 	if (!cpumask_test_cpu(cpu, buffer->cpumask))
@@ -6881,9 +6951,10 @@ bool ring_buffer_empty_cpu(struct trace_buffer *buffer, int cpu)
 
 	cpu_buffer = buffer->buffers[cpu];
 	local_irq_save(flags);
-	dolock = rb_reader_lock(cpu_buffer);
-	ret = rb_per_cpu_empty(cpu_buffer);
-	rb_reader_unlock(cpu_buffer, dolock);
+	state = rb_reader_lock(cpu_buffer);
+	/* Do not make an NMI dumper spin on a buffer being resized. */
+	ret = state == RB_READER_SKIP || rb_per_cpu_empty(cpu_buffer);
+	rb_reader_unlock(cpu_buffer, state);
 	local_irq_restore(flags);
 
 	return ret;

Re: [PATCH v3] tracing: ring-buffer: serialize shrink with NMI readers

[Steven Rostedt]

On Wed, 11 Nov 2026 17:43:01 +0800
Haoli Zhang <26031212468@stu.xidian.edu.cn> wrote:

> The syzbot report shows a use-after-free in rb_is_reader_page(), reached
> from rb_move_tail(), with the freed page released by rb_remove_pages().
> Source inspection suggests that a detached reader page can retain a link
> into the range removed during a shrink. This patch addresses that possible
> stale-link path and coordinates NMI readers with page removal.
> 
> When an NMI reader cannot acquire reader_lock, rb_reader_lock() falls back

What NMI readers?

The only time NMI reads the ring buffer is on a crash

> to reading the ring buffer without the lock. Gate lockless NMI readers

Yes, it reads without a lock because the system crashed and the NMI
handler is doing a *BEST EFFORT* to read the ring buffer to see if it
can produce output that tells us why the system crashed.

> while rb_remove_pages() removes pages. Drain readers that entered before
> the gate before changing the page list. New NMI readers that cannot acquire
> reader_lock while the gate is closed skip that CPU. Readers that acquire
> reader_lock remain serialized by it. The drain loop calls cond_resched()
> while waiting for readers to exit.
> 
> During shrink, redirect reader_page->list.prev if it points to a page being
> removed. Also detect a stale reader_page->list.next and redirect it after
> reconnecting the ring to the surviving successor link, including the
> transferred head flag.
> 
> When an NMI reader skips a CPU while resize_in_progress is set, treat that
> CPU buffer as empty in the empty-query helpers. This lets ftrace_dump_one()
> make progress instead of retrying forever when peek/consume also skip the buffer. Entries on a skipped CPU may be omitted from the dump.
> Clear event on every consume retry. Cache whether it is padding
> before unlocking. This avoids reading event fields after unlock.
> Clear timestamps on skipped peek and consume operations.
> 
> The NMI fallback increment of record_disabled is intentionally retained.
> The existing fallback comment says an unlocked NMI read may corrupt the
> buffer and that recording must remain disabled afterward; the existing
> unlock path does not undo that disable. The nmi_readers counter is separate
> and tracks only the temporary lifetime of a lockless NMI read.
> 
> Fixes: 83f40318dab0 ("ring-buffer: Make removal of ring buffer pages atomic")

This is not a fix.

> Reported-by: syzbot+37e1793bdb0061ea3a47@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=37e1793bdb0061ea3a47

No the bug report has NOTHING TO DO WITH NMIs!


> Co-developed-by: Jinning Zhang <25031212117@stu.xidian.edu.cn>
> Signed-off-by: Jinning Zhang <25031212117@stu.xidian.edu.cn>
> Co-developed-by: Bin Yu <byu@xidian.edu.cn>
> Signed-off-by: Bin Yu <byu@xidian.edu.cn>
> Signed-off-by: Haoli Zhang <26031212468@stu.xidian.edu.cn>
> 
> ---
> v3:

I'm not adding this complexity to handle the best effort case of an NMI
reading the ring buffer during a crash.

NAK!

-- Steve