The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [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 related	[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