bpf: Sanity check max value for var_off stack access
commit 107c26a70ca81bfc33657366ad69d02fdc9efc9d upstream. As discussed in [1] max value of variable offset has to be checked for overflow on stack access otherwise verifier would accept code like this: 0: (b7) r2 = 6 1: (b7) r3 = 28 2: (7a) *(u64 *)(r10 -16) = 0 3: (7a) *(u64 *)(r10 -8) = 0 4: (79) r4 = *(u64 *)(r1 +168) 5: (c5) if r4 s< 0x0 goto pc+4 R1=ctx(id=0,off=0,imm=0) R2=inv6 R3=inv28 R4=inv(id=0,umax_value=9223372036854775807,var_off=(0x0; 0x7fffffffffffffff)) R10=fp0,call_-1 fp-8=mmmmmmmm fp-16=mmmmmmmm 6: (17) r4 -= 16 7: (0f) r4 += r10 8: (b7) r5 = 8 9: (85) call bpf_getsockopt#57 10: (b7) r0 = 0 11: (95) exit , where R4 obviosly has unbounded max value. Fix it by checking that reg->smax_value is inside (-BPF_MAX_VAR_OFF; BPF_MAX_VAR_OFF) range. reg->smax_value is used instead of reg->umax_value because stack pointers are calculated using negative offset from fp. This is opposite to e.g. map access where offset must be non-negative and where umax_value is used. Also dedicated verbose logs are added for both min and max bound check failures to have diagnostics consistent with variable offset handling in check_map_access(). [1] https://marc.info/?l=linux-netdev&m=155433357510597&w=2 Fixes: 2011fccfb61b ("bpf: Support variable offset stack access from helpers") Reported-by: Daniel Borkmann <daniel@iogearbox.net> Signed-off-by: Andrey Ignatov <rdna@fb.com> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
committed by
Greg Kroah-Hartman
parent
14cf676ba6
commit
7667818ef1
@@ -1833,17 +1833,29 @@ static int check_stack_boundary(struct bpf_verifier_env *env, int regno,
|
|||||||
if (meta && meta->raw_mode)
|
if (meta && meta->raw_mode)
|
||||||
meta = NULL;
|
meta = NULL;
|
||||||
|
|
||||||
|
if (reg->smax_value >= BPF_MAX_VAR_OFF ||
|
||||||
|
reg->smax_value <= -BPF_MAX_VAR_OFF) {
|
||||||
|
verbose(env, "R%d unbounded indirect variable offset stack access\n",
|
||||||
|
regno);
|
||||||
|
return -EACCES;
|
||||||
|
}
|
||||||
min_off = reg->smin_value + reg->off;
|
min_off = reg->smin_value + reg->off;
|
||||||
max_off = reg->umax_value + reg->off;
|
max_off = reg->smax_value + reg->off;
|
||||||
err = __check_stack_boundary(env, regno, min_off, access_size,
|
err = __check_stack_boundary(env, regno, min_off, access_size,
|
||||||
zero_size_allowed);
|
zero_size_allowed);
|
||||||
if (err)
|
if (err) {
|
||||||
|
verbose(env, "R%d min value is outside of stack bound\n",
|
||||||
|
regno);
|
||||||
return err;
|
return err;
|
||||||
|
}
|
||||||
err = __check_stack_boundary(env, regno, max_off, access_size,
|
err = __check_stack_boundary(env, regno, max_off, access_size,
|
||||||
zero_size_allowed);
|
zero_size_allowed);
|
||||||
if (err)
|
if (err) {
|
||||||
|
verbose(env, "R%d max value is outside of stack bound\n",
|
||||||
|
regno);
|
||||||
return err;
|
return err;
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if (meta && meta->raw_mode) {
|
if (meta && meta->raw_mode) {
|
||||||
meta->access_size = access_size;
|
meta->access_size = access_size;
|
||||||
|
|||||||
Reference in New Issue
Block a user