Skip to contentSkip to conversation

tracing: More fixes for 7.3

PATCH New Steven Rostedt 1 participant 4 messages Aug 30

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

cover tracing: More fixes for 7.3

More fixes for 7.3:

- Clear user events state on fork in case of alloc failure

On fork, the child gets a pointer to the parent's user events state. It makes a copy of it then updates the child's pointer to it. But if the allocation fails, the duplication function leaves the child with a pointer to its parent's descriptor. When the child cleans up its data, it will free the parent's descriptor while the parent is still using it.

In the duplication function, set the child's user_event_mm to NULL before testing if the allocation succeeded, and when it exits it will not free the parent's descriptor.

- Fix retry exhaustion in simple ring buffer reader swap

simple_ring_buffer_swap_reader_page() starts with retry set to 8 and post-decrements it only after a failed link replacement. On the final attempt, a successful replacement leaves retry at zero, while a failed replacement leaves it at -1.

But the check for success expects the retry value to be non-zero and exits with an error on zero. This is the opposite result. Fix it.

- Fail nicely when the remote swap_reader_page() returns an error

Currently, if the swap_reader_page() of a remote buffer fails, it triggers a WARN_ON_ONCE() and continues normally. Instead, have it exit with an error and a pr_warn() print instead of a full WARNING.

git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
trace/fixes

Head SHA1: 5eab74874d11160725c42ab676ba97a797a362eb

Ivan Immanuel Shaji (2):
tracing: Fix retry exhaustion in simple ring buffer reader swap
ring-buffer: Stop remote reader update when page swap fails

Jérémy Jean (1):
tracing/user_events: Clear copied tracing state before fork duplication

----

 kernel/trace/ring_buffer.c        | 7 +++++--
 kernel/trace/simple_ring_buffer.c | 4 ++--
 kernel/trace/trace_events_user.c  | 3 +++
 3 files changed, 10 insertions(+), 4 deletions(-)
#1 lore raw

PATCH 1/3 tracing/user_events: Clear copied tracing state before fork duplication

From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Jean?= <[email protected]>

dup_task_struct() copies user_event_mm from the parent into the child, without grabbing a reference to it. user_event_mm_dup() should replace it, but it leaves that copied pointer unmodified if user_event_mm_alloc() fails.

When the child exits, user_event_mm_remove() decrements a reference the child never owned, which ultimately frees user_event_mm, while the parent still as a stale pointer to it. This creates a UAF, which KASAN reports as:

    BUG: KASAN: slab-use-after-free in
    current_user_event_mm+0x51/0x1d0 Write of size 4 at addr
    ffff888005010d30 by task init/44

    Call Trace:
     <TASK>
     kasan_report+0xce/0x100
     kasan_check_range+0x10f/0x1e0
     current_user_event_mm+0x51/0x1d0
     user_events_ioctl+0x82e/0x15c0
     __x64_sys_ioctl+0x139/0x1c0
     do_syscall_64+0xce/0x450
     entry_SYSCALL_64_after_hwframe+0x77/0x7f

    Allocated by task 44:
     __kasan_kmalloc+0x8f/0xa0
     __kmalloc_cache_noprof+0x180/0x3a0
     user_event_mm_alloc+0x3c/0x1f0
     current_user_event_mm+0x88/0x1d0

    Freed by task 42:
     __kasan_slab_free+0x43/0x70
     kfree+0x13a/0x390
     process_one_work+0x696/0xf90
     worker_thread+0x420/0xba0

The fix simply clears the copied pointer before any possible failure. In case of failure, the child then has nothing to free.

Fixes 7235759084a4 ("tracing/user_events: Use remote writes for event enablement")
Assisted-by Codex:gpt-5
---
 kernel/trace/trace_events_user.c | 3 +++
 1 file changed, 3 insertions(+)
kernel/trace/trace_events_user.c +3 -0
index 2bbc89d4a266..93cda2f6f269 100644
@@ -868,6 +868,9 @@ void user_event_mm_dup(struct task_struct *t, struct user_event_mm *old_mm)
 	struct user_event_mm *mm = user_event_mm_alloc(t);
 	struct user_event_enabler *enabler;
 
+	/* On failure, do not free parent's copy */
+	t->user_event_mm = NULL;
+
 	if (!mm)
 		return;
 
Signature · 1 line
2.53.0
#2 lore raw

PATCH 2/3 tracing: Fix retry exhaustion in simple ring buffer reader swap

In reply to Steven Rostedt

From: Ivan Immanuel Shaji <[email protected]>

simple_ring_buffer_swap_reader_page() starts with retry set to 8 and post-decrements it only after a failed link replacement. On the final attempt, a successful replacement leaves retry at zero, while a failed replacement leaves it at -1.

The current !retry test reverses both outcomes. It returns an error after a successful final replacement, leaving the link update complete but the reader bookkeeping unfinished. After a failed final replacement, it falls through and updates the head and reader pointers as though the replacement succeeded, which can corrupt the ring.

Treat only a negative counter as exhaustion and return the documented -EBUSY error.

Fixes 34e5b958bdad ("tracing: Introduce simple_ring_buffer")
Assisted-by LLM sparse
---
 kernel/trace/simple_ring_buffer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/trace/simple_ring_buffer.c +2 -2
index f4642f5adda3..49913bb0057a 100644
@@ -160,8 +160,8 @@ int simple_ring_buffer_swap_reader_page(struct simple_rb_per_cpu *cpu_buffer)
 		overrun = cpu_buffer->meta->overrun;
 	} while (!simple_bpage_unset_head_link(last, reader, SIMPLE_RB_LINK_NORMAL) && retry--);
 
-	if (!retry)
-		return -EINVAL;
+	if (retry < 0)
+		return -EBUSY;
 
 	cpu_buffer->head_page = simple_bpage_from_link(reader->link.next);
 	cpu_buffer->head_page->link.prev = &reader->link;
Signature · 1 line
2.53.0
#3 lore raw

PATCH 3/3 ring-buffer: Stop remote reader update when page swap fails

In reply to Steven Rostedt

From: Ivan Immanuel Shaji <[email protected]>

The remote swap_reader_page callback can return -EBUSY when the writer moves the head before the remote catches it, particularly during an event storm on a small buffer. __rb_get_reader_page_from_remote() currently warns about that failure but continues with the unchanged reader ID and rearranges the local page list as though the swap succeeded.

Handle the callback failure as a recoverable error. Report it with pr_warn_ratelimited() and return NULL. Callers already handle a NULL reader page as a failed attempt. This avoids splicing the same page as both the previous and new reader without flooding the log under contention.

Fixes 2e67fabd8b77 ("ring-buffer: Introduce ring-buffer remotes")
Assisted-by LLM sparse
---
 kernel/trace/ring_buffer.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
kernel/trace/ring_buffer.c +5 -2
index 5fc009edc1ec..b7d076b6edcf 100644
@@ -5805,8 +5805,11 @@ __rb_get_reader_page_from_remote(struct ring_buffer_per_cpu *cpu_buffer)
 
 	prev_reader = cpu_buffer->subbuf_ids[cpu_buffer->meta_page->reader.id];
 
-	WARN_ON_ONCE(cpu_buffer->remote->swap_reader_page(cpu_buffer->cpu,
-							  cpu_buffer->remote->priv));
+	if (cpu_buffer->remote->swap_reader_page(cpu_buffer->cpu,
+						 cpu_buffer->remote->priv)) {
+		pr_warn_ratelimited("Remote reader page swap failed\n");
+		return NULL;
+	}
 	/* nr_pages doesn't include the reader page */
 	if (WARN_ON_ONCE(cpu_buffer->meta_page->reader.id > cpu_buffer->nr_pages))
 		return NULL;
Signature · 1 line
2.53.0