* [RFC net-next] bpf: taint loading !is_gpl programs @ 2017-04-04 18:33 Aaron Conole 2017-04-04 21:12 ` Daniel Borkmann 0 siblings, 1 reply; 6+ messages in thread From: Aaron Conole @ 2017-04-04 18:33 UTC (permalink / raw) To: Alexei Starovoitov; +Cc: netdev, linux-kernel 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> --- kernel/bpf/syscall.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index ab0cf4c4..1255b51 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -860,6 +860,11 @@ static int bpf_prog_load(union bpf_attr *attr) bpf_prog_kallsyms_add(prog); trace_bpf_prog_load(prog, err); + if (type != BPF_PROG_TYPE_SOCKET_FILTER && !is_gpl && !(err < 0)) { + if (!test_taint(TAINT_PROPRIETARY_MODULE)) + pr_warn("bpf license '%s' taints kernel.\n", license); + add_taint(TAINT_PROPRIETARY_MODULE, LOCKDEP_STILL_OK); + } return err; free_used_maps: -- 2.9.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC net-next] bpf: taint loading !is_gpl programs 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 0 siblings, 1 reply; 6+ messages in thread From: Daniel Borkmann @ 2017-04-04 21:12 UTC (permalink / raw) To: Aaron Conole, Alexei Starovoitov; +Cc: netdev, linux-kernel 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> > --- > kernel/bpf/syscall.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > index ab0cf4c4..1255b51 100644 > --- a/kernel/bpf/syscall.c > +++ b/kernel/bpf/syscall.c > @@ -860,6 +860,11 @@ static int bpf_prog_load(union bpf_attr *attr) > > bpf_prog_kallsyms_add(prog); > trace_bpf_prog_load(prog, err); > + if (type != BPF_PROG_TYPE_SOCKET_FILTER && !is_gpl && !(err < 0)) { > + if (!test_taint(TAINT_PROPRIETARY_MODULE)) > + pr_warn("bpf license '%s' taints kernel.\n", license); > + add_taint(TAINT_PROPRIETARY_MODULE, LOCKDEP_STILL_OK); > + } > return err; > > free_used_maps: > Nacked-by: Daniel Borkmann <daniel@iogearbox.net> 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. Now, suddenly switching from one kernel version to another, existing programs would out of a sudden taint the kernel, which by itself is unacceptable. 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? Plus tracing programs are attached to passively monitor systems performance, not even modifying data structures ... 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC net-next] bpf: taint loading !is_gpl programs 2017-04-04 21:12 ` Daniel Borkmann @ 2017-04-06 2:59 ` Aaron Conole 2017-04-06 12:41 ` Alexei Starovoitov 0 siblings, 1 reply; 6+ messages in thread From: Aaron Conole @ 2017-04-06 2:59 UTC (permalink / raw) To: Daniel Borkmann; +Cc: Alexei Starovoitov, netdev, linux-kernel 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? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC net-next] bpf: taint loading !is_gpl programs 2017-04-06 2:59 ` Aaron Conole @ 2017-04-06 12:41 ` Alexei Starovoitov 2017-04-07 17:46 ` Aaron Conole 0 siblings, 1 reply; 6+ messages in thread From: Alexei Starovoitov @ 2017-04-06 12:41 UTC (permalink / raw) To: Aaron Conole; +Cc: Daniel Borkmann, Alexei Starovoitov, netdev, linux-kernel On Wed, Apr 05, 2017 at 10:59:49PM -0400, Aaron Conole wrote: > 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! Passive aggressive way of saying 'I want to waste your time'. Not cool. > > 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. It's breaking userspace, hence obvious Nack. > 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. bpf is the user space program. Whatever you mean by 'community' is obviously cannot 'support' all possible user space applications written using given kernel abi. The reason tracing progs are gpl because they look into gpl-ed kernel, networking progs don't. They deal with bytes on the wire. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC net-next] bpf: taint loading !is_gpl programs 2017-04-06 12:41 ` Alexei Starovoitov @ 2017-04-07 17:46 ` Aaron Conole 2017-04-08 23:11 ` Alexei Starovoitov 0 siblings, 1 reply; 6+ messages in thread From: Aaron Conole @ 2017-04-07 17:46 UTC (permalink / raw) To: Alexei Starovoitov Cc: Daniel Borkmann, Alexei Starovoitov, netdev, linux-kernel Hi Alexei, and Daniel, Alexei Starovoitov <alexei.starovoitov@gmail.com> writes: > On Wed, Apr 05, 2017 at 10:59:49PM -0400, Aaron Conole wrote: >> 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> Given we have different views about this, I think I am okay with some middle ground. Here's the next-steps plan. Please tell if you dislike it or want to change it: 1. Add a ref counter for tracking load and unload, which can be queried from a procfs or bpf fs interface 2. Add a new print during panic when the refcount is non-zero. This lets us know that there could be some kind of ebpf program loaded, and we would ask for sources before trying to disassemble. Does this sound reasonable? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC net-next] bpf: taint loading !is_gpl programs 2017-04-07 17:46 ` Aaron Conole @ 2017-04-08 23:11 ` Alexei Starovoitov 0 siblings, 0 replies; 6+ messages in thread From: Alexei Starovoitov @ 2017-04-08 23:11 UTC (permalink / raw) To: Aaron Conole; +Cc: Daniel Borkmann, Alexei Starovoitov, netdev, linux-kernel On Fri, Apr 07, 2017 at 01:46:28PM -0400, Aaron Conole wrote: > Hi Alexei, and Daniel, > > Alexei Starovoitov <alexei.starovoitov@gmail.com> writes: > > > On Wed, Apr 05, 2017 at 10:59:49PM -0400, Aaron Conole wrote: > >> 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> > > Given we have different views about this, I think I am okay with some > middle ground. > > Here's the next-steps plan. Please tell if you dislike it or want to > change it: > > 1. Add a ref counter for tracking load and unload, which can be queried > from a procfs or bpf fs interface > > 2. Add a new print during panic when the refcount is non-zero. > > This lets us know that there could be some kind of ebpf program loaded, > and we would ask for sources before trying to disassemble. > > Does this sound reasonable? yeah. for the purpose of identifying whether any classic or extended bpf programs loaded that makes sense. The only question how far we should take it, since nft and acpi bytecode falls into the same category. Also my understanding this is just one of out of many items on redhat todo list to make bpf supported in rhel, so I think it makes sense to discuss the whole list all at once. If we add patches here and there without having full picture we may end up with obsolete api or api superseded by other patches/features. So let's discuss all feature requests first. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-04-08 23:11 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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 2017-04-06 12:41 ` Alexei Starovoitov 2017-04-07 17:46 ` Aaron Conole 2017-04-08 23:11 ` Alexei Starovoitov
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.