On Sat, 2025-06-28 at 16:50 +0200, Luis Gerhorst wrote:
[...]
@@ -19955,11 +19960,11 @@ static int do_check(struct bpf_verifier_env *env) /* Prevent this speculative path from ever reaching the * insn that would have been unsafe to execute. */
cur_aux(env)->nospec = true;
prev_aux(env)->nospec = true;
I don't like the prev_aux() call in this position, as one needs to understand that after do_check_insn() call what was current became previous. This at-least requires a comment. Implementation with a temporary variable (as at the bottom of this email), imo, is less cognitive load.
/* IF it was an ADD/SUB insn, potentially remove any * markings for alu sanitization. */
cur_aux(env)->alu_state = 0;
prev_aux(env)->alu_state = 0; goto process_bpf_exit;
[...]
---
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index a136d9b1b25f..a923614b7104 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -19953,6 +19953,7 @@ static int do_check(struct bpf_verifier_env *env) bool pop_log = !(env->log.level & BPF_LOG_LEVEL2); struct bpf_verifier_state *state = env->cur_state; struct bpf_insn *insns = env->prog->insnsi; + struct bpf_insn_aux_data *insn_aux; int insn_cnt = env->prog->len; bool do_print_state = false; int prev_insn_idx = -1; @@ -19972,6 +19973,7 @@ static int do_check(struct bpf_verifier_env *env) }
insn = &insns[env->insn_idx]; + insn_aux = &env->insn_aux_data[env->insn_idx];
if (++env->insn_processed > BPF_COMPLEXITY_LIMIT_INSNS) { verbose(env, @@ -20048,7 +20050,7 @@ static int do_check(struct bpf_verifier_env *env) /* Reduce verification complexity by stopping speculative path * verification when a nospec is encountered. */ - if (state->speculative && cur_aux(env)->nospec) + if (state->speculative && insn_aux->nospec) goto process_bpf_exit;
err = do_check_insn(env, &do_print_state); @@ -20056,11 +20058,11 @@ static int do_check(struct bpf_verifier_env *env) /* Prevent this speculative path from ever reaching the * insn that would have been unsafe to execute. */ - cur_aux(env)->nospec = true; + insn_aux->nospec = true; /* If it was an ADD/SUB insn, potentially remove any * markings for alu sanitization. */ - cur_aux(env)->alu_state = 0; + insn_aux->alu_state = 0; goto process_bpf_exit; } else if (err < 0) { return err; @@ -20069,7 +20071,7 @@ static int do_check(struct bpf_verifier_env *env) } WARN_ON_ONCE(err);
- if (state->speculative && cur_aux(env)->nospec_result) { + if (state->speculative && insn_aux->nospec_result) { /* If we are on a path that performed a jump-op, this * may skip a nospec patched-in after the jump. This can * currently never happen because nospec_result is only