Re: [PATCHv2] net: bpf: reject invalid shifts

On 13.01.2016 00:59, Hannes Frederic Sowa wrote:

On 13.01.2016 00:47, Alexei Starovoitov wrote:

quoted

On Tue, Jan 12, 2016 at 03:28:22PM -0800, Eric Dumazet wrote:

quoted

On Tue, 2016-01-12 at 12:46 -0800, Alexei Starovoitov wrote:

quoted

On Tue, Jan 12, 2016 at 09:42:39PM +0100, Daniel Borkmann wrote:

quoted

quoted

quoted

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