1. Patchew
  2. linux
  3. View series

[PATH v3] tracing: Fix race condition in kprobe initialization causing NULL pointer dereference

chenyuan_fl@163.com posted 1 patch 2 months, 2 weeks ago
There is a newer version of this series
kernel/trace/trace_probe.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[PATH v3] tracing: Fix race condition in kprobe initialization causing NULL pointer dereference
Posted by chenyuan_fl@163.com 2 months, 2 weeks ago 
From: Yuan Chen <chenyuan@kylinos.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.260314] pstate: 404003c9 (nZcv DAIF +PAN -UAO)
[1135630.269239] pc : kprobe_perf_func+0x30/0x260
[1135630.277643] lr : kprobe_dispatcher+0x44/0x60
[1135630.286041] sp : ffffaeff4977fa40
[1135630.293441] x29: ffffaeff4977fa40 x28: ffffaf015340e400
[1135630.302837] x27: 0000000000000000 x26: 0000000000000000
[1135630.312257] x25: ffffaf029ed108a8 x24: ffffaf015340e528
[1135630.321705] x23: ffffaeff4977fc50 x22: ffffaeff4977fc50
[1135630.331154] x21: 0000000000000000 x20: ffffaeff4977fc50
[1135630.340586] x19: ffffaf015340e400 x18: 0000000000000000
[1135630.349985] x17: 0000000000000000 x16: 0000000000000000
[1135630.359285] x15: 0000000000000000 x14: 0000000000000000
[1135630.368445] x13: 0000000000000000 x12: 0000000000000000
[1135630.377473] x11: 0000000000000000 x10: 0000000000000000
[1135630.386411] x9 : 0000000000000000 x8 : 0000000000000000
[1135630.395252] x7 : 0000000000000000 x6 : 0000000000000000
[1135630.403963] x5 : 0000000000000000 x4 : 0000000000000000
[1135630.412545] x3 : 0000710a04630000 x2 : 0000000000000006
[1135630.421021] x1 : ffffaeff4977fc50 x0 : 0000710a04630000
[1135630.429410] Call trace:
[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
[1135630.475074]  el1_dbg+0x18/0x8c
[1135630.480582]  security_file_permission+0x0/0xd0
[1135630.487426]  vfs_write+0x70/0x1c0
[1135630.493059]  ksys_write+0x5c/0xc8
[1135630.498638]  __arm64_sys_write+0x24/0x30
[1135630.504821]  el0_svc_common+0x78/0x130
[1135630.510838]  el0_svc_handler+0x38/0x78
[1135630.516834]  el0_svc+0x8/0x1b0

kernel/trace/trace_kprobe.c: 1308
0xffff3df8995039ec <kprobe_perf_func+0x2c>:     ldr     x21, [x24,#120]
include/linux/compiler.h: 294
0xffff3df8995039f0 <kprobe_perf_func+0x30>:     ldr     x1, [x21,x0]

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

CPU1 sees that kprobe functionality is enabled but does not see that
perf_events has been assigned.

Add pairing read and write memory barriers to guarantee that if CPU1
sees that kprobe functionality is enabled, it must also see that
perf_events has been assigned.

v1->v2: Fix race analysis (per Masami) - kprobe arms in class->reg().
v2->v3: Adopt RELEASE/ACQUIRE semantics per Peter/John's suggestions,
        aligning with Steven's clarification on barrier purposes.

Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
 kernel/trace/trace_probe.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 842383fbc03b..98b838591edc 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -274,19 +274,19 @@ struct event_file_link {
 static inline bool trace_probe_test_flag(struct trace_probe *tp,
 					 unsigned int flag)
 {
-	return !!(tp->event->flags & flag);
+	return !!(smp_load_acquire(&tp->event->flags) & flag);
 }
 
 static inline void trace_probe_set_flag(struct trace_probe *tp,
 					unsigned int flag)
 {
-	tp->event->flags |= flag;
+	smp_store_release(&tp->event->flags, tp->event->flags | flag);
 }
 
 static inline void trace_probe_clear_flag(struct trace_probe *tp,
 					  unsigned int flag)
 {
-	tp->event->flags &= ~flag;
+	smp_store_release(&tp->event->flags, tp->event->flags & ~flag);
 }
 
 static inline bool trace_probe_is_enabled(struct trace_probe *tp)
-- 
2.39.5
Re: [PATH v3] tracing: Fix race condition in kprobe initialization causing NULL pointer dereference
Posted by John Ogness 2 months, 2 weeks ago 
On 2025-09-30, chenyuan_fl@163.com wrote:
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index 842383fbc03b..98b838591edc 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -274,19 +274,19 @@ struct event_file_link {
>  static inline bool trace_probe_test_flag(struct trace_probe *tp,
>  					 unsigned int flag)
>  {
> -	return !!(tp->event->flags & flag);
> +	return !!(smp_load_acquire(&tp->event->flags) & flag);
>  }
>  
>  static inline void trace_probe_set_flag(struct trace_probe *tp,
>  					unsigned int flag)
>  {
> -	tp->event->flags |= flag;
> +	smp_store_release(&tp->event->flags, tp->event->flags | flag);
>  }
>  
>  static inline void trace_probe_clear_flag(struct trace_probe *tp,
>  					  unsigned int flag)
>  {
> -	tp->event->flags &= ~flag;
> +	smp_store_release(&tp->event->flags, tp->event->flags & ~flag);
>  }
>  
>  static inline bool trace_probe_is_enabled(struct trace_probe *tp)

I don't have any feedback about the correctness with regards to tracing
and kprobes. However, I recommend writing a comment at each necessary
memory barrier site. The comment should mention the pairing memory
barrier and the ordering it is guaranteeing.

John
Re: [PATH v3] tracing: Fix race condition in kprobe initialization causing NULL pointer dereference
Posted by Peter Zijlstra 2 months, 2 weeks ago 
On Tue, Sep 30, 2025 at 09:18:48AM +0100, chenyuan_fl@163.com wrote:

> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index 842383fbc03b..98b838591edc 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -274,19 +274,19 @@ struct event_file_link {
>  static inline bool trace_probe_test_flag(struct trace_probe *tp,
>  					 unsigned int flag)
>  {
> -	return !!(tp->event->flags & flag);
> +	return !!(smp_load_acquire(&tp->event->flags) & flag);
>  }
>  
>  static inline void trace_probe_set_flag(struct trace_probe *tp,
>  					unsigned int flag)
>  {
> -	tp->event->flags |= flag;
> +	smp_store_release(&tp->event->flags, tp->event->flags | flag);
>  }
>  
>  static inline void trace_probe_clear_flag(struct trace_probe *tp,
>  					  unsigned int flag)
>  {
> -	tp->event->flags &= ~flag;
> +	smp_store_release(&tp->event->flags, tp->event->flags & ~flag);
>  }


I _think_ the clear one is superfluous. Is there anything that cares
about stores done before the clear when the flag is found not set?

Also, code like:

static int fentry_dispatcher(struct fprobe *fp, unsigned long entry_ip,
			     unsigned long ret_ip, struct ftrace_regs *fregs,
			     void *entry_data)
{
	struct trace_fprobe *tf = container_of(fp, struct trace_fprobe, fp);
	int ret = 0;

	if (trace_probe_test_flag(&tf->tp, TP_FLAG_TRACE))
		fentry_trace_func(tf, entry_ip, fregs);

#ifdef CONFIG_PERF_EVENTS
	if (trace_probe_test_flag(&tf->tp, TP_FLAG_PROFILE))
		ret = fentry_perf_func(tf, entry_ip, fregs);
#endif
	return ret;
}


Will now have two barriers; where one would suffice, eg.

	flags = smp_load_acquire(&tp->event->flags);

	if (flags & TP_FLAG_TRACE)
		fentry_trace_func(...);

	if (flags & TP_FLAG_PROFILE)
		fentry_perf_func(...);

Should be just fine afaict.


Is this something anybody cares about?
Re: [PATH v3] tracing: Fix race condition in kprobe initialization causing NULL pointer dereference
Posted by Masami Hiramatsu (Google) 2 months, 2 weeks ago 
On Tue, 30 Sep 2025 10:46:45 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> On Tue, Sep 30, 2025 at 09:18:48AM +0100, chenyuan_fl@163.com wrote:
> 
> > diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> > index 842383fbc03b..98b838591edc 100644
> > --- a/kernel/trace/trace_probe.h
> > +++ b/kernel/trace/trace_probe.h
> > @@ -274,19 +274,19 @@ struct event_file_link {
> >  static inline bool trace_probe_test_flag(struct trace_probe *tp,
> >  					 unsigned int flag)
> >  {
> > -	return !!(tp->event->flags & flag);
> > +	return !!(smp_load_acquire(&tp->event->flags) & flag);
> >  }
> >  
> >  static inline void trace_probe_set_flag(struct trace_probe *tp,
> >  					unsigned int flag)
> >  {
> > -	tp->event->flags |= flag;
> > +	smp_store_release(&tp->event->flags, tp->event->flags | flag);
> >  }
> >  
> >  static inline void trace_probe_clear_flag(struct trace_probe *tp,
> >  					  unsigned int flag)
> >  {
> > -	tp->event->flags &= ~flag;
> > +	smp_store_release(&tp->event->flags, tp->event->flags & ~flag);
> >  }
> 
> 
> I _think_ the clear one is superfluous. Is there anything that cares
> about stores done before the clear when the flag is found not set?
> 
> Also, code like:
> 
> static int fentry_dispatcher(struct fprobe *fp, unsigned long entry_ip,
> 			     unsigned long ret_ip, struct ftrace_regs *fregs,
> 			     void *entry_data)
> {
> 	struct trace_fprobe *tf = container_of(fp, struct trace_fprobe, fp);
> 	int ret = 0;
> 
> 	if (trace_probe_test_flag(&tf->tp, TP_FLAG_TRACE))
> 		fentry_trace_func(tf, entry_ip, fregs);
> 
> #ifdef CONFIG_PERF_EVENTS
> 	if (trace_probe_test_flag(&tf->tp, TP_FLAG_PROFILE))
> 		ret = fentry_perf_func(tf, entry_ip, fregs);
> #endif
> 	return ret;
> }
> 
> 
> Will now have two barriers; where one would suffice, eg.
> 
> 	flags = smp_load_acquire(&tp->event->flags);
> 
> 	if (flags & TP_FLAG_TRACE)
> 		fentry_trace_func(...);
> 
> 	if (flags & TP_FLAG_PROFILE)
> 		fentry_perf_func(...);
> 
> Should be just fine afaict.

Looks good to me. We should replace trace_probe_test_flag()
with trace_probe_load_flag().

Thanks,

> 
> 
> Is this something anybody cares about?


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>
[PATCH v4] tracing: Fix race condition in kprobe initialization causing NULL pointer dereference
Posted by chenyuan_fl@163.com 2 months, 2 weeks ago 
From: Yuan Chen <chenyuan@kylinos.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.260314] pstate: 404003c9 (nZcv DAIF +PAN -UAO)
[1135630.269239] pc : kprobe_perf_func+0x30/0x260
[1135630.277643] lr : kprobe_dispatcher+0x44/0x60
[1135630.286041] sp : ffffaeff4977fa40
[1135630.293441] x29: ffffaeff4977fa40 x28: ffffaf015340e400
[1135630.302837] x27: 0000000000000000 x26: 0000000000000000
[1135630.312257] x25: ffffaf029ed108a8 x24: ffffaf015340e528
[1135630.321705] x23: ffffaeff4977fc50 x22: ffffaeff4977fc50
[1135630.331154] x21: 0000000000000000 x20: ffffaeff4977fc50
[1135630.340586] x19: ffffaf015340e400 x18: 0000000000000000
[1135630.349985] x17: 0000000000000000 x16: 0000000000000000
[1135630.359285] x15: 0000000000000000 x14: 0000000000000000
[1135630.368445] x13: 0000000000000000 x12: 0000000000000000
[1135630.377473] x11: 0000000000000000 x10: 0000000000000000
[1135630.386411] x9 : 0000000000000000 x8 : 0000000000000000
[1135630.395252] x7 : 0000000000000000 x6 : 0000000000000000
[1135630.403963] x5 : 0000000000000000 x4 : 0000000000000000
[1135630.412545] x3 : 0000710a04630000 x2 : 0000000000000006
[1135630.421021] x1 : ffffaeff4977fc50 x0 : 0000710a04630000
[1135630.429410] Call trace:
[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
[1135630.475074]  el1_dbg+0x18/0x8c
[1135630.480582]  security_file_permission+0x0/0xd0
[1135630.487426]  vfs_write+0x70/0x1c0
[1135630.493059]  ksys_write+0x5c/0xc8
[1135630.498638]  __arm64_sys_write+0x24/0x30
[1135630.504821]  el0_svc_common+0x78/0x130
[1135630.510838]  el0_svc_handler+0x38/0x78
[1135630.516834]  el0_svc+0x8/0x1b0

kernel/trace/trace_kprobe.c: 1308
0xffff3df8995039ec <kprobe_perf_func+0x2c>:     ldr     x21, [x24,#120]
include/linux/compiler.h: 294
0xffff3df8995039f0 <kprobe_perf_func+0x30>:     ldr     x1, [x21,x0]

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

CPU1 sees that kprobe functionality is enabled but does not see that
perf_events has been assigned.

Add pairing read and write memory barriers to guarantee that if CPU1
sees that kprobe functionality is enabled, it must also see that
perf_events has been assigned.

v1->v2: Fix race analysis (per Masami) - kprobe arms in class->reg().
v2->v3: Adopt RELEASE/ACQUIRE semantics per Peter/John's suggestions,
        aligning with Steven's clarification on barrier purposes.
v3->v4: Introduce load_flag() (Masami) and optimize barrier usage in
        checks/clear (Peter).

Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
---
 kernel/trace/trace_fprobe.c | 10 ++++++----
 kernel/trace/trace_kprobe.c | 11 +++++++----
 kernel/trace/trace_probe.h  |  9 +++++++--
 kernel/trace/trace_uprobe.c | 12 ++++++++----
 4 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c
index b36ade43d4b3..ad9d6347b5fa 100644
--- a/kernel/trace/trace_fprobe.c
+++ b/kernel/trace/trace_fprobe.c
@@ -522,13 +522,14 @@ static int fentry_dispatcher(struct fprobe *fp, unsigned long entry_ip,
 			     void *entry_data)
 {
 	struct trace_fprobe *tf = container_of(fp, struct trace_fprobe, fp);
+	unsigned int flags = trace_probe_load_flag(&tf->tp);
 	int ret = 0;
 
-	if (trace_probe_test_flag(&tf->tp, TP_FLAG_TRACE))
+	if (flags & TP_FLAG_TRACE)
 		fentry_trace_func(tf, entry_ip, fregs);
 
 #ifdef CONFIG_PERF_EVENTS
-	if (trace_probe_test_flag(&tf->tp, TP_FLAG_PROFILE))
+	if (flags & TP_FLAG_PROFILE)
 		ret = fentry_perf_func(tf, entry_ip, fregs);
 #endif
 	return ret;
@@ -540,11 +541,12 @@ static void fexit_dispatcher(struct fprobe *fp, unsigned long entry_ip,
 			     void *entry_data)
 {
 	struct trace_fprobe *tf = container_of(fp, struct trace_fprobe, fp);
+	unsigned int flags = trace_probe_load_flag(&tf->tp);
 
-	if (trace_probe_test_flag(&tf->tp, TP_FLAG_TRACE))
+	if (flags & TP_FLAG_TRACE)
 		fexit_trace_func(tf, entry_ip, ret_ip, fregs, entry_data);
 #ifdef CONFIG_PERF_EVENTS
-	if (trace_probe_test_flag(&tf->tp, TP_FLAG_PROFILE))
+	if (flags & TP_FLAG_PROFILE)
 		fexit_perf_func(tf, entry_ip, ret_ip, fregs, entry_data);
 #endif
 }
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index ccae62d4fb91..b1b793b8f191 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1813,14 +1813,15 @@ static int kprobe_register(struct trace_event_call *event,
 static int kprobe_dispatcher(struct kprobe *kp, struct pt_regs *regs)
 {
 	struct trace_kprobe *tk = container_of(kp, struct trace_kprobe, rp.kp);
+	unsigned int flags = trace_probe_load_flag(&tk->tp);
 	int ret = 0;
 
 	raw_cpu_inc(*tk->nhit);
 
-	if (trace_probe_test_flag(&tk->tp, TP_FLAG_TRACE))
+	if (flags & TP_FLAG_TRACE)
 		kprobe_trace_func(tk, regs);
 #ifdef CONFIG_PERF_EVENTS
-	if (trace_probe_test_flag(&tk->tp, TP_FLAG_PROFILE))
+	if (flags & TP_FLAG_PROFILE)
 		ret = kprobe_perf_func(tk, regs);
 #endif
 	return ret;
@@ -1832,6 +1833,7 @@ kretprobe_dispatcher(struct kretprobe_instance *ri, struct pt_regs *regs)
 {
 	struct kretprobe *rp = get_kretprobe(ri);
 	struct trace_kprobe *tk;
+	unsigned int flags;
 
 	/*
 	 * There is a small chance that get_kretprobe(ri) returns NULL when
@@ -1844,10 +1846,11 @@ kretprobe_dispatcher(struct kretprobe_instance *ri, struct pt_regs *regs)
 	tk = container_of(rp, struct trace_kprobe, rp);
 	raw_cpu_inc(*tk->nhit);
 
-	if (trace_probe_test_flag(&tk->tp, TP_FLAG_TRACE))
+	flags = trace_probe_load_flag(&tk->tp);
+	if (flags & TP_FLAG_TRACE)
 		kretprobe_trace_func(tk, ri, regs);
 #ifdef CONFIG_PERF_EVENTS
-	if (trace_probe_test_flag(&tk->tp, TP_FLAG_PROFILE))
+	if (flags & TP_FLAG_PROFILE)
 		kretprobe_perf_func(tk, ri, regs);
 #endif
 	return 0;	/* We don't tweak kernel, so just return 0 */
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 842383fbc03b..08b5bda24da2 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -271,16 +271,21 @@ struct event_file_link {
 	struct list_head		list;
 };
 
+static inline unsigned int trace_probe_load_flag(struct trace_probe *tp)
+{
+	return smp_load_acquire(&tp->event->flags);
+}
+
 static inline bool trace_probe_test_flag(struct trace_probe *tp,
 					 unsigned int flag)
 {
-	return !!(tp->event->flags & flag);
+	return !!(trace_probe_load_flag(tp) & flag);
 }
 
 static inline void trace_probe_set_flag(struct trace_probe *tp,
 					unsigned int flag)
 {
-	tp->event->flags |= flag;
+	smp_store_release(&tp->event->flags, tp->event->flags | flag);
 }
 
 static inline void trace_probe_clear_flag(struct trace_probe *tp,
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 8b0bcc0d8f41..430d09c49462 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -1547,6 +1547,7 @@ static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs *regs,
 	struct trace_uprobe *tu;
 	struct uprobe_dispatch_data udd;
 	struct uprobe_cpu_buffer *ucb = NULL;
+	unsigned int flags;
 	int ret = 0;
 
 	tu = container_of(con, struct trace_uprobe, consumer);
@@ -1561,11 +1562,12 @@ static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs *regs,
 	if (WARN_ON_ONCE(!uprobe_cpu_buffer))
 		return 0;
 
-	if (trace_probe_test_flag(&tu->tp, TP_FLAG_TRACE))
+	flags = trace_probe_load_flag(&tu->tp);
+	if (flags & TP_FLAG_TRACE)
 		ret |= uprobe_trace_func(tu, regs, &ucb);
 
 #ifdef CONFIG_PERF_EVENTS
-	if (trace_probe_test_flag(&tu->tp, TP_FLAG_PROFILE))
+	if (flags & TP_FLAG_PROFILE)
 		ret |= uprobe_perf_func(tu, regs, &ucb);
 #endif
 	uprobe_buffer_put(ucb);
@@ -1579,6 +1581,7 @@ static int uretprobe_dispatcher(struct uprobe_consumer *con,
 	struct trace_uprobe *tu;
 	struct uprobe_dispatch_data udd;
 	struct uprobe_cpu_buffer *ucb = NULL;
+	unsigned int flags;
 
 	tu = container_of(con, struct trace_uprobe, consumer);
 
@@ -1590,11 +1593,12 @@ static int uretprobe_dispatcher(struct uprobe_consumer *con,
 	if (WARN_ON_ONCE(!uprobe_cpu_buffer))
 		return 0;
 
-	if (trace_probe_test_flag(&tu->tp, TP_FLAG_TRACE))
+	flags = trace_probe_load_flag(&tu->tp);
+	if (flags & TP_FLAG_TRACE)
 		uretprobe_trace_func(tu, func, regs, &ucb);
 
 #ifdef CONFIG_PERF_EVENTS
-	if (trace_probe_test_flag(&tu->tp, TP_FLAG_PROFILE))
+	if (flags & TP_FLAG_PROFILE)
 		uretprobe_perf_func(tu, func, regs, &ucb);
 #endif
 	uprobe_buffer_put(ucb);
-- 
2.39.5
Re: [PATCH v4] tracing: Fix race condition in kprobe initialization causing NULL pointer dereference
Posted by Peter Zijlstra 2 months, 2 weeks ago 
On Wed, Oct 01, 2025 at 03:20:25AM +0100, chenyuan_fl@163.com wrote:

> 
> v1->v2: Fix race analysis (per Masami) - kprobe arms in class->reg().
> v2->v3: Adopt RELEASE/ACQUIRE semantics per Peter/John's suggestions,
>         aligning with Steven's clarification on barrier purposes.
> v3->v4: Introduce load_flag() (Masami) and optimize barrier usage in
>         checks/clear (Peter).

Stuff like this ought to go below the --- such that git-am and similar
tools throw it out.

> Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
> ---

> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index ccae62d4fb91..b1b793b8f191 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -1813,14 +1813,15 @@ static int kprobe_register(struct trace_event_call *event,
>  static int kprobe_dispatcher(struct kprobe *kp, struct pt_regs *regs)
>  {
>  	struct trace_kprobe *tk = container_of(kp, struct trace_kprobe, rp.kp);
> +	unsigned int flags = trace_probe_load_flag(&tk->tp);
>  	int ret = 0;
>  
>  	raw_cpu_inc(*tk->nhit);
>  
> -	if (trace_probe_test_flag(&tk->tp, TP_FLAG_TRACE))
> +	if (flags & TP_FLAG_TRACE)
>  		kprobe_trace_func(tk, regs);
>  #ifdef CONFIG_PERF_EVENTS
> -	if (trace_probe_test_flag(&tk->tp, TP_FLAG_PROFILE))
> +	if (flags & TP_FLAG_PROFILE)
>  		ret = kprobe_perf_func(tk, regs);
>  #endif
>  	return ret;
> @@ -1832,6 +1833,7 @@ kretprobe_dispatcher(struct kretprobe_instance *ri, struct pt_regs *regs)
>  {
>  	struct kretprobe *rp = get_kretprobe(ri);
>  	struct trace_kprobe *tk;
> +	unsigned int flags;
>  
>  	/*
>  	 * There is a small chance that get_kretprobe(ri) returns NULL when
> @@ -1844,10 +1846,11 @@ kretprobe_dispatcher(struct kretprobe_instance *ri, struct pt_regs *regs)
>  	tk = container_of(rp, struct trace_kprobe, rp);
>  	raw_cpu_inc(*tk->nhit);
>  
> -	if (trace_probe_test_flag(&tk->tp, TP_FLAG_TRACE))
> +	flags = trace_probe_load_flag(&tk->tp);
> +	if (flags & TP_FLAG_TRACE)
>  		kretprobe_trace_func(tk, ri, regs);
>  #ifdef CONFIG_PERF_EVENTS
> -	if (trace_probe_test_flag(&tk->tp, TP_FLAG_PROFILE))
> +	if (flags & TP_FLAG_PROFILE)
>  		kretprobe_perf_func(tk, ri, regs);
>  #endif
>  	return 0;	/* We don't tweak kernel, so just return 0 */


These two are inconsistent in that the first does load_flag before
inc(nhit) while the second does it after. I don't think it matters, but
I noticed the inconsistent style and had to tell.


Anyway, patch looks reasonable now. Rest is up to Steve, this is his
code.
Re: [PATCH v4] tracing: Fix race condition in kprobe initialization causing NULL pointer dereference
Posted by Masami Hiramatsu (Google) 2 months, 2 weeks ago 
On Wed, 1 Oct 2025 14:32:00 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> On Wed, Oct 01, 2025 at 03:20:25AM +0100, chenyuan_fl@163.com wrote:
> 
> > 
> > v1->v2: Fix race analysis (per Masami) - kprobe arms in class->reg().
> > v2->v3: Adopt RELEASE/ACQUIRE semantics per Peter/John's suggestions,
> >         aligning with Steven's clarification on barrier purposes.
> > v3->v4: Introduce load_flag() (Masami) and optimize barrier usage in
> >         checks/clear (Peter).
> 
> Stuff like this ought to go below the --- such that git-am and similar
> tools throw it out.

Yeah, let me remove it manually this time.

Yuan, please put changelogs like below next time:

---
  v1->v2: Fix race analysis (per Masami) - kprobe arms in class->reg().
  ...



Thank you,

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>
Re: [PATCH v4] tracing: Fix race condition in kprobe initialization causing NULL pointer dereference
Posted by Steven Rostedt 2 months, 2 weeks ago 
On Wed, 1 Oct 2025 14:32:00 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> Anyway, patch looks reasonable now. Rest is up to Steve, this is his
> code.

Actually, it's Masami's ;-)  And it's up to him.

-- Steve
Re: [PATCH v4] tracing: Fix race condition in kprobe initialization causing NULL pointer dereference
Posted by Masami Hiramatsu (Google) 2 months, 2 weeks ago 
On Wed, 1 Oct 2025 10:31:49 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Wed, 1 Oct 2025 14:32:00 +0200
> Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > Anyway, patch looks reasonable now. Rest is up to Steve, this is his
> > code.
> 
> Actually, it's Masami's ;-)  And it's up to him.

Yeah, looks good to me. Let me pick it as a fix.

Thank you!

> 
> -- Steve


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>
Re: [PATCH v4] tracing: Fix race condition in kprobe initialization causing NULL pointer dereference
Posted by Masami Hiramatsu (Google) 2 months, 2 weeks ago 
On Thu, 2 Oct 2025 07:59:24 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> On Wed, 1 Oct 2025 10:31:49 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > On Wed, 1 Oct 2025 14:32:00 +0200
> > Peter Zijlstra <peterz@infradead.org> wrote:
> > 
> > > Anyway, patch looks reasonable now. Rest is up to Steve, this is his
> > > code.
> > 
> > Actually, it's Masami's ;-)  And it's up to him.
> 
> Yeah, looks good to me. Let me pick it as a fix.

BTW, I will add below, the commit introduced the flags.

    Fixes: 50d780560785 ("tracing/kprobes: Add probe handler dispatcher to support perf and ftrace concurrent use")
    Cc: stable@vger.kernel.org

Thanks,

> 
> Thank you!
> 
> > 
> > -- Steve
> 
> 
> -- 
> Masami Hiramatsu (Google) <mhiramat@kernel.org>


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.