bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Joel Fernandes <joel@joelfernandes.org>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: LKML <linux-kernel@vger.kernel.org>, bpf <bpf@vger.kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Network Development <netdev@vger.kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	kernel-team@android.com
Subject: Re: [PATCH RFC 0/4] Add support to directly attach BPF program to ftrace
Date: Wed, 17 Jul 2019 22:51:43 -0400	[thread overview]
Message-ID: <20190718025143.GB153617@google.com> (raw)
In-Reply-To: <CAADnVQJY_=yeY0C3k1ZKpRFu5oNbB4zhQf5tQnLr=Mi8i6cgeQ@mail.gmail.com>

Hi Alexei,

On Wed, Jul 17, 2019 at 02:40:42PM -0700, Alexei Starovoitov wrote:
> On Wed, Jul 17, 2019 at 6:01 AM Joel Fernandes <joel@joelfernandes.org> wrote:
> 
> I trimmed cc. some emails were bouncing.

Ok, thanks.

> > > I think allowing one tracepoint and disallowing another is pointless
> > > from security point of view. Tracing bpf program can do bpf_probe_read
> > > of anything.
> >
> > I think the assumption here is the user controls the program instructions at
> > runtime, but that's not the case. The BPF program we are loading is not
> > dynamically generated, it is built at build time and it is loaded from a
> > secure verified partition, so even though it can do bpf_probe_read, it is
> > still not something that the user can change.
> 
> so you're saying that by having a set of signed bpf programs which
> instructions are known to be non-malicious and allowed set of tracepoints
> to attach via selinux whitelist, such setup will be safe?
> Have you considered how mix and match will behave?

Do you mean the effect of mixing tracepoints and programs? I have not
considered this. I am Ok with further enforcing of this (only certain
tracepoints can be attached to certain programs) if needed. What do
you think? We could have a new bpf(2) syscall attribute specify which
tracepoint is expected, or similar.

I wanted to walk you through our 2 usecases we are working on:

1. timeinstate: By hooking 2 programs onto sched_switch and cpu_frequency
tracepoints, we are able to collect CPU power per-UID (specific app). Connor
O'Brien is working on that.

2. inode to file path mapping: By hooking onto VFS tracepoints we are adding to
the android kernels, we can collect data when the kernel resolves a file path
to a inode/device number. A BPF map stores the inode/dev number (key) and the
path (value). We have usecases where we need a high speed lookup of this
without having to scan all the files in the filesystem.

For the first usecase, the BPF program will be loaded and attached to the
scheduler and cpufreq tracepoints at boot time and will stay attached
forever.  This is why I was saying having a daemon to stay alive all the time
is pointless. However, if since you are completely against using tracefs
which it sounds like, then we can do a daemon that is always alive.

For the second usecase, the program attach is needed on-demand unlike the
first usecase, and then after the usecase completes, it is detached to avoid
overhead.

For the second usecase, privacy is important and we want the data to not be
available to any process. So we want to make sure only selected processes can
attach to that tracepoint. This is the reason why I was doing working on
these patches which use the tracefs as well, since we get that level of
control.

As you can see, I was trying to solve the sticky tracepoint problem in
usecase 1 and the privacy problem in usecase 2.

I had some discussions today at office and we think we can use the daemon
approach to solve both these problems as well which I think would make you
happy as well.

What do you think about all of this? Any other feedback?

> > And, we are planning to make it
> > even more secure by making it kernel verify the program at load time as well
> > (you were on some discussions about that a few months ago).
> 
> It sounds like api decisions for this sticky raw_tp feature are
> driven by security choices which are not actually secure.
> I'm suggesting to avoid bringing up point of security as a reason for
> this api design, since it's making the opposite effect.

Ok, that's a fair point.

thanks,

 - Joel


  reply	other threads:[~2019-07-18  2:51 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-10 14:15 [PATCH RFC 0/4] Add support to directly attach BPF program to ftrace Joel Fernandes (Google)
2019-07-10 14:15 ` [PATCH RFC 1/4] Move bpf_raw_tracepoint functionality into bpf_trace.c Joel Fernandes (Google)
2019-07-10 14:15 ` [PATCH RFC 2/4] trace/bpf: Add support for attach/detach of ftrace events to BPF Joel Fernandes (Google)
2019-07-10 14:15 ` [PATCH RFC 3/4] lib/bpf: Add support for ftrace event attach and detach Joel Fernandes (Google)
2019-07-10 14:15 ` [PATCH RFC 4/4] selftests/bpf: Add test for ftrace-based BPF attach/detach Joel Fernandes (Google)
2019-07-16 20:54 ` [PATCH RFC 0/4] Add support to directly attach BPF program to ftrace Alexei Starovoitov
2019-07-16 21:30   ` Joel Fernandes
2019-07-16 22:26     ` Alexei Starovoitov
2019-07-16 22:41       ` Joel Fernandes
2019-07-16 23:55         ` Joel Fernandes
2019-07-17  1:24           ` Alexei Starovoitov
2019-07-17 13:01             ` Joel Fernandes
2019-07-17 21:40               ` Alexei Starovoitov
2019-07-18  2:51                 ` Joel Fernandes [this message]
2019-07-23 22:11                   ` Alexei Starovoitov
2019-07-24 13:57                     ` Joel Fernandes
2019-07-26 18:39                       ` Alexei Starovoitov
2019-07-26 19:18                         ` Joel Fernandes
2019-07-26 19:49                           ` Joel Fernandes
2019-07-16 22:43       ` Steven Rostedt
2019-07-16 22:31     ` Steven Rostedt
2019-07-16 22:46       ` Joel Fernandes
2019-07-17  1:30       ` Alexei Starovoitov

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=20190718025143.GB153617@google.com \
    --to=joel@joelfernandes.org \
    --cc=alexei.starovoitov@gmail.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    /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).