From: Hannes Frederic Sowa <hannes@stressinduktion.org>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
Eric Dumazet <eric.dumazet@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>,
Rabin Vincent <rabin@rab.in>,
davem@davemloft.net, netdev@vger.kernel.org, ast@kernel.org,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCHv2] net: bpf: reject invalid shifts
Date: Wed, 13 Jan 2016 01:17:01 +0100 [thread overview]
Message-ID: <5695977D.6060100@stressinduktion.org> (raw)
In-Reply-To: <56959372.3040409@stressinduktion.org>
On 13.01.2016 00:59, Hannes Frederic Sowa wrote:
> On 13.01.2016 00:47, Alexei Starovoitov wrote:
>> On Tue, Jan 12, 2016 at 03:28:22PM -0800, Eric Dumazet wrote:
>>> On Tue, 2016-01-12 at 12:46 -0800, Alexei Starovoitov wrote:
>>>> On Tue, Jan 12, 2016 at 09:42:39PM +0100, Daniel Borkmann wrote:
>>>
>>>>>> yep and we all know who was able to code hundreds of cBPF insns by
>>>>>> hand ;)
>>>>>> But I'm sure that code doesn't have such broken shifts. :)))
>>>>>
>>>>> libpcap certainly supports raw filters now thanks to Chema [1].
>>>>> Alternative
>>>>> could be to just mask them here, but not in eBPF verifier, but that
>>>>> would be
>>>>> even more inconsistent (on the other hand, we also allow holes in
>>>>> BPF but not
>>>>> in eBPF, so wouldn't be the first time we make things different), hmm.
>>>>
>>>> I would rather see broken classic bpf program fixed instead of continue
>>>> running them with undefined behavior.
>>>
>>> This is your choice, because you are a developer.
>>>
>>> Some people might be stuck with old software they can not update,
>>> because they do not have the money to pay developers.
>>>
>>> And no, I did not code BPF programs like that, but maybe others did, and
>>> I feel the pain of customers that might be stuck.
>>>
>>> Linus Torvalds always made clear we must provide backward compatibility,
>>> and really this discussion should not even take place.
>>>
>>> As I said, we used to load such BPF program in the past.
>>>
>>> The fact that ARM64 crashes because of a faulty JIT implementation is
>>> not an excuse.
>>
>> I would agree if those loaded programs would do something sensible,
>> but they're broken. As shown arm and arm64 would execute them
>> differently without JIT, because HW treats such shifts differently.
>> I also checked that libpcap is sane and doesn't generate broken shifts.
>> imo we're not breaking backward compatiblity here.
>
> But on one specific platform those programs did something deterministic,
> reproducible and observable, no? Probably most developers only cared
> about that, probably especially in the embedded segment.
By the way, we can annotate the JIT interpreter with an
__attribute__((no_sanitize_undefined)) to get away with the ubsan report.
Then only the BUG_ONs in arm64 code emit lib are a problem, no?
Bye,
Hannes
next prev parent reply other threads:[~2016-01-13 0:17 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-12 17:55 [PATCH] net: bpf: reject invalid shifts Rabin Vincent
2016-01-12 18:51 ` Alexei Starovoitov
2016-01-12 19:17 ` [PATCHv2] " Rabin Vincent
2016-01-12 19:26 ` Alexei Starovoitov
2016-01-12 19:35 ` Daniel Borkmann
2016-01-12 19:48 ` Eric Dumazet
2016-01-12 19:53 ` Alexei Starovoitov
2016-01-12 20:42 ` Daniel Borkmann
2016-01-12 20:46 ` Alexei Starovoitov
2016-01-12 23:28 ` Eric Dumazet
2016-01-12 23:47 ` Alexei Starovoitov
2016-01-12 23:59 ` Hannes Frederic Sowa
2016-01-13 0:17 ` Hannes Frederic Sowa [this message]
2016-01-13 0:19 ` Alexei Starovoitov
2016-01-13 0:42 ` Hannes Frederic Sowa
2016-01-13 2:11 ` Eric Dumazet
2016-01-13 2:24 ` Alexei Starovoitov
2016-01-13 2:45 ` David Miller
2016-01-13 2:43 ` David Miller
2016-01-13 4:07 ` Eric Dumazet
2016-01-13 5:00 ` David Miller
2016-01-13 4:27 ` Hannes Frederic Sowa
2016-01-12 22:34 ` Eric Dumazet
2016-01-12 20:56 ` David Miller
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=5695977D.6060100@stressinduktion.org \
--to=hannes@stressinduktion.org \
--cc=alexei.starovoitov@gmail.com \
--cc=ast@kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=eric.dumazet@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=netdev@vger.kernel.org \
--cc=rabin@rab.in \
/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).