Re: [RFC PATCH] tracing/user_events: Limit showing event names to CAP_SYS_ADMIN users

[Beau Belgrave <beaub@linux.microsoft.com>]

On Fri, Mar 11, 2022 at 05:01:40PM -0800, Beau Belgrave wrote:
> Show actual names only to CAP_SYS_ADMIN capable users.
> 
> When user_events are configured to have broader write access than
> default, this allows seeing names of events from other containers, etc.
> Limit who can see the actual names to prevent event squatting or
> information leakage.
> 
> Signed-off-by: Beau Belgrave <beaub@linux.microsoft.com>
> ---
>  kernel/trace/trace_events_user.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c
> index 2b5e9fdb63a0..fb9fb2071173 100644
> --- a/kernel/trace/trace_events_user.c
> +++ b/kernel/trace/trace_events_user.c
> @@ -1480,6 +1480,9 @@ static int user_seq_show(struct seq_file *m, void *p)
>  	struct user_event *user;
>  	char status;
>  	int i, active = 0, busy = 0, flags;
> +	bool show_names;
> +
> +	show_names = capable(CAP_SYS_ADMIN);
>  
>  	mutex_lock(&reg_mutex);
>  
> @@ -1487,7 +1490,10 @@ static int user_seq_show(struct seq_file *m, void *p)
>  		status = register_page_data[user->index];
>  		flags = user->flags;
>  
> -		seq_printf(m, "%d:%s", user->index, EVENT_NAME(user));
> +		if (show_names)
> +			seq_printf(m, "%d:%s", user->index, EVENT_NAME(user));
> +		else
> +			seq_printf(m, "%d:<hidden>", user->index);
>  
>  		if (flags != 0 || status != 0)
>  			seq_puts(m, " #");
> 
> base-commit: 864ea0e10cc90416a01b46f0d47a6f26dc020820
> -- 
> 2.17.1

I wanted to get some comments on this. I think for scenarios where
user_events is used in a heavy cgroup environment, that we need to have
some tracing cgroup awareness.

Has this come up before? I would like to only show user_events that have
been created in the current cgroup (and below) like perf_events do for
capturing.

I would also like to get to a point where we can limit how many events
each cgroup can register under user_events.

To me, this sounds like a large feature that requires some alignment for
getting tracing cgroup aware.

Thoughts?

Thanks,
-Beau