Re: [PATCH v2] perf: allow non-privileged uprobe for user processes

[Song Liu <songliubraving@fb.com>]

Hi Peter, 

> On May 7, 2019, at 9:15 AM, Song Liu <songliubraving@fb.com> wrote:
> 
> Currently, non-privileged user could only use uprobe with
> 
>    kernel.perf_event_paranoid = -1
> 
> However, setting perf_event_paranoid to -1 leaks other users' processes to
> non-privileged uprobes.
> 
> To introduce proper permission control of uprobes, we are building the
> following system:
>  A daemon with CAP_SYS_ADMIN is in charge to create uprobes via tracefs;
>  Users asks the daemon to create uprobes;
>  Then user can attach uprobe only to processes owned by the user.
> 
> This patch allows non-privileged user to attach uprobe to processes owned
> by the user.
> 
> The following example shows how to use uprobe with non-privileged user.
> This is based on Brendan's blog post [1]
> 
> 1. Create uprobe with root:
>  sudo perf probe -x 'readline%return +0($retval):string'
> 
> 2. Then non-root user can use the uprobe as:
>  perf record -vvv -e probe_bash:readline__return -p <pid> sleep 20
>  perf script
> 
> [1] http://www.brendangregg.com/blog/2015-06-28/linux-ftrace-uprobe.html
> 
> Signed-off-by: Song Liu <songliubraving@fb.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Jiri Olsa <jolsa@redhat.com>
> ---
> kernel/events/core.c        | 4 ++--
> kernel/trace/trace_uprobe.c | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index abbd4b3b96c2..3005c80f621d 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -8532,9 +8532,9 @@ static int perf_tp_event_match(struct perf_event *event,
> 	if (event->hw.state & PERF_HES_STOPPED)
> 		return 0;
> 	/*
> -	 * All tracepoints are from kernel-space.
> +	 * If exclude_kernel, only trace user-space tracepoints (uprobes)
> 	 */
> -	if (event->attr.exclude_kernel)
> +	if (event->attr.exclude_kernel && !user_mode(regs))
> 		return 0;
> 
> 	if (!perf_tp_filter_match(event, data))
> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> index be78d99ee6bc..bfd3040b4cfb 100644
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -1304,7 +1304,7 @@ static inline void init_trace_event_call(struct trace_uprobe *tu,
> 	call->event.funcs = &uprobe_funcs;
> 	call->class->define_fields = uprobe_event_define_fields;
> 
> -	call->flags = TRACE_EVENT_FL_UPROBE;
> +	call->flags = TRACE_EVENT_FL_UPROBE | TRACE_EVENT_FL_CAP_ANY;
> 	call->class->reg = trace_uprobe_register;
> 	call->data = tu;
> }
> -- 
> 2.17.1
> 

Could you please share your feedback on this version?

Thanks,
Song