From: Prateek Sood <prsood@codeaurora.org>
To: rostedt@goodmis.org, mingo@redhat.com
Cc: linux-kernel@vger.kernel.org, sramana@codeaurora.org,
fweisbec@gmail.com, jolsa@redhat.com,
Prateek Sood <prsood@codeaurora.org>
Subject: [PATCH] perf: fix use after free of perf_trace_buf
Date: Tue, 19 Mar 2019 17:51:00 +0530 [thread overview]
Message-ID: <1552998060-5735-1-git-send-email-prsood@codeaurora.org> (raw)
In-Reply-To: <20190318151529.GT6058@hirez.programming.kicks-ass.net>
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
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 <prsood@codeaurora.org>
---
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.
next prev parent reply other threads:[~2019-03-19 12:21 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-18 10:47 [PATCH] perf: extend total_ref_count usage to protect perf_trace_buf access Prateek Sood
2019-03-18 15:02 ` Steven Rostedt
2019-03-18 15:15 ` Peter Zijlstra
2019-03-19 12:21 ` Prateek Sood [this message]
2019-03-19 13:31 ` [PATCH] perf: fix use after free of perf_trace_buf Steven Rostedt
2019-03-19 13:37 ` Jiri Olsa
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1552998060-5735-1-git-send-email-prsood@codeaurora.org \
--to=prsood@codeaurora.org \
--cc=fweisbec@gmail.com \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=rostedt@goodmis.org \
--cc=sramana@codeaurora.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.