netdev - bpf memory model. Was: [PATCH v4 bpf-next 1/9] bpf: introduce bpf_spin

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 25 Jan 2019 16:17:26 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Alexei Starovoitov <ast@...nel.org>, davem@...emloft.net,
        daniel@...earbox.net, jakub.kicinski@...ronome.com,
        netdev@...r.kernel.org, kernel-team@...com, mingo@...hat.com,
        will.deacon@....com, Paul McKenney <paulmck@...ux.vnet.ibm.com>,
        jannh@...gle.com
Subject: bpf memory model. Was: [PATCH v4 bpf-next 1/9] bpf: introduce
 bpf_spin_lock

On Fri, Jan 25, 2019 at 11:23:12AM +0100, Peter Zijlstra wrote:
> On Thu, Jan 24, 2019 at 03:58:59PM -0800, Alexei Starovoitov wrote:
> > On Thu, Jan 24, 2019 at 07:01:09PM +0100, Peter Zijlstra wrote:
> 
> > > And this would again be the moment where I go pester you about the BPF
> > > memory model :-)
> > 
> > hehe :)
> > How do you propose to define it in a way that it applies to all archs
> > and yet doesn't penalize x86 ?
> > "Assume minimum execution ordering model" the way kernel does
> > unfortunately is not usable, since bpf doesn't have a luxury
> > of using nice #defines that convert into nops on x86.
> 
> Why not? Surely the JIT can fix it up? That is, suppose you were to have
> smp_rmb() as a eBPF instruction then the JIT (which knows what
> architecture it is targeting) can simply avoid emitting instructions for
> it.

I'm all for adding new instructions that solve real use cases.
imo bpf_spin_lock() is the first step in helping with concurrency.
At plumbers conference we agreed to add new sync_fetch_and_add()
and xchg() instructions. That's a second step.
smp_rmb/wmb/mb should be added as well.
JITs will patch them depending on architecture.

What I want to avoid is to define the whole execution ordering model upfront.
We cannot say that BPF ISA is weakly ordered like alpha.
Most of the bpf progs are written and running on x86. We shouldn't
twist bpf developer's arm by artificially relaxing memory model.
BPF memory model is equal to memory model of underlying architecture.
What we can do is to make it bpf progs a bit more portable with
smp_rmb instructions, but we must not force weak execution on the developer.

> Similarly; could something like this not also help with the spinlock
> thing? Use that generic test-and-set thing for the interpreter, but
> provide a better implementation in the JIT?

May be eventually. We can add cmpxchg insn, but the verifier still
doesn't support loops. We made a lot of progress in bounded loop research
over the last 2 years, but loops in bpf are still a year away.
We considered adding 'bpf_spin_lock' as a new instruction instead of helper call,
but that approach has a lot of negatives, so we went with the helper.