linux-kernel - Re: [BUG] perf stat: events inheritance can break task targets

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 7 Jul 2014 19:00:40 +0200
From:	Jiri Olsa <jolsa@...hat.com>
To:	Alexander Yarygin <yarygin@...ux.vnet.ibm.com>
Cc:	Arnaldo Carvalho de Melo <acme@...nel.com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Paul Mackerras <paulus@...ba.org>,
	Ingo Molnar <mingo@...nel.org>,
	Christian Borntraeger <borntraeger@...ibm.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [BUG] perf stat: events inheritance can break task targets

On Mon, Jul 07, 2014 at 08:41:57PM +0400, Alexander Yarygin wrote:

SNIP

> ^C
> 
> When perf is running, every invoke of pthread_create() returns -EPERM.
> 
> On the kernel side, copy_process() creates a task, scheduled it,
> than perf_event_init_task() (kernel/events/core.c) returns an error,
> and the kernel cleans task's resources.
> 
> It looks like child process doesn't have access to trace events,
> so perf_trace_event_perm() (kernel/trace/trace_event_perf.c)
> returns -EPERM:
> 
> static int perf_trace_event_perm(struct ftrace_event_call *tp_event,
> 				 struct perf_event *p_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))
>         return -EPERM;
> ...
> }

right, so normaly it's done the check is done in current context
via syscall and the current task is the tracer

while in inheriting we are checking the tracee here,
we want to check the owner instead like in attached
patch.. totaly untested, just to ilustrate the point

we might have same issue for other event types

jirka


---
diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
index 5d12bb4..b44184b 100644
--- a/kernel/trace/trace_event_perf.c
+++ b/kernel/trace/trace_event_perf.c
@@ -24,6 +24,9 @@ static int	total_ref_count;
 static int perf_trace_event_perm(struct ftrace_event_call *tp_event,
 				 struct perf_event *p_event)
 {
+	struct task_struct owner = p_event->parent ? p_event->parent->owner :
+						     p_event->owner;
+
 	if (tp_event->perf_perm) {
 		int ret = tp_event->perf_perm(tp_event, p_event);
 		if (ret)
@@ -32,7 +35,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() && !has_capability(owner, CAP_SYS_ADMIN))
 			return -EPERM;
 
 		/*
@@ -65,7 +68,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() && !has_capability(owner, CAP_SYS_ADMIN))
 		return -EPERM;
 
 	return 0;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/