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

[Jiri Olsa <jolsa@redhat.com>]

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;