From: Yonghong Song <yonghong.song@linux.dev>
To: Amery Hung <ameryhung@gmail.com>
Cc: bpf@vger.kernel.org, Alexei Starovoitov <ast@kernel.org>,
Andrii Nakryiko <andrii@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
"Jose E . Marchesi" <jose.marchesi@oracle.com>,
kernel-team@fb.com, Martin KaFai Lau <martin.lau@kernel.org>,
Puranjay Mohan <puranjay@kernel.org>
Subject: Re: [PATCH bpf-next 4/9] bpf: Refactor to avoid redundant calculation of bpf_reg_state
Date: Tue, 21 Apr 2026 16:42:32 -0700 [thread overview]
Message-ID: <08436c41-f8ff-43ea-b0d5-f2a1dd3aff92@linux.dev> (raw)
In-Reply-To: <CAMB2axNsVNX2hRp0wMJvSfhpryH-vEFB23ACk0x8T8hcFE7VdQ@mail.gmail.com>
On 4/21/26 2:40 PM, Amery Hung wrote:
> On Tue, Apr 21, 2026 at 10:20 AM Yonghong Song <yonghong.song@linux.dev> wrote:
>> In many cases, once a bpf_reg_state is defined, it can pass to
>> callee's. Otherwise, callee will need to get bpf_reg_state again
>> based on regno. More importantly, this is needed for later stack
>> arguments for kfuncs since the register state for stack arguments does
>> not have a corresponding regno. So it makes sense to pass reg state
>> for callee's.
>>
>> The following is the only change to avoid compilation warning:
>> static int sanitize_check_bounds(struct bpf_verifier_env *env,
>> const struct bpf_insn *insn,
>> - const struct bpf_reg_state *dst_reg)
>> + struct bpf_reg_state *dst_reg)
>>
>> Acked-by: Puranjay Mohan <puranjay@kernel.org>
>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>> ---
>> kernel/bpf/verifier.c | 213 ++++++++++++++++++------------------------
>> 1 file changed, 93 insertions(+), 120 deletions(-)
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index ed04fef49f6c..b56a11fc3856 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -3929,13 +3929,13 @@ static int check_stack_write_fixed_off(struct bpf_verifier_env *env,
>> static int check_stack_write_var_off(struct bpf_verifier_env *env,
>> /* func where register points to */
>> struct bpf_func_state *state,
>> - int ptr_regno, int off, int size,
>> + struct bpf_reg_state *ptr_reg, int off, int size,
>> int value_regno, int insn_idx)
>> {
> The comment needs to be updated.
>
>> struct bpf_func_state *cur; /* state of the current function */
>> int min_off, max_off;
>> int i, err;
>> - struct bpf_reg_state *ptr_reg = NULL, *value_reg = NULL;
>> + struct bpf_reg_state *value_reg = NULL;
>> struct bpf_insn *insn = &env->prog->insnsi[insn_idx];
>> bool writing_zero = false;
>> /* set if the fact that we're writing a zero is used to let any
>> @@ -3944,7 +3944,6 @@ static int check_stack_write_var_off(struct bpf_verifier_env *env,
>> bool zero_used = false;
>>
>> cur = env->cur_state->frame[env->cur_state->curframe];
>> - ptr_reg = &cur->regs[ptr_regno];
>> min_off = ptr_reg->smin_value + off;
>> max_off = ptr_reg->smax_value + off + size;
>> if (value_regno >= 0)
>> @@ -4241,7 +4240,7 @@ enum bpf_access_src {
>> ACCESS_HELPER = 2, /* the access is performed by a helper */
>> };
>>
>> -static int check_stack_range_initialized(struct bpf_verifier_env *env,
>> +static int check_stack_range_initialized(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
>> int regno, int off, int access_size,
>> bool zero_size_allowed,
>> enum bpf_access_type type,
>> @@ -4265,18 +4264,16 @@ static struct bpf_reg_state *reg_state(struct bpf_verifier_env *env, int regno)
>> * offset; for a fixed offset check_stack_read_fixed_off should be used
>> * instead.
>> */
> Same here
>
>> -static int check_stack_read_var_off(struct bpf_verifier_env *env,
>> +static int check_stack_read_var_off(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
>> int ptr_regno, int off, int size, int dst_regno)
>> {
>> - /* The state of the source register. */
>> - struct bpf_reg_state *reg = reg_state(env, ptr_regno);
>> struct bpf_func_state *ptr_state = bpf_func(env, reg);
>> int err;
>> int min_off, max_off;
>>
>> /* Note that we pass a NULL meta, so raw access will not be permitted.
>> */
>> - err = check_stack_range_initialized(env, ptr_regno, off, size,
>> + err = check_stack_range_initialized(env, reg, ptr_regno, off, size,
>> false, BPF_READ, NULL);
>> if (err)
>> return err;
>> @@ -4298,10 +4295,9 @@ static int check_stack_read_var_off(struct bpf_verifier_env *env,
>> * can be -1, meaning that the read value is not going to a register.
>> */
>> static int check_stack_read(struct bpf_verifier_env *env,
>> - int ptr_regno, int off, int size,
>> + struct bpf_reg_state *reg, int ptr_regno, int off, int size,
>> int dst_regno)
>> {
>> - struct bpf_reg_state *reg = reg_state(env, ptr_regno);
>> struct bpf_func_state *state = bpf_func(env, reg);
>> int err;
>> /* Some accesses are only permitted with a static offset. */
>> @@ -4337,7 +4333,7 @@ static int check_stack_read(struct bpf_verifier_env *env,
>> * than fixed offset ones. Note that dst_regno >= 0 on this
>> * branch.
>> */
>> - err = check_stack_read_var_off(env, ptr_regno, off, size,
>> + err = check_stack_read_var_off(env, reg, ptr_regno, off, size,
>> dst_regno);
>> }
>> return err;
>> @@ -4354,10 +4350,9 @@ static int check_stack_read(struct bpf_verifier_env *env,
>> * The caller must ensure that the offset falls within the maximum stack size.
>> */
> Ditto
Ack. Will fix all the above in comments.
>
>> static int check_stack_write(struct bpf_verifier_env *env,
>> - int ptr_regno, int off, int size,
>> + struct bpf_reg_state *reg, int off, int size,
>> int value_regno, int insn_idx)
>> {
>> - struct bpf_reg_state *reg = reg_state(env, ptr_regno);
>> struct bpf_func_state *state = bpf_func(env, reg);
>> int err;
>>
>> @@ -4370,16 +4365,15 @@ static int check_stack_write(struct bpf_verifier_env *env,
>> * than fixed offset ones.
>> */
>> err = check_stack_write_var_off(env, state,
>> - ptr_regno, off, size,
>> + reg, off, size,
>> value_regno, insn_idx);
>> }
>> return err;
>> }
>>
> Otherwise looks good to me.
>
> Reviewed-by: Amery Hung <ameryhung@gmail.com>
next prev parent reply other threads:[~2026-04-21 23:42 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-21 17:19 [PATCH bpf-next 0/9] bpf: Prepare to support stack arguments Yonghong Song
2026-04-21 17:19 ` [PATCH bpf-next 1/9] bpf: Remove unused parameter from check_map_kptr_access() Yonghong Song
2026-04-21 17:19 ` [PATCH bpf-next 2/9] bpf: Fix tail_call_reachable leak Yonghong Song
2026-04-21 18:06 ` bot+bpf-ci
2026-04-22 0:29 ` Yonghong Song
2026-04-21 17:19 ` [PATCH bpf-next 3/9] bpf: Remove WARN_ON_ONCE in check_kfunc_mem_size_reg() Yonghong Song
2026-04-21 17:19 ` [PATCH bpf-next 4/9] bpf: Refactor to avoid redundant calculation of bpf_reg_state Yonghong Song
2026-04-21 21:40 ` Amery Hung
2026-04-21 23:42 ` Yonghong Song [this message]
2026-04-21 17:19 ` [PATCH bpf-next 5/9] bpf: Refactor to handle memory and size together Yonghong Song
2026-04-21 17:19 ` [PATCH bpf-next 6/9] bpf: Rename existing argno to arg Yonghong Song
2026-04-21 17:20 ` [PATCH bpf-next 7/9] bpf: Prepare verifier logs for upcoming kfunc stack arguments Yonghong Song
2026-04-21 22:07 ` Alexei Starovoitov
2026-04-21 23:56 ` Yonghong Song
2026-04-22 0:37 ` Alexei Starovoitov
2026-04-22 1:20 ` Yonghong Song
2026-04-22 1:52 ` Alexei Starovoitov
2026-04-21 17:20 ` [PATCH bpf-next 8/9] bpf: Introduce bpf register BPF_REG_PARAMS Yonghong Song
2026-04-21 22:10 ` Alexei Starovoitov
2026-04-22 0:09 ` Yonghong Song
2026-04-22 0:42 ` Alexei Starovoitov
2026-04-22 1:10 ` Yonghong Song
2026-04-21 17:20 ` [PATCH bpf-next 9/9] bpf: Reuse MAX_BPF_FUNC_ARGS for maximum number of arguments Yonghong Song
2026-04-21 17:52 ` bot+bpf-ci
2026-04-21 19:13 ` [PATCH bpf-next 0/9] bpf: Prepare to support stack arguments Kumar Kartikeya Dwivedi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=08436c41-f8ff-43ea-b0d5-f2a1dd3aff92@linux.dev \
--to=yonghong.song@linux.dev \
--cc=ameryhung@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=jose.marchesi@oracle.com \
--cc=kernel-team@fb.com \
--cc=martin.lau@kernel.org \
--cc=puranjay@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox