linux-kernel - [PATCH bpf-next 1/3] bpf: Update env->prev_insn_idx after do_check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250628125927.763088-2-luis.gerhorst@fau.de>
Date: Sat, 28 Jun 2025 14:59:25 +0200
From: Luis Gerhorst <luis.gerhorst@....de>
To: Andrii Nakryiko <andrii@...nel.org>,
	Eduard Zingerman <eddyz87@...il.com>,
	Mykola Lysenko <mykolal@...com>,
	Alexei Starovoitov <ast@...nel.org>,
	Daniel Borkmann <daniel@...earbox.net>,
	Martin KaFai Lau <martin.lau@...ux.dev>,
	Song Liu <song@...nel.org>,
	Yonghong Song <yonghong.song@...ux.dev>,
	John Fastabend <john.fastabend@...il.com>,
	KP Singh <kpsingh@...nel.org>,
	Stanislav Fomichev <sdf@...ichev.me>,
	Hao Luo <haoluo@...gle.com>,
	Jiri Olsa <jolsa@...nel.org>,
	Shuah Khan <shuah@...nel.org>,
	Kumar Kartikeya Dwivedi <memxor@...il.com>,
	Luis Gerhorst <luis.gerhorst@....de>,
	Peilin Ye <yepeilin@...gle.com>,
	Jiayuan Chen <mrpre@....com>,
	Saket Kumar Bhaskar <skb99@...ux.ibm.com>,
	Ihor Solodrai <isolodrai@...a.com>,
	Daniel Xu <dxu@...uu.xyz>,
	bpf@...r.kernel.org,
	linux-kselftest@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Paul Chaignon <paul.chaignon@...il.com>
Subject: [PATCH bpf-next 1/3] bpf: Update env->prev_insn_idx after do_check_insn()

To introduce prev_aux(env), env->prev_insn_idx must be up-to-date
directly after do_check_insn(). To achieve this, replace prev_insn_idx
with a tmp variable (to discourage use) and update env->prev_insn_idx
directly after do_check_insn().

A concern would be that some code relied on env->prev_insn_idx still
having the old value between do_check_insn() and the old
update-assignment. However, outside the do_check() function it is only
used through push_jmp_history()/do_check_insn()/is_state_visisted()
which are not called in-between the old and new assignment location.
This can also be seen from the -O0 call graph for push_jmp_history()
[1].

[1] https://sys.cs.fau.de/extern/person/gerhorst/25-06_d69baf_push_jmp_history_O0_callgraph.png

Signed-off-by: Luis Gerhorst <luis.gerhorst@....de>
---
 kernel/bpf/verifier.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index f403524bd215..3b24055117bc 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -19854,16 +19854,15 @@ static int do_check(struct bpf_verifier_env *env)
 	struct bpf_insn *insns = env->prog->insnsi;
 	int insn_cnt = env->prog->len;
 	bool do_print_state = false;
-	int prev_insn_idx = -1;
 
+	env->prev_insn_idx = -1;
 	for (;;) {
 		struct bpf_insn *insn;
-		int err;
+		int err, tmp;
 
 		/* reset current history entry on each new instruction */
 		env->cur_hist_ent = NULL;
 
-		env->prev_insn_idx = prev_insn_idx;
 		if (env->insn_idx >= insn_cnt) {
 			verbose(env, "invalid insn idx %d insn_cnt %d\n",
 				env->insn_idx, insn_cnt);
@@ -19942,7 +19941,6 @@ static int do_check(struct bpf_verifier_env *env)
 		}
 
 		sanitize_mark_insn_seen(env);
-		prev_insn_idx = env->insn_idx;
 
 		/* Reduce verification complexity by stopping speculative path
 		 * verification when a nospec is encountered.
@@ -19950,7 +19948,9 @@ static int do_check(struct bpf_verifier_env *env)
 		if (state->speculative && cur_aux(env)->nospec)
 			goto process_bpf_exit;
 
+		tmp = env->insn_idx;
 		err = do_check_insn(env, &do_print_state);
+		env->prev_insn_idx = tmp;
 		if (error_recoverable_with_nospec(err) && state->speculative) {
 			/* Prevent this speculative path from ever reaching the
 			 * insn that would have been unsafe to execute.
@@ -19984,7 +19984,7 @@ static int do_check(struct bpf_verifier_env *env)
 			err = update_branch_counts(env, env->cur_state);
 			if (err)
 				return err;
-			err = pop_stack(env, &prev_insn_idx, &env->insn_idx,
+			err = pop_stack(env, &env->prev_insn_idx, &env->insn_idx,
 					pop_log);
 			if (err < 0) {
 				if (err != -ENOENT)
-- 
2.49.0