netdev - Re: [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:   Thu, 24 Jan 2019 17:22:24 -0800
From:   "Paul E. McKenney" <paulmck@...ux.ibm.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        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, jannh@...gle.com
Subject: Re: [PATCH v4 bpf-next 1/9] bpf: introduce bpf_spin_lock

On Thu, Jan 24, 2019 at 04:05:16PM -0800, Alexei Starovoitov wrote:
> On Thu, Jan 24, 2019 at 03:42:32PM -0800, Paul E. McKenney wrote:
> > On Thu, Jan 24, 2019 at 07:56:52PM +0100, Peter Zijlstra wrote:
> > > On Thu, Jan 24, 2019 at 07:01:09PM +0100, Peter Zijlstra wrote:
> > > > 
> > > > Thanks for having kernel/locking people on Cc...
> > > > 
> > > > On Wed, Jan 23, 2019 at 08:13:55PM -0800, Alexei Starovoitov wrote:
> > > > 
> > > > > Implementation details:
> > > > > - on !SMP bpf_spin_lock() becomes nop
> > > > 
> > > > Because no BPF program is preemptible? I don't see any assertions or
> > > > even a comment that says this code is non-preemptible.
> > > > 
> > > > AFAICT some of the BPF_RUN_PROG things are under rcu_read_lock() only,
> > > > which is not sufficient.
> > > > 
> > > > > - on architectures that don't support queued_spin_lock trivial lock is used.
> > > > >   Note that arch_spin_lock cannot be used, since not all archs agree that
> > > > >   zero == unlocked and sizeof(arch_spinlock_t) != sizeof(__u32).
> > > > 
> > > > I really don't much like direct usage of qspinlock; esp. not as a
> > > > surprise.
> > 
> > Substituting the lightweight-reader SRCU as discussed earlier would allow
> > use of a more generic locking primitive, for example, one that allowed
> > blocking, at least in cases were the context allowed this.
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git
> > branch srcu-lr.2019.01.16a.
> > 
> > One advantage of a more generic locking primitive would be keeping BPF
> > programs independent of internal changes to spinlock primitives.
> 
> Let's keep "srcu in bpf" discussion separate from bpf_spin_lock discussion.
> bpf is not switching to srcu any time soon.
> If/when it happens it will be only for certain prog+map types
> like bpf syscall probes that need to be able to do copy_from_user
> from bpf prog.

Hmmm...  What prevents BPF programs from looping infinitely within an
RCU reader, and as you noted, preemption disabled?

If BPF programs are in fact allowed to loop infinitely, it would be
very good for the health of the kernel to have preemption enabled.
And to be within an SRCU read-side critical section instead of an RCU
read-side critical section.

							Thanx, Paul