Skip to content

Commit 107c26a

Browse files
rdnaborkmann
authored andcommitted
bpf: Sanity check max value for var_off stack access
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: 2011fcc ("bpf: Support variable offset stack access from helpers") Reported-by: Daniel Borkmann <[email protected]> Signed-off-by: Andrey Ignatov <[email protected]> Signed-off-by: Daniel Borkmann <[email protected]>
1 parent 2c6927d commit 107c26a

File tree

1 file changed

+15
-3
lines changed

1 file changed

+15
-3
lines changed

kernel/bpf/verifier.c

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2248,16 +2248,28 @@ static int check_stack_boundary(struct bpf_verifier_env *env, int regno,
22482248
if (meta && meta->raw_mode)
22492249
meta = NULL;
22502250

2251+
if (reg->smax_value >= BPF_MAX_VAR_OFF ||
2252+
reg->smax_value <= -BPF_MAX_VAR_OFF) {
2253+
verbose(env, "R%d unbounded indirect variable offset stack access\n",
2254+
regno);
2255+
return -EACCES;
2256+
}
22512257
min_off = reg->smin_value + reg->off;
2252-
max_off = reg->umax_value + reg->off;
2258+
max_off = reg->smax_value + reg->off;
22532259
err = __check_stack_boundary(env, regno, min_off, access_size,
22542260
zero_size_allowed);
2255-
if (err)
2261+
if (err) {
2262+
verbose(env, "R%d min value is outside of stack bound\n",
2263+
regno);
22562264
return err;
2265+
}
22572266
err = __check_stack_boundary(env, regno, max_off, access_size,
22582267
zero_size_allowed);
2259-
if (err)
2268+
if (err) {
2269+
verbose(env, "R%d max value is outside of stack bound\n",
2270+
regno);
22602271
return err;
2272+
}
22612273
}
22622274

22632275
if (meta && meta->raw_mode) {

0 commit comments

Comments
 (0)