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

[Eduard Zingerman <eddyz87@gmail.com>]

On Mon, 2026-07-20 at 16:37 +0000, Yiyang Chen wrote:
> When scalar += pointer is handled in adjust_ptr_min_max_vals(), the
> destination register inherits the pointer type and id from the source
> pointer. For PTR_TO_STACK, the inherited pointer state also has to carry
> the stack frame number.
> 
> Without the frame number copy, a stack pointer derived inside a callee
> through scalar += fp can be recorded as pointing to frame 0. Stack reads
> and writes through that register can then update or consult the caller
> frame while the actual instruction uses the callee frame.
> 
> Copy the frame number when PTR_TO_STACK state is inherited by the
> commuted arithmetic form.

Could you please infer a "Fixes" tag?

> Signed-off-by: Yiyang Chen <chenyy23@mails.tsinghua.edu.cn>
> ---
>  kernel/bpf/verifier.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 52be0a118cce0..58017141d52b7 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -13796,11 +13796,14 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
>  		return -EACCES;
>  	}
>  
> -	/* In case of 'scalar += pointer', dst_reg inherits pointer type and id.
> -	 * The id may be overwritten later if we create a new variable offset.
> +	/* In case of 'scalar += pointer', dst_reg inherits pointer type, id,
> +	 * and for stack pointers also the frame number. The id may be overwritten
> +	 * later if we create a new variable offset.
>  	 */
>  	dst_reg->type = ptr_reg->type;
>  	dst_reg->id = ptr_reg->id;
> +	if (base_type(ptr_reg->type) == PTR_TO_STACK)
> +		dst_reg->frameno = ptr_reg->frameno;

This patch fixes a real issue and this a surprisingly buggy piece of
code. Looking at other fields defined in bpf_reg_state, it appears
that several additional modifications are necessary:
- dst_reg->delta = 0
- dst_reg->parent_id = ptr_reg->parent_id

Given such an error prone nature, I think it would be better to:
- stash the value of the off_reg in a temporary variable and adjust
  off_reg pointer accordingly (see bpf_verifier_env for a collection
  of similar temporaries).
- do *dst_reg = *ptr_reg instead of the fixup.

Wdyt?

>  
>  	if (!check_reg_sane_offset_scalar(env, off_reg, ptr_reg->type) ||
>  	    !check_reg_sane_offset_ptr(env, ptr_reg, ptr_reg->type))