All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.