linux-trace-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Beau Belgrave <beaub@linux.microsoft.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-trace-kernel@vger.kernel.org,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>, bpf <bpf@vger.kernel.org>,
	David Vernet <void@manifault.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	dthaler@microsoft.com, brauner@kernel.org, hch@infradead.org
Subject: Re: [PATCH] tracing/user_events: Run BPF program if attached
Date: Wed, 17 May 2023 09:50:28 -0700	[thread overview]
Message-ID: <20230517165028.GA71@W11-BEAU-MD.localdomain> (raw)
In-Reply-To: <20230516212658.2f5cc2c6@gandalf.local.home>

On Tue, May 16, 2023 at 09:26:58PM -0400, Steven Rostedt wrote:
> On Tue, 16 May 2023 17:36:28 -0700
> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> 
> 
> > "
> > The user that will generate events must have x access to the tracing directory, e.g. chmod a+x /sys/kernel/tracing
> > The user that will generate events must have rw access to the tracing/user_events_data file, e.g. chmod a+rw /sys/kernel/tracing/user_events_data
> > "
> > So any unpriv user can create and operate user events.
> > Including seeing and enabling other user's user_events with 'ls/echo/cat' in tracefs.
> 
> It can see user_events_data, but x only gives you access into the directory.
> It does not get you the contents of the files within the directory. The
> above only gives access to the user_events_data. Which is to create events.
> 
> I recommended using groups and not giving access to all tasks.
> 
> > 
> > Looks like user events were designed with intention to be unprivileged.
> > When I looked at kernel/trace/trace_events_user.c I assumed root.
> > I doubt other people reviewed it from security perspective.
> > 
> > Recommending "chmod a+rw /sys/kernel/tracing/user_events_data" doesn't sound like a good idea.
> > 
> > For example, I think the following is possible:
> > fd = open("/sys/kernel/tracing/user_events_data")
> > ioclt(fd, DIAG_IOCSDEL)
> >   user_events_ioctl_del
> >      delete_user_event(info->group, name);
> > 
> > 'info' is different for every FD, but info->group is the same for all users/processes/fds,
> > because only one global init_group is created.
> > So one user can unregister other user event by knowing 'name'.
> > A security hole, no?
> > 
> > > and libside [2] will also help here.  
> > 
> > > [2] https://github.com/compudj/libside  
> > 
> > That's an interesting project. It doesn't do any user_events access afaict,
> 
> I'll let Beau answer the rest.
> 
> -- Steve

Mathieu and I have talked for the last year to align user_events with
the ability to also run user-space tracers together. I've sent a patch
out to Mathieu to add user_events to libside and was the main reason why
the ABI moved toward remote writes of bits.

Libside uses a binary description of event data that the kernel cannot
handle (yet). We talk about this almost each tracefs meeting, libside
can be used with user_events, however, the kernel side decoding is hard
to describe at the moment. We are working on a way to tell the kernel
about events via a binary format to achieve this.

Regarding deleting events, only users that are given access can delete
events. They must know the event name, just like users with access to
delete files must know a path (and have access to it). Since the
write_index and other details are per-process, unless the user has
access to either /sys/kernel/tracing/events/user_events/* or
/sys/kernel/tracing/user_events_status, they do not know which names are
being used.

If that is not enough, we could require CAP_SYSADMIN to be able to
delete events even when they have access to the file. Users can also
apply SELinux policies per-file to achieve further isolation, if
required.

Thanks,
-Beau

  reply	other threads:[~2023-05-17 16:51 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-08 16:37 [PATCH] tracing/user_events: Run BPF program if attached Beau Belgrave
2023-05-09 15:24 ` Alexei Starovoitov
2023-05-09 17:01   ` Steven Rostedt
2023-05-09 20:30     ` Steven Rostedt
2023-05-09 20:42       ` Steven Rostedt
2023-05-15 16:57       ` Alexei Starovoitov
2023-05-15 18:33         ` Steven Rostedt
2023-05-15 19:35           ` Beau Belgrave
2023-05-15 21:38             ` Steven Rostedt
2023-05-15 19:24         ` Beau Belgrave
2023-05-15 21:57           ` Steven Rostedt
2023-05-17  0:36           ` Alexei Starovoitov
2023-05-17  0:56             ` Linus Torvalds
2023-05-17  1:46               ` Linus Torvalds
2023-05-17  2:29                 ` Steven Rostedt
2023-05-17  3:03                   ` Linus Torvalds
2023-05-17 17:22                     ` Beau Belgrave
2023-05-17 18:15                       ` Linus Torvalds
2023-05-17 19:07                         ` Beau Belgrave
2023-05-17 19:26                           ` Linus Torvalds
2023-05-17 19:36                             ` Beau Belgrave
2023-05-17 19:36                             ` Linus Torvalds
2023-05-17 19:37                               ` Linus Torvalds
2023-05-17 23:00                                 ` Beau Belgrave
2023-05-17 23:14                                   ` Linus Torvalds
2023-05-17 23:25                                     ` Steven Rostedt
2023-05-18  0:14                                       ` Beau Belgrave
2023-05-18  0:23                                         ` Linus Torvalds
2023-05-17 20:08                               ` Linus Torvalds
2023-05-17  1:26             ` Steven Rostedt
2023-05-17 16:50               ` Beau Belgrave [this message]
2023-05-18  0:10                 ` Alexei Starovoitov
2023-05-18  0:19                   ` Beau Belgrave
2023-05-18  0:56                     ` Alexei Starovoitov
2023-05-18  1:18                       ` Beau Belgrave
2023-05-18  2:08                         ` Steven Rostedt
2023-05-18  3:14                           ` Alexei Starovoitov
2023-05-18 13:36                             ` Steven Rostedt
2023-05-18 17:28                               ` Beau Belgrave
2023-06-01  9:46                   ` Christian Brauner
2023-06-01 15:24                     ` Beau Belgrave
2023-06-01 15:57                       ` Christian Brauner
2023-06-01 16:29                         ` Beau Belgrave
2023-06-06 13:37                           ` Masami Hiramatsu
2023-06-06 17:05                             ` Beau Belgrave
2023-06-07 14:07                               ` Masami Hiramatsu
2023-06-07 19:26                                 ` Beau Belgrave
2023-06-08  0:25                                   ` Masami Hiramatsu
2023-05-17 17:51             ` Beau Belgrave
2023-06-06 13:57             ` Masami Hiramatsu
2023-06-06 16:57               ` Andrii Nakryiko
2023-06-06 20:57                 ` Beau Belgrave

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=20230517165028.GA71@W11-BEAU-MD.localdomain \
    --to=beaub@linux.microsoft.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=brauner@kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=dthaler@microsoft.com \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=torvalds@linux-foundation.org \
    --cc=void@manifault.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;
as well as URLs for NNTP newsgroup(s).