From: Jiri Olsa <jolsa@kernel.org>
To: linux-kernel@vger.kernel.org
Cc: Jiri Olsa <jolsa@kernel.org>,
Alexander Yarygin <yarygin@linux.vnet.ibm.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Corey Ashford <cjashfor@linux.vnet.ibm.com>,
Frederic Weisbecker <fweisbec@gmail.com>,
Ingo Molnar <mingo@kernel.org>, Paul Mackerras <paulus@samba.org>,
Peter Zijlstra <a.p.zijlstra@chello.nl>
Subject: [PATCH 5/5] perf: Check event's owner permission in tracepoint init callback
Date: Fri, 11 Jul 2014 13:56:22 +0200 [thread overview]
Message-ID: <1405079782-8139-6-git-send-email-jolsa@kernel.org> (raw)
In-Reply-To: <1405079782-8139-1-git-send-email-jolsa@kernel.org>
Checking event's owner task permissions instead of 'current'
process' ones. This way we get proper check when this is
called from fork callback, where 'current' process does not
represent the owner.
Checking parent event owner task in case of child event.
Reported-by: Alexander Yarygin <yarygin@linux.vnet.ibm.com>
Cc: Alexander Yarygin <yarygin@linux.vnet.ibm.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
kernel/events/core.c | 4 ++++
kernel/trace/trace_event_perf.c | 19 +++++++++++++++++--
2 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index faba9e0..c478828 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3315,6 +3315,10 @@ struct task_struct *perf_event_get_owner(struct perf_event *event)
{
struct task_struct *owner;
+ /* Only the parent holds owner task. */
+ if (event->parent)
+ event = event->parent;
+
rcu_read_lock();
owner = ACCESS_ONCE(event->owner);
/*
diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
index 5d12bb4..35b76ec 100644
--- a/kernel/trace/trace_event_perf.c
+++ b/kernel/trace/trace_event_perf.c
@@ -21,9 +21,24 @@ typedef typeof(unsigned long [PERF_MAX_TRACE_SIZE / sizeof(unsigned long)])
/* Count the events in use (per event id, not per instance) */
static int total_ref_count;
+static bool is_owner_admin(struct perf_event *event)
+{
+ struct task_struct *owner = perf_event_get_owner(event);
+ bool admin = false;
+
+ if (owner) {
+ admin = has_capability(owner, CAP_SYS_ADMIN);
+ put_task_struct(owner);
+ }
+
+ return admin;
+}
+
static int perf_trace_event_perm(struct ftrace_event_call *tp_event,
struct perf_event *p_event)
{
+ bool admin = is_owner_admin(p_event);
+
if (tp_event->perf_perm) {
int ret = tp_event->perf_perm(tp_event, p_event);
if (ret)
@@ -32,7 +47,7 @@ static int perf_trace_event_perm(struct ftrace_event_call *tp_event,
/* The ftrace function trace is allowed only for root. */
if (ftrace_event_is_function(tp_event)) {
- if (perf_paranoid_tracepoint_raw() && !capable(CAP_SYS_ADMIN))
+ if (perf_paranoid_tracepoint_raw() && !admin)
return -EPERM;
/*
@@ -65,7 +80,7 @@ static int perf_trace_event_perm(struct ftrace_event_call *tp_event,
* ...otherwise raw tracepoint data can be a severe data leak,
* only allow root to have these.
*/
- if (perf_paranoid_tracepoint_raw() && !capable(CAP_SYS_ADMIN))
+ if (perf_paranoid_tracepoint_raw() && !admin)
return -EPERM;
return 0;
--
1.8.3.1
next prev parent reply other threads:[~2014-07-11 11:57 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-11 11:56 [PATCH 0/5] perf: Fix tracepoint events permissions check Jiri Olsa
2014-07-11 11:56 ` [PATCH 1/5] perf: Make perf_init_event function static Jiri Olsa
2014-07-11 11:56 ` [PATCH 2/5] perf: Destroy event's children on task exit Jiri Olsa
2014-07-11 13:23 ` Peter Zijlstra
2014-07-11 13:31 ` Jiri Olsa
2014-07-16 12:20 ` Ingo Molnar
2014-07-14 11:18 ` Peter Zijlstra
2014-07-14 11:43 ` Jiri Olsa
2014-07-14 13:02 ` Peter Zijlstra
2014-07-14 13:22 ` Jiri Olsa
2014-07-14 13:35 ` Peter Zijlstra
2014-07-14 14:21 ` Jiri Olsa
2014-07-16 12:14 ` Jiri Olsa
2014-07-14 20:18 ` Jiri Olsa
2014-07-15 9:11 ` Peter Zijlstra
2014-07-15 9:31 ` Jiri Olsa
2014-07-11 11:56 ` [PATCH 3/5] perf: Initialize owner before calling event_init callback Jiri Olsa
2014-07-11 11:56 ` [PATCH 4/5] perf: Move event owner retrieval into perf_event_get_owner Jiri Olsa
2014-07-11 11:56 ` Jiri Olsa [this message]
2014-07-11 12:02 ` [PATCH 0/5] perf: Fix tracepoint events permissions check Jiri Olsa
2014-07-28 8:28 ` [tip:perf/core] perf: Check permission only for parent tracepoint event tip-bot for 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=1405079782-8139-6-git-send-email-jolsa@kernel.org \
--to=jolsa@kernel.org \
--cc=a.p.zijlstra@chello.nl \
--cc=acme@kernel.org \
--cc=cjashfor@linux.vnet.ibm.com \
--cc=fweisbec@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=paulus@samba.org \
--cc=yarygin@linux.vnet.ibm.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