Re: [PATCH bpf-next v7 1/9] bpf: mark instructions accessing program stack

[email protected]
Newsgroups org.kernel.vger.bpf,org.kernel.vger.linux-kernel,org.kernel.vger.linux-kselftest
Message-ID <f6a88fe55d543f5fe2a6d9e856acb4cf1b87b8210de58459df1272c8a456d926@mail.kernel.org>
> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index 004b06785521..412a13a664fa 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -725,6 +725,8 @@ struct bpf_insn_aux_data {
>  	u16 const_reg_map_mask;
>  	u16 const_reg_subprog_mask;
>  	u32 const_reg_vals[10];
> +	/* instruction can access non-stack memory */
> +	bool non_stack_access;
>  };

[ ... ]

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index e036ae20bf6b..040af75d9550 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -3272,6 +3272,11 @@ static void mark_indirect_target(struct bpf_verifier_env *env, int idx)
>  	env->insn_aux_data[idx].indirect_target = true;
>  }
>
> +static void mark_non_stack_access(struct bpf_verifier_env *env, int idx)
> +{
> +	env->insn_aux_data[idx].non_stack_access = true;
> +}
> +

[ ... ]

> @@ -6654,6 +6659,10 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, struct b
>  				clear_scalar_id(&regs[value_regno]);
>  		}
>  	}
> +
> +	if (!err && reg->type != PTR_TO_STACK)
> +		mark_non_stack_access(env, insn_idx);
> +
>  	return err;
>  }

Does this test read the correct register type? For a load instruction where
dst_reg == src_reg (e.g., rX = *(u64 *)(rX + off)), check_mem_access()
receives reg = &regs[src_reg] and value_regno = dst_reg, so reg points to
the same slot that gets overwritten by the load.

When the pointer is PTR_TO_STACK, check_stack_read() runs:

check_stack_read_fixed_off() {
    state->regs[dst_regno] = *reg;  /* or mark_reg_stack_read() */
}

By the time execution reaches the new test, reg->type is the loaded value's
type (typically SCALAR_VALUE), not PTR_TO_STACK. The condition evaluates to
true and marks a stack-only access as non-stack.

Both check_load_mem() and check_store_reg() snapshot the pointer type before
calling check_mem_access():

    /* verifier.c check_load_mem */
    src_reg_type = regs[insn->src_reg].type;
    err = check_mem_access(env, env->insn_idx, ...);

    /* verifier.c check_store_reg */
    dst_reg_type = regs[insn->dst_reg].type;
    err = check_mem_access(env, env->insn_idx, ...);

Could check_mem_access() snapshot reg->type at entry and test that snapshot
instead?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32534828209
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.