Re: [PATH v3] tracing: Fix race condition in kprobe initialization causing NULL pointer dereference
From: Peter Zijlstra <peterz@infradead.org>
Date: 2025-09-30 08:46:54
Also in:
lkml
On Tue, Sep 30, 2025 at 09:18:48AM +0100, chenyuan_fl@163.com wrote:
quoted hunk ↗ jump to hunk
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?