public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
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 00:59:46 +0100	[thread overview]
Message-ID: <56959372.3040409@stressinduktion.org> (raw)
In-Reply-To: <20160112234707.GA36167@ast-mbp.thefacebook.com>

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.

Bye,
Hannes

  reply	other threads:[~2016-01-12 23:59 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 [this message]
2016-01-13  0:17                   ` Hannes Frederic Sowa
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=56959372.3040409@stressinduktion.org \
    --to=hannes@stressinduktion$(echo .)org \
    --cc=alexei.starovoitov@gmail$(echo .)com \
    --cc=ast@kernel$(echo .)org \
    --cc=daniel@iogearbox$(echo .)net \
    --cc=davem@davemloft$(echo .)net \
    --cc=eric.dumazet@gmail$(echo .)com \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=rabin@rab$(echo .)in \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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