Skip to content

Commit 68f364b

Browse files
chenyuan0001intel-lab-lkp
authored andcommitted
tracing: Fix race condition in kprobe initialization causing NULL pointer dereference
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,torvalds#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 <[email protected]>
1 parent 90ddf2d commit 68f364b

File tree

4 files changed

+28
-14
lines changed

4 files changed

+28
-14
lines changed

kernel/trace/trace_fprobe.c

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -522,13 +522,14 @@ static int fentry_dispatcher(struct fprobe *fp, unsigned long entry_ip,
522522
void *entry_data)
523523
{
524524
struct trace_fprobe *tf = container_of(fp, struct trace_fprobe, fp);
525+
unsigned int flags = trace_probe_load_flag(&tf->tp);
525526
int ret = 0;
526527

527-
if (trace_probe_test_flag(&tf->tp, TP_FLAG_TRACE))
528+
if (flags & TP_FLAG_TRACE)
528529
fentry_trace_func(tf, entry_ip, fregs);
529530

530531
#ifdef CONFIG_PERF_EVENTS
531-
if (trace_probe_test_flag(&tf->tp, TP_FLAG_PROFILE))
532+
if (flags & TP_FLAG_PROFILE)
532533
ret = fentry_perf_func(tf, entry_ip, fregs);
533534
#endif
534535
return ret;
@@ -540,11 +541,12 @@ static void fexit_dispatcher(struct fprobe *fp, unsigned long entry_ip,
540541
void *entry_data)
541542
{
542543
struct trace_fprobe *tf = container_of(fp, struct trace_fprobe, fp);
544+
unsigned int flags = trace_probe_load_flag(&tf->tp);
543545

544-
if (trace_probe_test_flag(&tf->tp, TP_FLAG_TRACE))
546+
if (flags & TP_FLAG_TRACE)
545547
fexit_trace_func(tf, entry_ip, ret_ip, fregs, entry_data);
546548
#ifdef CONFIG_PERF_EVENTS
547-
if (trace_probe_test_flag(&tf->tp, TP_FLAG_PROFILE))
549+
if (flags & TP_FLAG_PROFILE)
548550
fexit_perf_func(tf, entry_ip, ret_ip, fregs, entry_data);
549551
#endif
550552
}

kernel/trace/trace_kprobe.c

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1815,14 +1815,15 @@ static int kprobe_register(struct trace_event_call *event,
18151815
static int kprobe_dispatcher(struct kprobe *kp, struct pt_regs *regs)
18161816
{
18171817
struct trace_kprobe *tk = container_of(kp, struct trace_kprobe, rp.kp);
1818+
unsigned int flags = trace_probe_load_flag(&tk->tp);
18181819
int ret = 0;
18191820

18201821
raw_cpu_inc(*tk->nhit);
18211822

1822-
if (trace_probe_test_flag(&tk->tp, TP_FLAG_TRACE))
1823+
if (flags & TP_FLAG_TRACE)
18231824
kprobe_trace_func(tk, regs);
18241825
#ifdef CONFIG_PERF_EVENTS
1825-
if (trace_probe_test_flag(&tk->tp, TP_FLAG_PROFILE))
1826+
if (flags & TP_FLAG_PROFILE)
18261827
ret = kprobe_perf_func(tk, regs);
18271828
#endif
18281829
return ret;
@@ -1834,6 +1835,7 @@ kretprobe_dispatcher(struct kretprobe_instance *ri, struct pt_regs *regs)
18341835
{
18351836
struct kretprobe *rp = get_kretprobe(ri);
18361837
struct trace_kprobe *tk;
1838+
unsigned int flags;
18371839

18381840
/*
18391841
* There is a small chance that get_kretprobe(ri) returns NULL when
@@ -1846,10 +1848,11 @@ kretprobe_dispatcher(struct kretprobe_instance *ri, struct pt_regs *regs)
18461848
tk = container_of(rp, struct trace_kprobe, rp);
18471849
raw_cpu_inc(*tk->nhit);
18481850

1849-
if (trace_probe_test_flag(&tk->tp, TP_FLAG_TRACE))
1851+
flags = trace_probe_load_flag(&tk->tp);
1852+
if (flags & TP_FLAG_TRACE)
18501853
kretprobe_trace_func(tk, ri, regs);
18511854
#ifdef CONFIG_PERF_EVENTS
1852-
if (trace_probe_test_flag(&tk->tp, TP_FLAG_PROFILE))
1855+
if (flags & TP_FLAG_PROFILE)
18531856
kretprobe_perf_func(tk, ri, regs);
18541857
#endif
18551858
return 0; /* We don't tweak kernel, so just return 0 */

kernel/trace/trace_probe.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -271,16 +271,21 @@ struct event_file_link {
271271
struct list_head list;
272272
};
273273

274+
static inline unsigned int trace_probe_load_flag(struct trace_probe *tp)
275+
{
276+
return smp_load_acquire(&tp->event->flags);
277+
}
278+
274279
static inline bool trace_probe_test_flag(struct trace_probe *tp,
275280
unsigned int flag)
276281
{
277-
return !!(tp->event->flags & flag);
282+
return !!(trace_probe_load_flag(tp) & flag);
278283
}
279284

280285
static inline void trace_probe_set_flag(struct trace_probe *tp,
281286
unsigned int flag)
282287
{
283-
tp->event->flags |= flag;
288+
smp_store_release(&tp->event->flags, tp->event->flags | flag);
284289
}
285290

286291
static inline void trace_probe_clear_flag(struct trace_probe *tp,

kernel/trace/trace_uprobe.c

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1529,6 +1529,7 @@ static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs *regs,
15291529
struct trace_uprobe *tu;
15301530
struct uprobe_dispatch_data udd;
15311531
struct uprobe_cpu_buffer *ucb = NULL;
1532+
unsigned int flags;
15321533
int ret = 0;
15331534

15341535
tu = container_of(con, struct trace_uprobe, consumer);
@@ -1543,11 +1544,12 @@ static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs *regs,
15431544
if (WARN_ON_ONCE(!uprobe_cpu_buffer))
15441545
return 0;
15451546

1546-
if (trace_probe_test_flag(&tu->tp, TP_FLAG_TRACE))
1547+
flags = trace_probe_load_flag(&tu->tp);
1548+
if (flags & TP_FLAG_TRACE)
15471549
ret |= uprobe_trace_func(tu, regs, &ucb);
15481550

15491551
#ifdef CONFIG_PERF_EVENTS
1550-
if (trace_probe_test_flag(&tu->tp, TP_FLAG_PROFILE))
1552+
if (flags & TP_FLAG_PROFILE)
15511553
ret |= uprobe_perf_func(tu, regs, &ucb);
15521554
#endif
15531555
uprobe_buffer_put(ucb);
@@ -1561,6 +1563,7 @@ static int uretprobe_dispatcher(struct uprobe_consumer *con,
15611563
struct trace_uprobe *tu;
15621564
struct uprobe_dispatch_data udd;
15631565
struct uprobe_cpu_buffer *ucb = NULL;
1566+
unsigned int flags;
15641567

15651568
tu = container_of(con, struct trace_uprobe, consumer);
15661569

@@ -1572,11 +1575,12 @@ static int uretprobe_dispatcher(struct uprobe_consumer *con,
15721575
if (WARN_ON_ONCE(!uprobe_cpu_buffer))
15731576
return 0;
15741577

1575-
if (trace_probe_test_flag(&tu->tp, TP_FLAG_TRACE))
1578+
flags = trace_probe_load_flag(&tu->tp);
1579+
if (flags & TP_FLAG_TRACE)
15761580
uretprobe_trace_func(tu, func, regs, &ucb);
15771581

15781582
#ifdef CONFIG_PERF_EVENTS
1579-
if (trace_probe_test_flag(&tu->tp, TP_FLAG_PROFILE))
1583+
if (flags & TP_FLAG_PROFILE)
15801584
uretprobe_perf_func(tu, func, regs, &ucb);
15811585
#endif
15821586
uprobe_buffer_put(ucb);

0 commit comments

Comments
 (0)