Re: [PATCH 1/2] bpf: Preserve stack frame number for commuted arithmetic

[Shung-Hsi Yu <shung-hsi.yu@suse.com>]

On Tue, Jul 21, 2026 at 09:48:28AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] The patch completely breaks the security check that prohibits subtracting a pointer from a scalar (`scalar -= pointer`), allowing unprivileged eBPF programs to bypass memory isolation and achieve arbitrary kernel memory read/write.
> --
> 
> --- Patch [1]: [PATCH 1/2] bpf: Preserve stack frame number for commuted arithmetic ---
> commit 3b8c2832d6eb05fc6ef4d1f2af9ee96ccdf5566e
> Author: Yiyang Chen <chenyy23@mails.tsinghua.edu.cn>
> 
> bpf: Preserve stack frame number for commuted arithmetic
> 
> This patch modifies how commuted pointer arithmetic is handled in
> adjust_reg_min_max_vals(). It uses a temporary off_reg to copy the
> destination register before calling adjust_ptr_min_max_vals().
> 
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 52be0a118cce0..3c0af83db4672 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> 
> [ ... ]
> 
> > @@ -14854,15 +14855,18 @@ static int adjust_reg_min_max_vals(struct bpf_verifier_env *env,
> >  					bpf_alu_string[opcode >> 4]);
> >  				return -EACCES;
> >  			} else {
> > +				struct bpf_reg_state off_reg;
> > +
> >  				/* scalar += pointer
> >  				 * This is legal, but we have to reverse our
> >  				 * src/dest handling in computing the range
> >  				 */
> > +				off_reg = *dst_reg;
> >  				err = mark_chain_precision(env, insn->dst_reg);
> >  				if (err)
> >  					return err;
> >  				return adjust_ptr_min_max_vals(env, insn,
> > -							       src_reg, dst_reg);
> > +							       src_reg, &off_reg);
> 
> [Severity: Critical]
> Does passing a pointer to the local stack variable off_reg here break the
> pointer equality check in adjust_ptr_min_max_vals()?
> 
> In adjust_ptr_min_max_vals(), we have this check to prevent subtracting a
> pointer from a scalar:
> 
> 	case BPF_SUB:
> 		if (dst_reg == off_reg) {
> 			/* scalar -= pointer.  Creates an unknown scalar */
> 			verbose(env, "R%d tried to subtract pointer from scalar\n",
> 				dst);
> 			return -EACCES;
> 		}
> 
> Since off_reg is now a stack pointer from adjust_reg_min_max_vals(), it will
> never equal the verifier state array pointer dst_reg. Could this regression
> allow eBPF programs to bypass the restriction and execute scalar -= pointer,
> potentially breaking memory isolation?

This seems right. adjust_ptr_min_max_vals() has the assumptions that
either one of these cases has to be true
1. off_reg == dst_reg && ptr_reg == src_reg
2. ptr_reg == dst_reg && off_reg == src_reg

By passing neither dst_reg and src_reg as off_reg we're breaking the
assumption. Perhaps we should have add a ptr_is_dst_reg argument for
adjust_ptr_min_max_vals(), which could also then passed down to
sanitize_ptr_alu() as well (which currently computes ptr_is_dst_reg
itself).