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;
> } else if (err < 0) {
> return err;
[...]
---
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
