From: Alexei Starovoitov <ast@plumgrid.com>
To: "David S. Miller" <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>,
Peter Zijlstra <a.p.zijlstra@chello.nl>,
Wang Nan <wangnan0@huawei.com>, He Kuang <hekuang@huawei.com>,
Kaixu Xia <xiakaixu@huawei.com>,
Daniel Borkmann <daniel@iogearbox.net>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v2 net-next] bpf: fix bpf_perf_event_read() helper
Date: Wed, 21 Oct 2015 15:58:03 -0700 [thread overview]
Message-ID: <1445468283-4592-1-git-send-email-ast@kernel.org> (raw)
Fix safety checks for bpf_perf_event_read():
- only non-inherited events can be added to perf_event_array map
(do this check statically at map insertion time)
- dynamically check that event is local and !pmu->count
Otherwise buggy bpf program can cause kernel splat.
Fixes: 35578d798400 ("bpf: Implement function bpf_perf_event_read() that get the selected hardware PMU conuter")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
v1->v2: fix compile in case of !CONFIG_PERF_EVENTS
This patch is on top of
http://patchwork.ozlabs.org/patch/533585/
to avoid conflicts.
Even in the worst case the crash is not possible.
Only warn_on_once, so imo net-next is ok.
kernel/bpf/arraymap.c | 9 +++++----
kernel/events/core.c | 16 ++++++++++------
2 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index e3cfe46b074f..75529cc94304 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -294,10 +294,11 @@ static void *perf_event_fd_array_get_ptr(struct bpf_map *map, int fd)
if (IS_ERR(attr))
return (void *)attr;
- if (attr->type != PERF_TYPE_RAW &&
- !(attr->type == PERF_TYPE_SOFTWARE &&
- attr->config == PERF_COUNT_SW_BPF_OUTPUT) &&
- attr->type != PERF_TYPE_HARDWARE) {
+ if ((attr->type != PERF_TYPE_RAW &&
+ !(attr->type == PERF_TYPE_SOFTWARE &&
+ attr->config == PERF_COUNT_SW_BPF_OUTPUT) &&
+ attr->type != PERF_TYPE_HARDWARE) ||
+ attr->inherit) {
perf_event_release_kernel(event);
return ERR_PTR(-EINVAL);
}
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 64754bfecd70..0b6333265872 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3258,7 +3258,7 @@ static inline u64 perf_event_count(struct perf_event *event)
u64 perf_event_read_local(struct perf_event *event)
{
unsigned long flags;
- u64 val;
+ u64 val = -EINVAL;
/*
* Disabling interrupts avoids all counter scheduling (context
@@ -3267,12 +3267,14 @@ u64 perf_event_read_local(struct perf_event *event)
local_irq_save(flags);
/* If this is a per-task event, it must be for current */
- WARN_ON_ONCE((event->attach_state & PERF_ATTACH_TASK) &&
- event->hw.target != current);
+ if ((event->attach_state & PERF_ATTACH_TASK) &&
+ event->hw.target != current)
+ goto out;
/* If this is a per-CPU event, it must be for this CPU */
- WARN_ON_ONCE(!(event->attach_state & PERF_ATTACH_TASK) &&
- event->cpu != smp_processor_id());
+ if (!(event->attach_state & PERF_ATTACH_TASK) &&
+ event->cpu != smp_processor_id())
+ goto out;
/*
* It must not be an event with inherit set, we cannot read
@@ -3284,7 +3286,8 @@ u64 perf_event_read_local(struct perf_event *event)
* It must not have a pmu::count method, those are not
* NMI safe.
*/
- WARN_ON_ONCE(event->pmu->count);
+ if (event->pmu->count)
+ goto out;
/*
* If the event is currently on this CPU, its either a per-task event,
@@ -3295,6 +3298,7 @@ u64 perf_event_read_local(struct perf_event *event)
event->pmu->read(event);
val = local64_read(&event->count);
+out:
local_irq_restore(flags);
return val;
--
1.7.9.5
next reply other threads:[~2015-10-21 22:58 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-21 22:58 Alexei Starovoitov [this message]
2015-10-22 4:49 ` [PATCH v2 net-next] bpf: fix bpf_perf_event_read() helper Wangnan (F)
2015-10-22 5:00 ` Alexei Starovoitov
2015-10-22 5:30 ` Wangnan (F)
2015-10-22 5:31 ` Wangnan (F)
2015-10-22 6:21 ` Alexei Starovoitov
2015-10-22 7:39 ` Wangnan (F)
2015-10-22 15:51 ` Alexei Starovoitov
2015-10-22 12:30 ` Wangnan (F)
2015-10-22 12:41 ` Peter Zijlstra
2015-10-22 13:57 ` Peter Zijlstra
2015-10-22 15:53 ` Alexei Starovoitov
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=1445468283-4592-1-git-send-email-ast@kernel.org \
--to=ast@plumgrid.com \
--cc=a.p.zijlstra@chello.nl \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=hekuang@huawei.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=wangnan0@huawei.com \
--cc=xiakaixu@huawei.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).