Message ID | 20210706182635.37501-1-colin.king@canonical.com |
---|---|
State | New |
Headers | show |
Series | [bpf,v2,1/2] bpf, x86: Validate computation of branch displacements for x86-64 | expand |
Igore, I fat fingered a git send. Colin On 06/07/2021 19:26, Colin King wrote: > From: Piotr Krysiuk <piotras@gmail.com> > > The branch displacement logic in the BPF JIT compilers for x86 assumes > that, for any generated branch instruction, the distance cannot > increase between optimization passes. > > But this assumption can be violated due to how the distances are > computed. Specifically, whenever a backward branch is processed in > do_jit(), the distance is computed by subtracting the positions in the > machine code from different optimization passes. This is because part > of addrs[] is already updated for the current optimization pass, before > the branch instruction is visited. > > And so the optimizer can expand blocks of machine code in some cases. > > This can confuse the optimizer logic, where it assumes that a fixed > point has been reached for all machine code blocks once the total > program size stops changing. And then the JIT compiler can output > abnormal machine code containing incorrect branch displacements. > > To mitigate this issue, we assert that a fixed point is reached while > populating the output image. This rejects any problematic programs. > > The issue affects both x86-32 and x86-64. We mitigate separately to > ease backporting. > > Signed-off-by: Piotr Krysiuk <piotras@gmail.com> > --- > arch/x86/net/bpf_jit_comp.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c > index 023ac12f54a2..f104c36f1e79 100644 > --- a/arch/x86/net/bpf_jit_comp.c > +++ b/arch/x86/net/bpf_jit_comp.c > @@ -1476,7 +1476,15 @@ xadd: if (is_imm8(insn->off)) > } > > if (image) { > - if (unlikely(proglen + ilen > oldproglen)) { > + /* > + * When populating the image, assert that > + * i) we do not write beyond the allocated space, and > + * ii) addrs[i] did not change from the prior run, in order > + * to validate assumptions made for computing branch > + * displacements > + */ > + if (unlikely(proglen + ilen > oldproglen || > + proglen + ilen != addrs[i])) { > pr_err("bpf_jit: fatal error\n"); > return -EFAULT; > } >
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c index 023ac12f54a2..f104c36f1e79 100644 --- a/arch/x86/net/bpf_jit_comp.c +++ b/arch/x86/net/bpf_jit_comp.c @@ -1476,7 +1476,15 @@ xadd: if (is_imm8(insn->off)) } if (image) { - if (unlikely(proglen + ilen > oldproglen)) { + /* + * When populating the image, assert that + * i) we do not write beyond the allocated space, and + * ii) addrs[i] did not change from the prior run, in order + * to validate assumptions made for computing branch + * displacements + */ + if (unlikely(proglen + ilen > oldproglen || + proglen + ilen != addrs[i])) { pr_err("bpf_jit: fatal error\n"); return -EFAULT; }