Re: [PATCH bpf v4 3/4] bpf: Propagate untrusted pointer state in commuted arithmetic

Eduard Zingerman <[email protected]> Fri, 31 Jul 2026 12:36:55 -0700
Newsgroups org.kernel.vger.bpf,dev.linux.lists.sashiko-reviews
Message-ID <[email protected]>
On Wed, 2026-07-29 at 15:35 +0000, [email protected] 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 <[email protected]>
> 
> 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.