tracing: More fixes for 7.3
Subsystems (3)
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(-)
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/0xba0The fix simply clears the copied pointer before any possible failure. In case of failure, the child then has nothing to free.
--- kernel/trace/trace_events_user.c | 3 +++ 1 file changed, 3 insertions(+)
kernel/trace/trace_events_user.c +3 -0
@@ -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
PATCH 2/3 tracing: Fix retry exhaustion in simple ring buffer reader swap
In reply to Steven RostedtFrom: 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.
--- kernel/trace/simple_ring_buffer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/trace/simple_ring_buffer.c +2 -2
@@ -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
PATCH 3/3 ring-buffer: Stop remote reader update when page swap fails
In reply to Steven RostedtFrom: 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.
--- kernel/trace/ring_buffer.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
kernel/trace/ring_buffer.c +5 -2
@@ -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