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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250929091223.GG4067720@noisy.programming.kicks-ass.net>
Date: Mon, 29 Sep 2025 11:12:23 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: chenyuan_fl@....com, mhiramat@...nel.org,
	mathieu.desnoyers@...icios.com, linux-kernel@...r.kernel.org,
	linux-trace-kernel@...r.kernel.org, Yuan Chen <chenyuan@...inos.cn>,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
	John Ogness <john.ogness@...utronix.de>
Subject: Re: [PATCH v2] tracing: Fix race condition in kprobe initialization
 causing NULL pointer dereference

On Mon, Sep 29, 2025 at 04:48:36AM -0400, Steven Rostedt wrote:
> On Mon, 29 Sep 2025 07:57:31 +0100
> chenyuan_fl@....com wrote:
> 
> > From: Yuan Chen <chenyuan@...inos.cn>
> > 
> > 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

> > [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

> > 
> > 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() ?

> > 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.

> > Signed-off-by: Yuan Chen <chenyuan@...inos.cn>
> > ---
> >  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.