From: Eduard Zingerman <eddyz87@gmail.com>
To: sashiko-reviews@lists.linux.dev,
Yiyang Chen <chenyy23@mails.tsinghua.edu.cn>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH bpf v4 3/4] bpf: Propagate untrusted pointer state in commuted arithmetic
Date: Fri, 31 Jul 2026 12:36:55 -0700 [thread overview]
Message-ID: <a9475d6a37b2569aaa754c5b2186ba53b37c405d.camel@gmail.com> (raw)
In-Reply-To: <20260729153536.F3E2E1F000E9@smtp.kernel.org>
On Wed, 2026-07-29 at 15:35 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] sanitize_err prints the wrong register name for immediate operations (BPF_K) when R0 contains a pointer.
> --
>
> --- Patch [3]: [PATCH bpf v4 3/4] bpf: Propagate untrusted pointer state in commuted arithmetic ---
> commit 74ae77e327bede792130d0bf76a485d3ad9ef6b4
> Author: Yiyang Chen <chenyy23@mails.tsinghua.edu.cn>
>
> bpf: Propagate untrusted pointer state in commuted arithmetic
>
> This patch modifies the BPF verifier's pointer arithmetic logic by moving
> the early return for untrusted PTR_TO_MEM accesses. Placing it after the
> pointer state is fully propagated ensures that operations like scalar +=
> untrusted_pointer correctly leave the destination register as a PTR_TO_MEM.
>
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index cdb61fab84351..fdc5fbb1f78ca 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -13707,13 +13707,6 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
> > return -EACCES;
> > }
> >
> > - /*
> > - * Accesses to untrusted PTR_TO_MEM are done through probe
> > - * instructions, hence no need to track offsets.
> > - */
> > - if (base_type(ptr_reg->type) == PTR_TO_MEM && (ptr_reg->type & PTR_UNTRUSTED))
> > - return 0;
> > -
> > switch (base_type(ptr_reg->type)) {
> > case PTR_TO_CTX:
> > case PTR_TO_MAP_VALUE:
>
> [ ... ]
>
> > @@ -13750,6 +13743,13 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
> > if (dst_reg != ptr_reg)
> > *dst_reg = *ptr_reg;
> >
> > + /*
> > + * Accesses to untrusted PTR_TO_MEM are done through probe
> > + * instructions, hence no need to track offsets.
> > + */
> > + if (base_type(ptr_reg->type) == PTR_TO_MEM && (ptr_reg->type & PTR_UNTRUSTED))
> > + return 0;
> > +
> > if (!check_reg_sane_offset_scalar(env, off_reg, ptr_reg->type) ||
> > !check_reg_sane_offset_ptr(env, ptr_reg, ptr_reg->type))
> > return -EINVAL;
>
> [Severity: Medium]
> Does sanitize_err() print the wrong register name for immediate operations
> (BPF_K) when R0 contains a pointer?
>
> When an immediate pointer arithmetic operation is rejected, sanitize_err()
> is called downstream of this offset checking logic:
>
> kernel/bpf/verifier.c:sanitize_err() {
> ...
> u32 dst = insn->dst_reg, src = insn->src_reg;
> ...
> case REASON_TYPE:
> verbose(env, "R%d has pointer with unsupported alu operation, %s\n",
> regs[src].type == SCALAR_VALUE ? dst : src, err);
> }
>
> For BPF_K operations, insn->src_reg is 0. If a pointer was previously placed
> in R0, or if R0 is uninitialized, regs[0].type will not be SCALAR_VALUE.
> The ternary operation will then incorrectly choose src (R0) instead of the
> destination register, emitting a confusing diagnostic message.
This seem to be a valid point.
Yiyang, could you please special-case BPF_K in the patch-set #1?
Let's do it as a follow-up.
next prev parent reply other threads:[~2026-07-31 19:36 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-29 15:18 [PATCH bpf v4 0/4] bpf: Preserve pointer state for commuted arithmetic Yiyang Chen via B4 Relay
2026-07-29 15:18 ` [PATCH bpf v4 1/4] bpf: simplify sanitize_err() signature Yiyang Chen via B4 Relay
2026-07-30 8:22 ` Shung-Hsi Yu
2026-07-29 15:18 ` [PATCH bpf v4 2/4] bpf: Preserve pointer state for commuted arithmetic Yiyang Chen via B4 Relay
2026-07-30 8:25 ` Shung-Hsi Yu
2026-07-29 15:18 ` [PATCH bpf v4 3/4] bpf: Propagate untrusted pointer state in " Yiyang Chen via B4 Relay
2026-07-29 15:35 ` sashiko-bot
2026-07-31 19:36 ` Eduard Zingerman [this message]
2026-07-29 15:18 ` [PATCH bpf v4 4/4] selftests/bpf: Cover commuted pointer state propagation Yiyang Chen via B4 Relay
2026-07-31 19:50 ` [PATCH bpf v4 0/4] bpf: Preserve pointer state for commuted arithmetic patchwork-bot+netdevbpf
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=a9475d6a37b2569aaa754c5b2186ba53b37c405d.camel@gmail.com \
--to=eddyz87@gmail.com \
--cc=bpf@vger.kernel.org \
--cc=chenyy23@mails.tsinghua.edu.cn \
--cc=sashiko-reviews@lists.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