..
eBPF verifier bug: Incorrect bounds update when BPF_JNE{EQ}
--[ Description
In check_cond_jmp_op(),when `dst_reg->type == SCALAR_VALUE` it will
call reg_set_min_max() to update the bounds.
```
__attribute__((optimize("O0"))) void reg_set_min_max(struct
bpf_reg_state *true_reg,
struct bpf_reg_state *false_reg,
u64 val, u32 val32,
u8 opcode, bool is_jmp32)
{
struct tnum false_32off = tnum_subreg(false_reg->var_off);
struct tnum false_64off = false_reg->var_off;
struct tnum true_32off = tnum_subreg(true_reg->var_off);
struct tnum true_64off = true_reg->var_off;
s64 sval = (s64)val;
s32 sval32 = (s32)val32;
if (__is_pointer_value(false, false_reg))
return;
switch (opcode) {
case BPF_JEQ:
case BPF_JNE:
{
struct bpf_reg_state *reg =
opcode == BPF_JEQ ? true_reg : false_reg;
if (is_jmp32)
__mark_reg32_known(reg, val32);
else
___mark_reg_known(reg, val);
break;
}
...
default:
return;
}
if (is_jmp32) {
false_reg->var_off = tnum_or(tnum_clear_subreg(false_64off),
tnum_subreg(false_32off));
true_reg->var_off = tnum_or(tnum_clear_subreg(true_64off),
tnum_subreg(true_32off));
__reg_combine_32_into_64(false_reg);
__reg_combine_32_into_64(true_reg);
} else {
false_reg->var_off = false_64off;
true_reg->var_off = true_64off;
__reg_combine_64_into_32(false_reg);
__reg_combine_64_into_32(true_reg);
}
}
```
Note that when opcode == BPF_JNE{EQ}, it will mark dst_reg as known
const and then break the switch, finally, it will execute some generic
bounds update code.
The problem is it only marks false{true}_reg as known in the switch,
but at the end of the function, it also uses false{true}_64{32}off to
update the bounds, which is the old value of false{true}_reg->var_off
and finally results in incorrect bounds.
-- [ Affected Versions
The latest version of Linux.
-- [ Reproducer
With these instructions:
```
BPF_MOV64_IMM(BPF_REG_2, 0),
BPF_MOV64_IMM(BPF_REG_6, 0x233),
BPF_ALU64_IMM(BPF_NEG, BPF_REG_2, 0),
BPF_ALU64_IMM(BPF_NEG, BPF_REG_2, 0),
BPF_ALU32_REG(BPF_OR, BPF_REG_2, BPF_REG_6),
BPF_JMP32_IMM(BPF_JNE, BPF_REG_2, 8, 1),
BPF_EXIT_INSN(),
BPF_EXIT_INSN()
```
And check the log:
```
func#0 @0
0: R1=ctx(off=0,imm=0) R10=fp0
0: (b7) r2 = 0 ; R2_w=0
1: (b7) r6 = 563 ; R6_w=563
2: (87) r2 = -r2 ; R2_w=scalar()
3: (87) r2 = -r2 ; R2_w=scalar()
4: (4c) w2 |= w6 ;
R2_w=scalar(umin=563,umax=4294967295,var_off=(0x233;
0xfffffdcc),s32_min=-2147483085) R6_w=563
5: (56) if w2 != 0x8 goto pc+1 ; R2_w=571 // < --- [1]
6: (95) exit
R0 !read_ok
processed 7 insns (limit 1000000) max_states_per_insn 0 total_states 0
peak_states 0 mark_read 0
```
Note that the annotate in [1] will show the state of false_reg, which
should be const 8 there, but have
`smin_val=571;smax_val=8;umin_val=571;umax_val=8;s32_min_val=571;s32_max_val=8;u32_min_val=571;u32_max_val=8;tnum_value=571;tnum_mask=0;`
-- [ Mitigation
Update false{true}_64{32}off, or just return after __mark_reg{32}_known().
-- [ Exploitability
The fallthrough path will never execute, so it canno be exploited.
-- [ References
* https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=a49b8ce7306c
* https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=a12ca6277eca