On Tue, Mar 19, 2019 at 05:51:00PM +0530, Prateek Sood wrote: > SyS_perf_event_open() > free_event() > _free_event() > tp_perf_event_destroy() > perf_trace_destroy() > perf_trace_event_unreg() //free perf_trace_buf > > trace_cpu_frequency() > perf_trace_cpu() > perf_trace_buf_alloc() //access perf_trace_buf > > CPU0 CPU1 > perf_trace_event_unreg() perf_trace_cpu() > head = (event_call->perf_events) > > free_percpu(tp_event->perf_events) > tp_event->perf_events = NULL > --total_ref_count > free_percpu(perf_trace_buf[i]) > perf_trace_buf[i] = NULL > > raw_data = perf_trace_buf[rctx] > memset(raw_data) > > A potential race exists between access of perf_trace_buf from
so there's no actual backtrace of the crash, right? jirka > perf_trace_buf_alloc() and perf_trace_event_unreg(). This can > result in perf_trace_buf[rctx] being NULL during access from memset() > in perf_trace_buf_alloc(). > > Change-Id: I95ae774b9fcc653aa808f2d9f3e4359b3605e909 > Signed-off-by: Prateek Sood <[email protected]> > --- > include/linux/trace_events.h | 2 ++ > include/trace/perf.h | 5 +++- > kernel/trace/trace_event_perf.c | 63 > ++++++++++++++++++++++++++++++++++------- > kernel/trace/trace_kprobe.c | 10 +++++-- > kernel/trace/trace_syscalls.c | 14 ++++++--- > kernel/trace/trace_uprobe.c | 2 ++ > 6 files changed, 79 insertions(+), 17 deletions(-) > > diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h > index 8a62731..dbdad19 100644 > --- a/include/linux/trace_events.h > +++ b/include/linux/trace_events.h > @@ -591,6 +591,8 @@ extern int ftrace_profile_set_filter(struct perf_event > *event, int event_id, > extern void ftrace_profile_free_filter(struct perf_event *event); > void perf_trace_buf_update(void *record, u16 type); > void *perf_trace_buf_alloc(int size, struct pt_regs **regs, int *rctxp); > +void get_perf_trace_buf(void); > +void put_perf_trace_buf(void); > > void bpf_trace_run1(struct bpf_prog *prog, u64 arg1); > void bpf_trace_run2(struct bpf_prog *prog, u64 arg1, u64 arg2); > diff --git a/include/trace/perf.h b/include/trace/perf.h > index dbc6c74..f808c33 100644 > --- a/include/trace/perf.h > +++ b/include/trace/perf.h > @@ -55,9 +55,10 @@ > sizeof(u64)); \ > __entry_size -= sizeof(u32); \ > \ > + get_perf_trace_buf(); \ > entry = perf_trace_buf_alloc(__entry_size, &__regs, &rctx); \ > if (!entry) \ > - return; \ > + goto out; \ > \ > perf_fetch_caller_regs(__regs); \ > \ > @@ -68,6 +69,8 @@ > perf_trace_run_bpf_submit(entry, __entry_size, rctx, \ > event_call, __count, __regs, \ > head, __task); \ > +out: \ > + put_perf_trace_buf(); \ > } > > /* > diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c > index 4629a61..6caca88 100644 > --- a/kernel/trace/trace_event_perf.c > +++ b/kernel/trace/trace_event_perf.c > @@ -21,7 +21,8 @@ typedef typeof(unsigned long [PERF_MAX_TRACE_SIZE / > sizeof(unsigned long)]) > perf_trace_t; > > /* Count the events in use (per event id, not per instance) */ > -static int total_ref_count; > +static int alloc_ref_count; > +static atomic_t access_ref_count[PERF_NR_CONTEXTS]; > > static int perf_trace_event_perm(struct trace_event_call *tp_event, > struct perf_event *p_event) > @@ -88,6 +89,34 @@ static int perf_trace_event_perm(struct trace_event_call > *tp_event, > return 0; > } > > +void get_perf_trace_buf(void) > +{ > + int rctx; > + > + rctx = perf_swevent_get_recursion_context(); > + if (rctx < 0) > + return; > + > + atomic_inc(&access_ref_count[rctx]); > + perf_swevent_put_recursion_context(rctx); > +} > +EXPORT_SYMBOL_GPL(get_perf_trace_buf); > +NOKPROBE_SYMBOL(get_perf_trace_buf); > + > +void put_perf_trace_buf(void) > +{ > + int rctx; > + > + rctx = perf_swevent_get_recursion_context(); > + if (rctx < 0) > + return; > + > + atomic_dec(&access_ref_count[rctx]); > + perf_swevent_put_recursion_context(rctx); > +} > +EXPORT_SYMBOL_GPL(put_perf_trace_buf); > +NOKPROBE_SYMBOL(put_perf_trace_buf); > + > static int perf_trace_event_reg(struct trace_event_call *tp_event, > struct perf_event *p_event) > { > @@ -108,7 +137,7 @@ static int perf_trace_event_reg(struct trace_event_call > *tp_event, > > tp_event->perf_events = list; > > - if (!total_ref_count) { > + if (!alloc_ref_count) { > char __percpu *buf; > int i; > > @@ -125,11 +154,11 @@ static int perf_trace_event_reg(struct trace_event_call > *tp_event, > if (ret) > goto fail; > > - total_ref_count++; > + alloc_ref_count++; > return 0; > > fail: > - if (!total_ref_count) { > + if (!alloc_ref_count) { > int i; > > for (i = 0; i < PERF_NR_CONTEXTS; i++) { > @@ -150,6 +179,7 @@ static void perf_trace_event_unreg(struct perf_event > *p_event) > { > struct trace_event_call *tp_event = p_event->tp_event; > int i; > + bool retry; > > if (--tp_event->perf_refcount > 0) > goto out; > @@ -165,10 +195,21 @@ static void perf_trace_event_unreg(struct perf_event > *p_event) > free_percpu(tp_event->perf_events); > tp_event->perf_events = NULL; > > - if (!--total_ref_count) { > - for (i = 0; i < PERF_NR_CONTEXTS; i++) { > - free_percpu(perf_trace_buf[i]); > - perf_trace_buf[i] = NULL; > + if (!--alloc_ref_count) { > +again: > + retry = false; > + for (i = 0; (i < PERF_NR_CONTEXTS) && perf_trace_buf[i]; i++) { > + if (!atomic_read(&access_ref_count[i])) { > + free_percpu(perf_trace_buf[i]); > + perf_trace_buf[i] = NULL; > + } else > + retry = true; > + } > + > + if (retry) { > + set_current_state(TASK_INTERRUPTIBLE); > + schedule_timeout(msecs_to_jiffies(1)); > + goto again; > } > } > out: > @@ -453,15 +494,17 @@ void perf_trace_buf_update(void *record, u16 type) > memset(®s, 0, sizeof(regs)); > perf_fetch_caller_regs(®s); > > + get_perf_trace_buf(); > entry = perf_trace_buf_alloc(ENTRY_SIZE, NULL, &rctx); > if (!entry) > - return; > + goto out; > > entry->ip = ip; > entry->parent_ip = parent_ip; > perf_trace_buf_submit(entry, ENTRY_SIZE, rctx, TRACE_FN, > 1, ®s, &head, NULL); > - > +out: > + put_perf_trace_buf(); > #undef ENTRY_SIZE > } > > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c > index 5d5129b..7830190 100644 > --- a/kernel/trace/trace_kprobe.c > +++ b/kernel/trace/trace_kprobe.c > @@ -1166,15 +1166,18 @@ static int kretprobe_event_define_fields(struct > trace_event_call *event_call) > size = ALIGN(__size + sizeof(u32), sizeof(u64)); > size -= sizeof(u32); > > + get_perf_trace_buf(); > entry = perf_trace_buf_alloc(size, NULL, &rctx); > if (!entry) > - return 0; > + goto out; > > entry->ip = (unsigned long)tk->rp.kp.addr; > memset(&entry[1], 0, dsize); > store_trace_args(&entry[1], &tk->tp, regs, sizeof(*entry), dsize); > perf_trace_buf_submit(entry, size, rctx, call->event.type, 1, regs, > head, NULL); > +out: > + put_perf_trace_buf(); > return 0; > } > NOKPROBE_SYMBOL(kprobe_perf_func); > @@ -1202,15 +1205,18 @@ static int kretprobe_event_define_fields(struct > trace_event_call *event_call) > size = ALIGN(__size + sizeof(u32), sizeof(u64)); > size -= sizeof(u32); > > + get_perf_trace_buf(); > entry = perf_trace_buf_alloc(size, NULL, &rctx); > if (!entry) > - return; > + goto out; > > entry->func = (unsigned long)tk->rp.kp.addr; > entry->ret_ip = (unsigned long)ri->ret_addr; > store_trace_args(&entry[1], &tk->tp, regs, sizeof(*entry), dsize); > perf_trace_buf_submit(entry, size, rctx, call->event.type, 1, regs, > head, NULL); > +out: > + put_perf_trace_buf(); > } > NOKPROBE_SYMBOL(kretprobe_perf_func); > > diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c > index f93a56d..a08110f 100644 > --- a/kernel/trace/trace_syscalls.c > +++ b/kernel/trace/trace_syscalls.c > @@ -608,9 +608,10 @@ static void perf_syscall_enter(void *ignore, struct > pt_regs *regs, long id) > size = ALIGN(size + sizeof(u32), sizeof(u64)); > size -= sizeof(u32); > > + get_perf_trace_buf(); > rec = perf_trace_buf_alloc(size, NULL, &rctx); > if (!rec) > - return; > + goto out; > > rec->nr = syscall_nr; > syscall_get_arguments(current, regs, 0, sys_data->nb_args, > @@ -620,12 +621,14 @@ static void perf_syscall_enter(void *ignore, struct > pt_regs *regs, long id) > !perf_call_bpf_enter(sys_data->enter_event, regs, sys_data, rec)) > || > hlist_empty(head)) { > perf_swevent_put_recursion_context(rctx); > - return; > + goto out; > } > > perf_trace_buf_submit(rec, size, rctx, > sys_data->enter_event->event.type, 1, regs, > head, NULL); > +out: > + put_perf_trace_buf(); > } > > static int perf_sysenter_enable(struct trace_event_call *call) > @@ -706,9 +709,10 @@ static void perf_syscall_exit(void *ignore, struct > pt_regs *regs, long ret) > size = ALIGN(sizeof(*rec) + sizeof(u32), sizeof(u64)); > size -= sizeof(u32); > > + get_perf_trace_buf(); > rec = perf_trace_buf_alloc(size, NULL, &rctx); > if (!rec) > - return; > + goto out; > > rec->nr = syscall_nr; > rec->ret = syscall_get_return_value(current, regs); > @@ -717,11 +721,13 @@ static void perf_syscall_exit(void *ignore, struct > pt_regs *regs, long ret) > !perf_call_bpf_exit(sys_data->exit_event, regs, rec)) || > hlist_empty(head)) { > perf_swevent_put_recursion_context(rctx); > - return; > + goto out; > } > > perf_trace_buf_submit(rec, size, rctx, sys_data->exit_event->event.type, > 1, regs, head, NULL); > +out: > + put_perf_trace_buf(); > } > > static int perf_sysexit_enable(struct trace_event_call *call) > diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c > index be78d99..c931b22 100644 > --- a/kernel/trace/trace_uprobe.c > +++ b/kernel/trace/trace_uprobe.c > @@ -1116,6 +1116,7 @@ static void __uprobe_perf_func(struct trace_uprobe *tu, > if (hlist_empty(head)) > goto out; > > + get_perf_trace_buf(); > entry = perf_trace_buf_alloc(size, NULL, &rctx); > if (!entry) > goto out; > @@ -1140,6 +1141,7 @@ static void __uprobe_perf_func(struct trace_uprobe *tu, > perf_trace_buf_submit(entry, size, rctx, call->event.type, 1, regs, > head, NULL); > out: > + put_perf_trace_buf(); > preempt_enable(); > } > > -- > Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, > Inc., > is a member of Code Aurora Forum, a Linux Foundation Collaborative Project. >
