bpf: Fix mask direction swap upon off reg sign change
authorDaniel Borkmann <daniel@iogearbox.net>
Mon, 31 May 2021 18:25:55 +0000 (18:25 +0000)
committerCosmin Tanislav <demonsingur@gmail.com>
Thu, 16 May 2024 07:58:24 +0000 (10:58 +0300)
commit bb01a1bba579b4b1c5566af24d95f1767859771e upstream.

Masking direction as indicated via mask_to_left is considered to be
calculated once and then used to derive pointer limits. Thus, this
needs to be placed into bpf_sanitize_info instead so we can pass it
to sanitize_ptr_alu() call after the pointer move. Piotr noticed a
corner case where the off reg causes masking direction change which
then results in an incorrect final aux->alu_limit.

Fixes: 7fedb63a8307 ("bpf: Tighten speculative pointer arithmetic mask")
Reported-by: Piotr Krysiuk <piotras@gmail.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Reviewed-by: Piotr Krysiuk <piotras@gmail.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
kernel/bpf/verifier.c

index db359b7964423cafab1357c58304292d1f4ddee9..f3f14519708d15d15c14943df50ac2e28102a92c 100644 (file)
@@ -2033,18 +2033,10 @@ enum {
 };
 
 static int retrieve_ptr_limit(const struct bpf_reg_state *ptr_reg,
-                             const struct bpf_reg_state *off_reg,
-                             u32 *alu_limit, u8 opcode)
+                             u32 *alu_limit, bool mask_to_left)
 {
-       bool off_is_neg = off_reg->smin_value < 0;
-       bool mask_to_left = (opcode == BPF_ADD &&  off_is_neg) ||
-                           (opcode == BPF_SUB && !off_is_neg);
        u32 max = 0, ptr_limit = 0;
 
-       if (!tnum_is_const(off_reg->var_off) &&
-           (off_reg->smin_value < 0) != (off_reg->smax_value < 0))
-               return REASON_BOUNDS;
-
        switch (ptr_reg->type) {
        case PTR_TO_STACK:
                /* Offset 0 is out-of-bounds, but acceptable start for the
@@ -2112,6 +2104,7 @@ static bool sanitize_needed(u8 opcode)
 
 struct bpf_sanitize_info {
        struct bpf_insn_aux_data aux;
+       bool mask_to_left;
 };
 
 static int sanitize_ptr_alu(struct bpf_verifier_env *env,
@@ -2143,7 +2136,16 @@ static int sanitize_ptr_alu(struct bpf_verifier_env *env,
        if (vstate->speculative)
                goto do_sim;
 
-       err = retrieve_ptr_limit(ptr_reg, off_reg, &alu_limit, opcode);
+       if (!commit_window) {
+               if (!tnum_is_const(off_reg->var_off) &&
+                   (off_reg->smin_value < 0) != (off_reg->smax_value < 0))
+                       return REASON_BOUNDS;
+
+               info->mask_to_left = (opcode == BPF_ADD &&  off_is_neg) ||
+                                    (opcode == BPF_SUB && !off_is_neg);
+       }
+
+       err = retrieve_ptr_limit(ptr_reg, &alu_limit, info->mask_to_left);
        if (err < 0)
                return err;