netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Aaron Conole <aconole@bytheb.org>
To: Daniel Borkmann <daniel@iogearbox.net>
Cc: Alexei Starovoitov <ast@kernel.org>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC net-next] bpf: taint loading !is_gpl programs
Date: Wed, 05 Apr 2017 22:59:49 -0400	[thread overview]
Message-ID: <f7tpogqmeai.fsf@redhat.com> (raw)
In-Reply-To: <58E40C2B.7040802@iogearbox.net> (Daniel Borkmann's message of "Tue, 04 Apr 2017 23:12:11 +0200")

Hi Daniel,

Daniel Borkmann <daniel@iogearbox.net> writes:

> On 04/04/2017 08:33 PM, Aaron Conole wrote:
>> The eBPF framework is used for more than just socket level filtering.  It
>> can also provide tracing, and even change the way packets coming into the
>> system look.  Most of the eBPF callable symbols are available to non-gpl
>> programs, and this includes helper functions which modify packets.  This
>> allows proprietary eBPF code to link to the kernel and make decisions
>> which can negatively impact network performance.
>>
>> Since the sources for these programs are only available under a proprietary
>> license, it seems better to treat them the same as other proprietary
>> modules: set the system taint flag.  An exemption is made for socket-level
>> filters, since they do not really impact networking for the whole kernel.
>>
>> Signed-off-by: Aaron Conole <aconole@bytheb.org>
>> ---
>
> Nacked-by: Daniel Borkmann <daniel@iogearbox.net>

Thanks so much for looking at the patch!

> This is proposal completely unreasonable; what the purpose of .gpl_only
> flags is agreed upon since the beginning is that some of the helpers
> are only available if the program is loaded as gpl, f.e. bpf_ktime_get_ns(),
> bpf_probe_read(), bpf_probe_write_user(), bpf_trace_printk(),
> bpf_skb_event_output(), etc.

This behavior isn't changing with this patch.

> Now, suddenly switching from one kernel
> version to another, existing programs would out of a sudden taint the
> kernel, which by itself is unacceptable.

I'm not sure what you mean here.  The kernel should still be usable.  This
basically says that if someone runs non-GPL eBPF code, they are tainting
the kernel.  More below.

> There are also many other
> subsystems that can modify packets, or affect system performance
> negatively if configured wrongly and which in addition *don't require* a
> hard capable(CAP_SYS_ADMIN) restriction like such eBPF programs already
> do, perhaps should we taint them as well?

This is a good point that there are other methods of doing damage to the
network.  I think it means my commit message wasn't clear enough to
describe why I wanted the change.  The reason I propose this isn't
because someone can theoretically damage things.  It's because eBPF
really is a way of writing specialized kernel modules.

I am really making the distinction here that eBPF code (except for the
case of user-space socket filter) is a kernel module.  I realize that
may not be something folks have considered.  Never-the-less, it is code
which runs in the context of the kernel, out lives the lifetime of user
space, and modifies kernel behavior.  These are the main reasons I
believe this is a kernel module.  And since it is a kernel module, it
shouldn't bypass the existing taint flag that says 'someone is running
non-gpl code in kernel space.'  Do you disagree?  This is also why I
exempt socket filter code.  That really is something which I would
consider running as part of a user-space application.

> Plus tracing programs are
> attached to passively monitor systems performance, not even modifying
> data structures

Tracing code, afaict, must be gpl_only = true to be useful.  So I don't see
how it enters into the equation.  Did I misread something?  Most, if not
all of the networking functions, are gpl_only = false.  This means the
community will have a difficult time supporting reports from this
system.  After all, there's no way to know exactly how this eBPF program
has changed packets in the network without a license to the code.

> ... The current purpose of .gpl_only is fine as-is, and
> there's work in progress for a generic dump mechanism that works with
> all program types to improve introspection aspect if that's what you're
> after, starting to taint is, in a way, breaking existing applications
> and this is not acceptable.

I don't see how this breaks applications.  They continue to run fine.
This patch does not restrict functionality.  Again, did I misunderstand
something?

I'm not sure how a dumping mechanism changes anything either.  I agree
such a utility is very useful.  However, if the poor user who is running
a non-gpl eBPF program is asked to provide a dump of that eBPF program,
they may be barred from doing so by licensing.  How can those cases be
supported?

  reply	other threads:[~2017-04-06  2:59 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-04 18:33 [RFC net-next] bpf: taint loading !is_gpl programs Aaron Conole
2017-04-04 21:12 ` Daniel Borkmann
2017-04-06  2:59   ` Aaron Conole [this message]
2017-04-06 12:41     ` Alexei Starovoitov
2017-04-07 17:46       ` Aaron Conole
2017-04-08 23:11         ` 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=f7tpogqmeai.fsf@redhat.com \
    --to=aconole@bytheb.org \
    --cc=ast@kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.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).