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 | dev.linux.lists.sashiko-reviews,org.kernel.vger.bpf |
|---|---|
| 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 issu= e(s) to consider: > - [Medium] sanitize_err prints the wrong register name for immediate oper= ations (BPF_K) when R0 contains a pointer. > -- >=20 > --- Patch [3]: [PATCH bpf v4 3/4] bpf: Propagate untrusted pointer state = in commuted arithmetic --- > commit 74ae77e327bede792130d0bf76a485d3ad9ef6b4 > Author: Yiyang Chen <[email protected]> >=20 > bpf: Propagate untrusted pointer state in commuted arithmetic >=20 > 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 += =3D > untrusted_pointer correctly leave the destination register as a PTR_TO_ME= M. >=20 > > 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; > > } > > =20 > > - /* > > - * Accesses to untrusted PTR_TO_MEM are done through probe > > - * instructions, hence no need to track offsets. > > - */ > > - if (base_type(ptr_reg->type) =3D=3D PTR_TO_MEM && (ptr_reg->type & PT= R_UNTRUSTED)) > > - return 0; > > - > > switch (base_type(ptr_reg->type)) { > > case PTR_TO_CTX: > > case PTR_TO_MAP_VALUE: >=20 > [ ... ] >=20 > > @@ -13750,6 +13743,13 @@ static int adjust_ptr_min_max_vals(struct bpf_= verifier_env *env, > > if (dst_reg !=3D ptr_reg) > > *dst_reg =3D *ptr_reg; > > =20 > > + /* > > + * Accesses to untrusted PTR_TO_MEM are done through probe > > + * instructions, hence no need to track offsets. > > + */ > > + if (base_type(ptr_reg->type) =3D=3D PTR_TO_MEM && (ptr_reg->type & PT= R_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; >=20 > [Severity: Medium] > Does sanitize_err() print the wrong register name for immediate operation= s > (BPF_K) when R0 contains a pointer? >=20 > When an immediate pointer arithmetic operation is rejected, sanitize_err(= ) > is called downstream of this offset checking logic: >=20 > kernel/bpf/verifier.c:sanitize_err() { > ... > u32 dst =3D insn->dst_reg, src =3D insn->src_reg; > ... > case REASON_TYPE: > verbose(env, "R%d has pointer with unsupported alu operation, %s\= n", > regs[src].type =3D=3D SCALAR_VALUE ? dst : src, err); > } >=20 > For BPF_K operations, insn->src_reg is 0. If a pointer was previously pla= ced > 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 th= e > 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.