* [PATCH bpf] bpf: Fix sanitize_err() register selection for BPF_K
@ 2026-08-03 14:44 Yiyang Chen
2026-08-03 15:25 ` bot+bpf-ci
0 siblings, 1 reply; 3+ messages in thread
From: Yiyang Chen @ 2026-08-03 14:44 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Eduard Zingerman, Kumar Kartikeya Dwivedi
Cc: John Fastabend, Martin KaFai Lau, Song Liu, Yonghong Song,
Jiri Olsa, Emil Tsalapatis, bpf, linux-kernel, Sashiko AI review,
Yiyang Chen
sanitize_err() determines whether the source or destination register holds
the pointer by inspecting regs[insn->src_reg]. For BPF_K instructions,
src_reg is zero because the offset is encoded as an immediate. R0 must
therefore not influence the register reported for REASON_TYPE.
Treat BPF_K as an immediate scalar source and select the destination
register for the diagnostic.
Fixes: a15970d916b3 ("bpf: Simplify sanitize_err() signature")
Reported-by: Sashiko AI review <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/bpf/20260729153536.F3E2E1F000E9@smtp.kernel.org/
Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Yiyang Chen <chenyy23@mails.tsinghua.edu.cn>
---
This follows up on Eduard's request to special-case BPF_K when selecting
the register reported by sanitize_err():
https://lore.kernel.org/bpf/a9475d6a37b2569aaa754c5b2186ba53b37c405d.camel@gmail.com/
---
kernel/bpf/verifier.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index fdc5fbb1f78ca..e5a8547ad7231 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -13562,6 +13562,7 @@ static int sanitize_err(struct bpf_verifier_env *env, const struct bpf_insn *ins
static const char *err = "pointer arithmetic with it prohibited for !root";
const char *op = BPF_OP(insn->code) == BPF_ADD ? "add" : "sub";
u32 dst = insn->dst_reg, src = insn->src_reg;
+ bool src_is_imm = BPF_SRC(insn->code) == BPF_K;
struct bpf_reg_state *regs = cur_regs(env);
switch (reason) {
@@ -13571,7 +13572,7 @@ static int sanitize_err(struct bpf_verifier_env *env, const struct bpf_insn *ins
break;
case REASON_TYPE:
verbose(env, "R%d has pointer with unsupported alu operation, %s\n",
- regs[src].type == SCALAR_VALUE ? dst : src, err);
+ src_is_imm || regs[src].type == SCALAR_VALUE ? dst : src, err);
break;
case REASON_PATHS:
verbose(env, "R%d tried to %s from different maps, paths or scalars, %s\n",
---
base-commit: e5fd3f514e27db1f05fbd72ba615d74941e23c51
change-id: 20260803-c3-035-sanitize-err-v1-3aef2bf838bf
--
2.43.0
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH bpf] bpf: Fix sanitize_err() register selection for BPF_K 2026-08-03 14:44 [PATCH bpf] bpf: Fix sanitize_err() register selection for BPF_K Yiyang Chen @ 2026-08-03 15:25 ` bot+bpf-ci 2026-08-06 17:52 ` Eduard Zingerman 0 siblings, 1 reply; 3+ messages in thread From: bot+bpf-ci @ 2026-08-03 15:25 UTC (permalink / raw) To: chenyy23, ast, daniel, andrii, eddyz87, memxor Cc: john.fastabend, martin.lau, song, yonghong.song, jolsa, emil, bpf, linux-kernel, sashiko-bot, chenyy23, ast, andrii, daniel, martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai [-- Attachment #1: Type: text/plain, Size: 4764 bytes --] > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index fdc5fbb1f78c..e5a8547ad723 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -13562,6 +13562,7 @@ static int sanitize_err(struct bpf_verifier_env *env, const struct bpf_insn *ins > static const char *err = "pointer arithmetic with it prohibited for !root"; > const char *op = BPF_OP(insn->code) == BPF_ADD ? "add" : "sub"; > u32 dst = insn->dst_reg, src = insn->src_reg; > + bool src_is_imm = BPF_SRC(insn->code) == BPF_K; > struct bpf_reg_state *regs = cur_regs(env); > > switch (reason) { > @@ -13571,7 +13572,7 @@ static int sanitize_err(struct bpf_verifier_env *env, const struct bpf_insn *ins > break; > case REASON_TYPE: > verbose(env, "R%d has pointer with unsupported alu operation, %s\n", > - regs[src].type == SCALAR_VALUE ? dst : src, err); > + src_is_imm || regs[src].type == SCALAR_VALUE ? dst : src, err); > break; > case REASON_PATHS: > verbose(env, "R%d tried to %s from different maps, paths or scalars, %s\n", Can this code path be reached for BPF_K instructions? The new src_is_imm check appears to be unreachable. All three call sites of sanitize_err() (at verifier.c:13765, :13858 and :14610) fire only when sanitize_ptr_alu() or sanitize_val_alu() return a negative REASON_* code. Both of those functions begin with: static bool can_skip_alu_sanitation(const struct bpf_verifier_env *env, const struct bpf_insn *insn) { return env->bypass_spec_v1 || BPF_SRC(insn->code) == BPF_K || cur_aux(env)->nospec; } and check this at their entry: sanitize_val_alu() verifier.c:13415: if (can_skip_alu_sanitation(env, insn)) return 0; sanitize_ptr_alu() verifier.c:13469: if (can_skip_alu_sanitation(env, insn)) return 0; Every REASON_* return in both functions is downstream of that gate, so for BPF_K instructions they return 0 early. This means sanitize_err() never executes for BPF_K, making src_is_imm always false inside that function. The change adds a variable and an always-false disjunct to the conditional, but produces identical output for all reachable cases. Note also that the sibling REASON_BOUNDS arm two lines above uses the same regs[src] test and was left untouched. If this is defensive hardening against a future change that removes the BPF_K gate, should that be stated explicitly? Otherwise the code and commit message imply a functional fix when the behavior is unchanged. > commit cd52f137178ae1ebdcb1abceac8ccda767f69fe2 > Author: Yiyang Chen <chenyy23@mails.tsinghua.edu.cn> > > bpf: Fix sanitize_err() register selection for BPF_K > > sanitize_err() determines whether the source or destination register holds > the pointer by inspecting regs[insn->src_reg]. For BPF_K instructions, > src_reg is zero because the offset is encoded as an immediate. R0 must > therefore not influence the register reported for REASON_TYPE. > > Treat BPF_K as an immediate scalar source and select the destination > register for the diagnostic. > > Fixes: a15970d916b3 ("bpf: Simplify sanitize_err() signature") Does the Fixes: tag accurately describe a bug that existed in a15970d916b3? The format of the tag is correct: SHA-1 a15970d916b3 is 12 hex chars, exists and is reachable, and the subject matches exactly. However, the tag asserts a reachable defect that the code analysis above contradicts. The commit a15970d916b3 replaced the off_reg == dst_reg comparison with regs[src].type == SCALAR_VALUE, and its changelog states: "regs[src].type is safe to access, as it is not mutated by the callers." That commit did not introduce a reachable misreport, because sanitize_err() was already unreachable for BPF_K via can_skip_alu_sanitation() both before and after it. The scenario the current commit message describes (a pointer in R0 being selected as the reported register for a BPF_K instruction) cannot occur because sanitize_err() is never called for BPF_K instructions. Per the reachability analysis, the emitted verifier message is byte-for-byte identical before and after this patch for every possible input program. If this patch is intended as hardening for a future relaxation of the BPF_K gate, stating that explicitly and dropping the Fixes: tag would more accurately describe the change. Otherwise the tag attributes a bug that cannot be triggered and may cause this to be backported as a functional fix when it has no runtime effect. --- AI reviewed your patch. Please fix the bug or email reply why it's not a bug. See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md CI run summary: https://github.com/kernel-patches/bpf/actions/runs/30825118624 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH bpf] bpf: Fix sanitize_err() register selection for BPF_K 2026-08-03 15:25 ` bot+bpf-ci @ 2026-08-06 17:52 ` Eduard Zingerman 0 siblings, 0 replies; 3+ messages in thread From: Eduard Zingerman @ 2026-08-06 17:52 UTC (permalink / raw) To: bot+bpf-ci, chenyy23, ast, daniel, andrii, memxor Cc: john.fastabend, martin.lau, song, yonghong.song, jolsa, emil, bpf, linux-kernel, sashiko-bot, martin.lau, clm, ihor.solodrai On Mon, 2026-08-03 at 15:25 +0000, bot+bpf-ci@kernel.org wrote: > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > > index fdc5fbb1f78c..e5a8547ad723 100644 > > --- a/kernel/bpf/verifier.c > > +++ b/kernel/bpf/verifier.c > > @@ -13562,6 +13562,7 @@ static int sanitize_err(struct bpf_verifier_env *env, const struct bpf_insn *ins > > static const char *err = "pointer arithmetic with it prohibited for !root"; > > const char *op = BPF_OP(insn->code) == BPF_ADD ? "add" : "sub"; > > u32 dst = insn->dst_reg, src = insn->src_reg; > > + bool src_is_imm = BPF_SRC(insn->code) == BPF_K; > > struct bpf_reg_state *regs = cur_regs(env); > > > > switch (reason) { > > @@ -13571,7 +13572,7 @@ static int sanitize_err(struct bpf_verifier_env *env, const struct bpf_insn *ins > > break; > > case REASON_TYPE: > > verbose(env, "R%d has pointer with unsupported alu operation, %s\n", > > - regs[src].type == SCALAR_VALUE ? dst : src, err); > > + src_is_imm || regs[src].type == SCALAR_VALUE ? dst : src, err); > > break; > > case REASON_PATHS: > > verbose(env, "R%d tried to %s from different maps, paths or scalars, %s\n", > > Can this code path be reached for BPF_K instructions? The new src_is_imm > check appears to be unreachable. > > All three call sites of sanitize_err() (at verifier.c:13765, :13858 and > :14610) fire only when sanitize_ptr_alu() or sanitize_val_alu() return a > negative REASON_* code. Both of those functions begin with: > > static bool can_skip_alu_sanitation(const struct bpf_verifier_env *env, > const struct bpf_insn *insn) > { > return env->bypass_spec_v1 || > BPF_SRC(insn->code) == BPF_K || > cur_aux(env)->nospec; > } > > and check this at their entry: > > sanitize_val_alu() verifier.c:13415: if (can_skip_alu_sanitation(env, insn)) return 0; > sanitize_ptr_alu() verifier.c:13469: if (can_skip_alu_sanitation(env, insn)) return 0; Hi Yiyang, Thank you for the follow-up. I double-checked what bot is saying, and it correct. The original report from Sashiko was a false positive. Given this, let's keep the code as-is for now. ... ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-08-06 17:52 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-08-03 14:44 [PATCH bpf] bpf: Fix sanitize_err() register selection for BPF_K Yiyang Chen 2026-08-03 15:25 ` bot+bpf-ci 2026-08-06 17:52 ` Eduard Zingerman
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®