Thread (20 messages) 20 messages, 5 authors, 2025-10-02

Re: [PATCH v2] tracing: Fix race condition in kprobe initialization causing NULL pointer dereference

From: Peter Zijlstra <peterz@infradead.org>
Date: 2025-09-29 09:12:35
Also in: lkml

On Mon, Sep 29, 2025 at 04:48:36AM -0400, Steven Rostedt wrote:
On Mon, 29 Sep 2025 07:57:31 +0100
chenyuan_fl@163.com wrote:
quoted
From: Yuan Chen <redacted>

There is a critical race condition in kprobe initialization that can lead to
NULL pointer dereference and kernel crash.

[1135630.084782] Unable to handle kernel paging request at virtual address 0000710a04630000
quoted
[1135630.434828]  kprobe_perf_func+0x30/0x260
[1135630.441661]  kprobe_dispatcher+0x44/0x60
[1135630.448396]  aggr_pre_handler+0x70/0xc8
[1135630.454959]  kprobe_breakpoint_handler+0x140/0x1e0
[1135630.462435]  brk_handler+0xbc/0xd8
[1135630.468437]  do_debug_exception+0x84/0x138
quoted
kernel/trace/trace_kprobe.c
1308: head = this_cpu_ptr(call->perf_events);
1309:   if (hlist_empty(head))
1310:           return 0;

crash> struct trace_event_call -o  
struct trace_event_call {
  ...
  [120] struct hlist_head *perf_events;  //(call->perf_event)
  ...
}

crash> struct trace_event_call ffffaf015340e528  
struct trace_event_call {
  ...
  perf_events = 0xffff0ad5fa89f088, //this value is correct, but x21 = 0
  ...
}

Race Condition Analysis:

The race occurs between kprobe activation and perf_events initialization:

  CPU0                                    CPU1
  ====                                    ====
  perf_kprobe_init
    perf_trace_event_init
      tp_event->perf_events = list;(1)
      tp_event->class->reg (2)← KPROBE ACTIVE
                                          Debug exception triggers
                                          ...
                                          kprobe_dispatcher
                                            kprobe_perf_func (tk->tp.flags & TP_FLAG_PROFILE)
                                              head = this_cpu_ptr(call->perf_events)(3)
                                              (perf_events is still NULL)

Problem:
1. CPU0 executes (1) assigning tp_event->perf_events = list
2. CPU0 executes (2) enabling kprobe functionality via class->reg()
This is kprobe_register() doing enable_trace_kprobe() ?
quoted
3. CPU1 triggers and reaches kprobe_dispatcher
4. CPU1 checks TP_FLAG_PROFILE - condition passes (step 2 completed)
5. CPU1 calls kprobe_perf_func() and crashes at (3) because
   call->perf_events is still NULL

The issue: Assignment in step 1 may not be visible to CPU1 due to
missing memory barriers before step 2 sets TP_FLAG_PROFILE flag.

Add smp_mb() barrier between perf_events assignment and enabling
profile functionality to ensure visibility ordering across CPUs.
Yeah, that cannot be right.
quoted
Signed-off-by: Yuan Chen <redacted>
---
 kernel/trace/trace_event_perf.c | 5 +++++
 1 file changed, 5 insertions(+)
diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
index a6bb7577e8c5..6eff8c9d6bae 100644
--- a/kernel/trace/trace_event_perf.c
+++ b/kernel/trace/trace_event_perf.c
@@ -113,6 +113,11 @@ static int perf_trace_event_reg(struct trace_event_call *tp_event,
 
 	tp_event->perf_events = list;
 
+	/* Ensure perf_events assignment is visible to all CPUs before enabling
+	 * profile functionality
+	 */
+	smp_mb();
So from other discussions I had with John and Sebastian (both Cc'd),
memory barriers are not for "making memory visible", but instead are
for interactions between memory and two different CPUs, where both CPUs
have memory barriers.
Correct, barriers have to be paired. The above doesn't have enough clues
for me to know what code is affected, but if we're talking about
kprobe_register(PERF_REG) := enable_trace_kprobe(,NULL), when it might
be that trace_probe_set_flag() should be an smp_store_release(), while
trace_probe_test_flag(PROFILE) in kprobe_dispatch() needs to be a
smp_load_acquire().

Without the acquire it might still be possible for the CPU to lift the
call->perf_event load up before the event->flags load, rendering your
wmb pointless.

The guarantee you're looking for is that if the flag is set, it sees a
fully formed event. This is done with RELEASE on publish and ACQUIRE on
access.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help