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
| ||
|
Message-ID: <20190102141101.58e28b28@cakuba.hsd1.ca.comcast.net> Date: Wed, 2 Jan 2019 14:11:01 -0800 From: Jakub Kicinski <jakub.kicinski@...ronome.com> To: Daniel Borkmann <daniel@...earbox.net> Cc: ast@...nel.org, jannh@...gle.com, davem@...emloft.net, netdev@...r.kernel.org Subject: Re: [PATCH bpf v2 8/9] bpf: prevent out of bounds speculation on pointer arithmetic On Wed, 2 Jan 2019 00:20:45 +0100, Daniel Borkmann wrote: > Jann reported that the original commit back in b2157399cc98 > ("bpf: prevent out-of-bounds speculation") was not sufficient > to stop CPU from speculating out of bounds memory access: > While b2157399cc98 only focussed on masking array map access > for unprivileged users for tail calls and data access such > that the user provided index gets sanitized from BPF program > and syscall side, there is still a more generic form affected > from BPF programs that applies to most maps that hold user > data in relation to dynamic map access when dealing with > unknown scalars or "slow" known scalars as access offset... > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 8e5da1c..448a828 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -5499,6 +5610,13 @@ static bool states_equal(struct bpf_verifier_env *env, > if (old->curframe != cur->curframe) > return false; > > + /* Verification state from speculative execution simulation > + * must never prune a non-speculative execution one. > + */ > + if (old->speculative != cur->speculative || > + (old->speculative && !cur->speculative)) > + return false; nit: if I read this correctly it looks more conservative than the comment suggests. The second case (old->speculative && !cur->speculative) implies the first case (old->speculative != cur->speculative). Perhaps: if (old->speculative && !cur->speculative) Or: if (old->speculative > cur->speculative) > + > /* for states to be equal callsites have to be the same > * and all frame states need to be equivalent > */ > @@ -5530,6 +5648,11 @@ static int propagate_liveness(struct bpf_verifier_env *env, > vparent->curframe, vstate->curframe); > return -EFAULT; > } > + > + /* Don't propagate to non-speculative parent. */ > + if (vparent->speculative != vstate->speculative) I haven't thought this trough fully, but is this really necessary? Do we assume the CPU will not speculate twice? It seems not impossible to have a register only accessed on the speculation path, and therefore if we don't propagate liveness non-speculative walk may prune speculative checks. > + return 0; > + > /* Propagate read liveness of registers... */ > BUILD_BUG_ON(BPF_REG_FP + 1 != MAX_BPF_REG); > /* We don't need to worry about FP liveness because it's read-only */
Powered by blists - more mailing lists