..

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