mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Luis Gerhorst <luis.gerhorst@fau.de>
To: Andrii Nakryiko <andrii@kernel.org>,
	Eduard Zingerman <eddyz87@gmail.com>,
	Mykola Lysenko <mykolal@fb.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Martin KaFai Lau <martin.lau@linux.dev>,
	Song Liu <song@kernel.org>,
	Yonghong Song <yonghong.song@linux.dev>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@kernel.org>,
	Stanislav Fomichev <sdf@fomichev.me>, Hao Luo <haoluo@google.com>,
	Jiri Olsa <jolsa@kernel.org>, Shuah Khan <shuah@kernel.org>,
	Kumar Kartikeya Dwivedi <memxor@gmail.com>,
	Luis Gerhorst <luis.gerhorst@fau.de>,
	Peilin Ye <yepeilin@google.com>, Jiayuan Chen <mrpre@163.com>,
	Saket Kumar Bhaskar <skb99@linux.ibm.com>,
	Ihor Solodrai <isolodrai@meta.com>, Daniel Xu <dxu@dxuuu.xyz>,
	bpf@vger.kernel.org, linux-kselftest@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Paul Chaignon <paul.chaignon@gmail.com>
Subject: [PATCH bpf-next v2 1/3] bpf: Update env->prev_insn_idx after do_check_insn()
Date: Sat, 28 Jun 2025 16:50:14 +0200	[thread overview]
Message-ID: <20250628145016.784256-2-luis.gerhorst@fau.de> (raw)
In-Reply-To: <20250628145016.784256-1-luis.gerhorst@fau.de>

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@fau.de>
---
 kernel/bpf/verifier.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index f403524bd215..b33bc37d5372 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.
@@ -19978,13 +19978,13 @@ static int do_check(struct bpf_verifier_env *env)
 			 * to document this in case nospec_result is used
 			 * elsewhere in the future.
 			 */
-			WARN_ON_ONCE(env->insn_idx != prev_insn_idx + 1);
+			WARN_ON_ONCE(env->insn_idx != env->prev_insn_idx + 1);
 process_bpf_exit:
 			mark_verifier_state_scratched(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


  reply	other threads:[~2025-06-28 14:54 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-28 14:50 [PATCH bpf-next v2 0/3] bpf: Fix and test aux usage " Luis Gerhorst
2025-06-28 14:50 ` Luis Gerhorst [this message]
2025-06-28 14:50 ` [PATCH bpf-next v2 2/3] bpf: Fix " Luis Gerhorst
2025-06-30  1:05   ` Eduard Zingerman
2025-07-02  8:56     ` Luis Gerhorst
2025-06-28 14:50 ` [PATCH bpf-next v2 3/3] selftests/bpf: Add Spectre v4 tests Luis Gerhorst

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250628145016.784256-2-luis.gerhorst@fau.de \
    --to=luis.gerhorst@fau.de \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=dxu@dxuuu.xyz \
    --cc=eddyz87@gmail.com \
    --cc=haoluo@google.com \
    --cc=isolodrai@meta.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=memxor@gmail.com \
    --cc=mrpre@163.com \
    --cc=mykolal@fb.com \
    --cc=paul.chaignon@gmail.com \
    --cc=sdf@fomichev.me \
    --cc=shuah@kernel.org \
    --cc=skb99@linux.ibm.com \
    --cc=song@kernel.org \
    --cc=yepeilin@google.com \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®