[PATCHv2] net: bpf: reject invalid shifts

[Hannes Frederic Sowa]

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