Messages in this thread | | | From | Jann Horn <> | Date | Fri, 5 Oct 2018 19:48:55 +0200 | Subject | Re: [PATCH] bpf: 32-bit RSH verification must truncate input before the ALU op |
| |
On Fri, Oct 5, 2018 at 7:45 PM Edward Cree <ecree@solarflare.com> wrote: > On 05/10/18 17:17, Jann Horn wrote: > > When I wrote commit 468f6eafa6c4 ("bpf: fix 32-bit ALU op verification"), I > > assumed that, in order to emulate 64-bit arithmetic with 32-bit logic, it > > is sufficient to just truncate the output to 32 bits; and so I just moved > > the register size coercion that used to be at the start of the function to > > the end of the function. > > > > That assumption is true for almost every op, but not for 32-bit right > > shifts, because those can propagate information towards the least > > significant bit. Fix it by always truncating inputs for 32-bit ops to 32 > > bits. > > > > Also get rid of the coerce_reg_to_size() after the ALU op, since that has > > no effect. > Might be worth saying something like "because src_reg is passed by value". > > Fixes: 468f6eafa6c4 ("bpf: fix 32-bit ALU op verification") > > Acked-by: Daniel Borkmann <daniel@iogearbox.net> > > Signed-off-by: Jann Horn <jannh@google.com> > > --- > Acked-by: Edward Cree <ecree@solarflare.com> > > kernel/bpf/verifier.c | 10 +++++++++- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > > index bb07e74b34a2..465952a8e465 100644 > > --- a/kernel/bpf/verifier.c > > +++ b/kernel/bpf/verifier.c > > @@ -2896,6 +2896,15 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env, > > u64 umin_val, umax_val; > > u64 insn_bitness = (BPF_CLASS(insn->code) == BPF_ALU64) ? 64 : 32; > Incidentally, I don't see why this needs to be a u64 (rather than say a u8).
Yeah, the size of the integer doesn't really matter there... but it's being compared against other u64 values further down, so I also don't see a particular need to change it.
| |