From: Alexei Starovoitov 2: r8 &= 60 3: r8 *= 7 4: if r8 >= 44 goto pc+2 5: if r8 < 34 goto pc+1 6: r8 += 5 verifier bug: REG INVARIANTS VIOLATION (alu): range bounds violation 7 * (x & 60) is a multiple of 28 and [34, 43] has none: the fall-through branch of insn 5 is dead. The prog is safe, the verifier has no bug here, and a kernel with CONFIG_DEBUG_KERNEL prints a WARN_ONCE(): a panic with panic_on_warn=1. The verifier gets to insn 6 with r8 in [35, 42], var_off=(0x20; 0xc) and step=0+7. var_off leaves 36 and 40, the congruence leaves 35 and 42. The register has no value, but no part of it is empty. After the addition reg_bounds_sync() cuts the range by var_off to [41, 45], where no value is 5 modulo 7: now r64 is empty. var_off, r64, r32 and the congruence are a product that reg_bounds_sync() does not reduce to the end. reg_is_empty(), which finds dead branches, compares var_off with r64 and with r32. So the verifier takes infeasible paths with registers that have no value. That is imprecise, not wrong: no execution gets there. Commit 5f99f312bd3b ("bpf: add register bounds sanity checks and sanitization") added the check: a line in the log, the ranges are forgotten, -EFAULT with the test flag. In that form it showed bugs: 92424801261d ("bpf: Fix reg_set_min_max corruption of fake_reg") 44b7f7151dfc ("bpf: Add missed var_off setting in coerce_subreg_to_size_sx()") Commit 0df1a55afa83 ("bpf: Warn on internal verifier errors") made it a verifier_bug(). Fixes for it since then: 6279846b9b25 ("bpf: Forget ranges when refining tnum after JSET") d43ad9da8052 ("bpf: Skip bounds adjustment for conditional jumps on same scalar register") efc11a667878 ("bpf: Improve bounds when tnum has a single possible value") b254c6d816e5 ("bpf: Simulate branches to prune based on range violations"), its log lists three more 150aeba624e8 ("bpf: Fix REG INVARIANTS VIOLATION on speculative pointer arithmetic") 84d326943649 ("bpf: Detect a dead branch when a range and var_off don't overlap") Five of the six are about a dead branch, one is about a speculative path. A new part of the product can add such states again: the listing above needs the congruence, which is newer than b254c6d816e5. WARN*() is not for what user space can trigger: "Do not WARN lightly" in Documentation/process/coding-style.rst. Print the line with verbose(), as before 0df1a55afa83, without "verifier bug: ". The rest stays: the ranges are forgotten, and a prog that is loaded with BPF_F_TEST_REG_INVARIANTS fails with -EFAULT: tests with that __flag(), veristat -r, a fuzzer. 'invalid base/step' keeps verifier_bug(): step == 0 or base >= step is a malformed congruence, not an empty register. Fixes: 0df1a55afa83 ("bpf: Warn on internal verifier errors") Signed-off-by: Alexei Starovoitov --- kernel/bpf/verifier.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index c9c6b1367a0b..5a30ce76da73 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -2355,6 +2355,12 @@ static bool reg_is_empty(struct bpf_reg_state *reg) U32_MAX); } +/* + * var_off, r64, r32 and the line base + step * k are a product that reg_bounds_sync() doesn't + * reduce to the end. The verifier can take an infeasible path with a register that has no value + * while each part looks sane, and a later insn finds it out. That is not a verifier bug: no WARN, + * a line in the log, the ranges are forgotten. A malformed base or step is a bug. + */ static int reg_bounds_sanity_check(struct bpf_verifier_env *env, struct bpf_reg_state *reg, const char *ctx) { @@ -2375,20 +2381,20 @@ static int reg_bounds_sanity_check(struct bpf_verifier_env *env, goto out; } - if (reg->step == 0 || reg->base >= reg->step) { + if (verifier_bug_if(reg->step == 0 || reg->base >= reg->step, env, "%s", ctx)) { msg = "invalid base/step"; goto out; } return 0; out: - verifier_bug(env, "REG INVARIANTS VIOLATION (%s): %s r64={.base=%#llx, .size=%#llx} " - "r32={.base=%#x, .size=%#x} var_off=(%#llx, %#llx) step=%u+%u", - ctx, msg, - reg->r64.base, reg->r64.size, - reg->r32.base, reg->r32.size, - reg->var_off.value, reg->var_off.mask, - reg->base, reg->step); + verbose(env, "REG INVARIANTS VIOLATION (%s): %s r64={.base=%#llx, .size=%#llx} " + "r32={.base=%#x, .size=%#x} var_off=(%#llx, %#llx) step=%u+%u\n", + ctx, msg, + reg->r64.base, reg->r64.size, + reg->r32.base, reg->r32.size, + reg->var_off.value, reg->var_off.mask, + reg->base, reg->step); if (env->test_reg_invariants) return -EFAULT; __mark_reg_unbounded(reg); -- 2.55.0