bpf: fix mark_all_scalars_precise use in mark_chain_precision

When precision backtracking bails out due to some unsupported sequence
of instructions (e.g., stack access through register other than r10), we
need to mark all SCALAR registers as precise to be safe. Currently,
though, we mark SCALARs precise only starting from the state we detected
unsupported condition, which could be one of the parent states of the
actual current state. This will leave some registers potentially not
marked as precise, even though they should. So make sure we start
marking scalars as precise from current state (env->cur_state).

Further, we don't currently detect a situation when we end up with some
stack slots marked as needing precision, but we ran out of available
states to find the instructions that populate those stack slots. This is
akin the `i >= func->allocated_stack / BPF_REG_SIZE` check and should be
handled similarly by falling back to marking all SCALARs precise. Add
this check when we run out of states.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/r/20230505043317.3629845-8-andrii@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 13bbaa2485fc7bc61a21abe63ff2610cdef551c9..899122832d8edc1eff1dfdf51837ac35a9359e52 100644 (file)
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3806,7 +3806,7 @@ static int __mark_chain_precision(struct bpf_verifier_env *env, int regno)
                                err = backtrack_insn(env, i, bt);
                        }
                        if (err == -ENOTSUPP) {
-                               mark_all_scalars_precise(env, st);
+                               mark_all_scalars_precise(env, env->cur_state);
                                bt_reset(bt);
                                return 0;
                        } else if (err) {
@@ -3868,7 +3868,7 @@ static int __mark_chain_precision(struct bpf_verifier_env *env, int regno)
                                         * fp-8 and it's "unallocated" stack space.
                                         * In such case fallback to conservative.
                                         */
-                                       mark_all_scalars_precise(env, st);
+                                       mark_all_scalars_precise(env, env->cur_state);
                                        bt_reset(bt);
                                        return 0;
                                }
@@ -3896,11 +3896,21 @@ static int __mark_chain_precision(struct bpf_verifier_env *env, int regno)
                }
 
                if (bt_empty(bt))
-                       break;
+                       return 0;
 
                last_idx = st->last_insn_idx;
                first_idx = st->first_insn_idx;
        }
+
+       /* if we still have requested precise regs or slots, we missed
+        * something (e.g., stack access through non-r10 register), so
+        * fallback to marking all precise
+        */
+       if (!bt_empty(bt)) {
+               mark_all_scalars_precise(env, env->cur_state);
+               bt_reset(bt);
+       }
+
        return 0;
 }
 

diff --git a/tools/testing/selftests/bpf/verifier/precise.c b/tools/testing/selftests/bpf/verifier/precise.c
index 77ea018582c5e604329f8ac749632926f8068feb..b8c0aae8e7ec56c19fd171e2e6dde272dcc1d399 100644 (file)
--- a/tools/testing/selftests/bpf/verifier/precise.c
+++ b/tools/testing/selftests/bpf/verifier/precise.c
@@ -159,8 +159,9 @@
        mark_precise: frame0: regs=r4 stack= before 3\
        mark_precise: frame0: regs= stack=-8 before 2\
        mark_precise: frame0: falling back to forcing all scalars precise\
+       force_precise: frame0: forcing r0 to be precise\
        mark_precise: frame0: last_idx 5 first_idx 5\
-       mark_precise: frame0: parent state regs=r0 stack=:",
+       mark_precise: frame0: parent state regs= stack=:",
        .result = VERBOSE_ACCEPT,
        .retval = -1,
 },
@@ -187,10 +188,10 @@
        mark_precise: frame0: falling back to forcing all scalars precise\
        force_precise: frame0: forcing r0 to be precise\
        force_precise: frame0: forcing r0 to be precise\
+       force_precise: frame0: forcing r0 to be precise\
+       force_precise: frame0: forcing r0 to be precise\
        mark_precise: frame0: last_idx 6 first_idx 6\
-       mark_precise: frame0: parent state regs=r0 stack=:\
-       mark_precise: frame0: last_idx 5 first_idx 3\
-       mark_precise: frame0: regs=r0 stack= before 5",
+       mark_precise: frame0: parent state regs= stack=:",
        .result = VERBOSE_ACCEPT,
        .retval = -1,
 },