From: sashiko-bot@kernel.org
To: "Qiliang Yuan" <odys.yuan@gmail.com>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next v2 14/22] bpf: Move bpf_convert_ctx_accesses() to the patch list
Date: Fri, 30 Oct 2026 12:24:46 +0000 [thread overview]
Message-ID: <sashiko-outbox-191010@kernel.org> (raw)
In-Reply-To: <20261030-bpf-verifier-patch-batch-v2-14-55cd43439f7c@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] bpf: lost in-place insn 0 rewrites in bpf_convert_ctx_accesses()
--
commit 4384b7c62a67d8070d7d3e407afbbb78ae703556
Author: Qiliang Yuan <odys.yuan@gmail.com>
bpf: Move bpf_convert_ctx_accesses() to the patch list
This commit converts bpf_convert_ctx_accesses() from patching instructions
immediately via bpf_patch_insn_data() to queuing prologues, epilogue context
stores, and instruction rewrites in env->patch_list and committing them at
the end of the pass.
> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
> index 6d99de1be2581..08c89f2baffb6 100644
> --- a/kernel/bpf/fixups.c
> +++ b/kernel/bpf/fixups.c
> @@ -1090,16 +1090,15 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env)
> {
> struct bpf_subprog_info *subprogs = env->subprog_info;
> const struct bpf_verifier_ops *ops = env->ops;
> - int i, cnt, size, ctx_field_size, ret, delta = 0, epilogue_cnt = 0;
> + int i, cnt, size, ctx_field_size, ret, epilogue_cnt = 0;
> const int insn_cnt = env->prog->len;
> struct bpf_insn *epilogue_buf = env->epilogue_buf;
> struct bpf_insn *insn_buf = env->insn_buf;
> struct bpf_insn *insn;
> u32 target_size, size_default, off;
> - struct bpf_prog *new_prog;
> enum bpf_access_type type;
> bool is_narrower_load;
> - int epilogue_idx = 0;
> + int epilogue_idx = -1;
>
> if (ops->gen_epilogue) {
> epilogue_cnt = ops->gen_epilogue(epilogue_buf, env->prog,
> @@ -1109,17 +1108,12 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env)
> return -EFAULT;
> } else if (epilogue_cnt) {
> /* Save the ARG_PTR_TO_CTX for the epilogue to use */
> - cnt = 0;
> subprogs[0].stack_depth += 8;
> - insn_buf[cnt++] = BPF_STX_MEM(BPF_DW, BPF_REG_FP, BPF_REG_1,
> - -subprogs[0].stack_depth);
> - insn_buf[cnt++] = env->prog->insnsi[0];
> - new_prog = __bpf_patch_insn_data(env, 0, insn_buf, cnt,
> - BPF_PATCH_MOVE_TARGET);
> - if (!new_prog)
> - return -ENOMEM;
> - env->prog = new_prog;
> - delta += cnt - 1;
> + insn_buf[0] = BPF_STX_MEM(BPF_DW, BPF_REG_FP, BPF_REG_1,
> + -subprogs[0].stack_depth);
> + ret = bpf_patch_list_insert(env, 0, insn_buf, 1);
> + if (ret)
> + return ret;
>
> ret = add_kfunc_in_insns(env, epilogue_buf, epilogue_cnt - 1);
> if (ret < 0)
> @@ -1150,13 +1144,14 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env)
> env->prog->aux->attach_btf_id))
> return -EINVAL;
> env->prog->aux->has_prologue = true;
> - new_prog = __bpf_patch_insn_data(env, 0, insn_buf, cnt,
> - BPF_PATCH_MOVE_TARGET);
> - if (!new_prog)
> - return -ENOMEM;
> -
> - env->prog = new_prog;
> - delta += cnt - 1;
> + /* the prologue ends with the first insn of the program */
> + if (verifier_bug_if(memcmp(&insn_buf[cnt - 1], env->prog->insnsi,
> + sizeof(*insn_buf)), env,
> + "prologue doesn't end with the first insn"))
> + return -EFAULT;
> + ret = bpf_patch_list_insert(env, 0, insn_buf, cnt - 1);
> + if (ret)
> + return ret;
>
> ret = add_kfunc_in_insns(env, insn_buf, cnt - 1);
> if (ret < 0)
> @@ -1164,21 +1159,18 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env)
> }
> }
>
> - if (delta)
> - WARN_ON(adjust_jmp_off(env->prog, 0, delta));
> -
> if (bpf_prog_is_offloaded(env->prog->aux))
> - return 0;
> -
> - insn = env->prog->insnsi + delta;
> + return bpf_patch_list_commit(env);
>
> - for (i = 0; i < insn_cnt; i++, insn++) {
> + for (i = 0; i < insn_cnt; i++) {
> bpf_convert_ctx_access_t convert_ctx_access;
> enum bpf_reg_type ptr_type;
> u8 mode;
>
> - if (env->insn_aux_data[i + delta].nospec) {
> - WARN_ON_ONCE(env->insn_aux_data[i + delta].alu_state);
> + insn = &env->prog->insnsi[i];
[ ... ]
> @@ -1226,42 +1214,34 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env)
> insn->code == (BPF_STX | BPF_ATOMIC | BPF_H) ||
> insn->code == (BPF_STX | BPF_ATOMIC | BPF_W) ||
> insn->code == (BPF_STX | BPF_ATOMIC | BPF_DW)) &&
> - env->insn_aux_data[i + delta].ptr_type == PTR_TO_ARENA) {
> + env->insn_aux_data[i].ptr_type == PTR_TO_ARENA) {
> insn->code = BPF_STX | BPF_PROBE_ATOMIC | BPF_SIZE(insn->code);
> env->prog->aux->num_exentries++;
> continue;
[Severity: Critical]
When ops->gen_epilogue or ops->gen_prologue emits instructions before the
loop in bpf_convert_ctx_accesses(), bpf_patch_list_insert(env, 0, ...)
creates a patch entry at offset 0 in env->patch_list and copies the
unpatched env->prog->insnsi[0] into env->patch_list.insns.
In the loop, insn is initialized to &env->prog->insnsi[i]. When i == 0 and
env->insn_aux_data[0].nospec is false, several branches modify insn->code in
place and execute continue without calling bpf_patch_list_add():
1. PTR_TO_ARENA atomic operations rewritten to BPF_STX | BPF_PROBE_ATOMIC:
insn->code = BPF_STX | BPF_PROBE_ATOMIC | BPF_SIZE(insn->code);
env->prog->aux->num_exentries++;
continue;
2. PTR_TO_ARENA loads and stores rewritten to BPF_PROBE_MEM32SX or
BPF_PROBE_MEM32:
if (BPF_MODE(insn->code) == BPF_MEMSX) {
...
insn->code = BPF_CLASS(insn->code) | BPF_PROBE_MEM32SX | BPF_SIZE(insn->code);
} else {
insn->code = BPF_CLASS(insn->code) | BPF_PROBE_MEM32 | BPF_SIZE(insn->code);
}
env->prog->aux->num_exentries++;
continue;
3. Fault-prone pointer loads (bpf_may_fault_on_deref(ptr_type)) rewritten to
BPF_LDX | BPF_PROBE_MEM or BPF_LDX | BPF_PROBE_MEMSX when
arena_member_fixup() returns 0:
if (BPF_MODE(insn->code) == BPF_MEM)
insn->code = BPF_LDX | BPF_PROBE_MEM |
BPF_SIZE(insn->code);
else
insn->code = BPF_LDX | BPF_PROBE_MEMSX |
BPF_SIZE(insn->code);
env->prog->aux->num_exentries++;
Because those branches only mutate env->prog->insnsi[0].code and do not
update the copy already queued in env->patch_list.insns (nor call
bpf_patch_list_add()), won't bpf_patch_list_commit() overwrite instruction 0
with the unpatched copy from env->patch_list.insns, losing the
BPF_PROBE_ATOMIC, BPF_PROBE_MEM32, or BPF_PROBE_MEM rewrite and leaving the
instruction without arena address relocation or exception table protection?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261030-bpf-verifier-patch-batch-v2-0-55cd43439f7c@gmail.com?part=14
next prev parent reply other threads:[~2026-10-30 12:24 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-30 11:51 [PATCH bpf-next v2 00/22] bpf: Replace bpf_patch_insn_data() with a list of deferred patches Qiliang Yuan
2026-10-30 11:51 ` [PATCH bpf-next v2 01/22] bpf: Add a list of deferred instruction patches Qiliang Yuan
2026-10-30 11:51 ` [PATCH bpf-next v2 02/22] bpf: Remap the verifier state through the patch list Qiliang Yuan
2026-10-30 12:44 ` bot+bpf-ci
2026-10-30 11:51 ` [PATCH bpf-next v2 03/22] bpf: Apply single-instruction patches of the verifier in place Qiliang Yuan
2026-10-30 11:51 ` [PATCH bpf-next v2 04/22] bpf: Queue the rewrites of bpf_do_misc_fixups() Qiliang Yuan
2026-10-30 11:51 ` [PATCH bpf-next v2 05/22] bpf: Expand a backward may_goto to gotol when patches push it out of range Qiliang Yuan
2026-10-30 12:59 ` bot+bpf-ci
2026-10-30 11:51 ` [PATCH bpf-next v2 06/22] bpf: Drop the constant delta from bpf_do_misc_fixups() Qiliang Yuan
2026-10-30 11:51 ` [PATCH bpf-next v2 07/22] bpf: Let the patch list remove instructions Qiliang Yuan
2026-10-30 12:11 ` sashiko-bot
2026-10-30 11:51 ` [PATCH bpf-next v2 08/22] bpf: Remove dead code through the patch list Qiliang Yuan
2026-10-30 11:51 ` [PATCH bpf-next v2 09/22] bpf: Strip nops " Qiliang Yuan
2026-10-30 12:45 ` bot+bpf-ci
2026-10-30 11:51 ` [PATCH bpf-next v2 10/22] bpf: Batch the patches of bpf_opt_subreg_zext_lo32_rnd_hi32() Qiliang Yuan
2026-10-30 11:51 ` [PATCH bpf-next v2 11/22] bpf: Allow patching an instruction twice through the patch list Qiliang Yuan
2026-10-30 11:51 ` [PATCH bpf-next v2 12/22] bpf: Teach the patch list to insert in front of an instruction Qiliang Yuan
2026-10-30 12:44 ` bot+bpf-ci
2026-10-30 11:51 ` [PATCH bpf-next v2 13/22] bpf: Allow inserting in front of a queued patch Qiliang Yuan
2026-10-30 11:51 ` [PATCH bpf-next v2 14/22] bpf: Move bpf_convert_ctx_accesses() to the patch list Qiliang Yuan
2026-10-30 12:24 ` sashiko-bot [this message]
2026-10-30 11:52 ` [PATCH bpf-next v2 15/22] bpf: Insert the may_goto counter setup through " Qiliang Yuan
2026-10-30 11:52 ` [PATCH bpf-next v2 16/22] bpf: Append the hidden subprog " Qiliang Yuan
2026-10-30 11:52 ` [PATCH bpf-next v2 17/22] bpf: Inline bpf_loop() calls " Qiliang Yuan
2026-10-30 11:52 ` [PATCH bpf-next v2 18/22] bpf: Apply the patch list without a verifier env Qiliang Yuan
2026-10-30 11:52 ` [PATCH bpf-next v2 19/22] bpf: Blind constants through the patch list Qiliang Yuan
2026-10-30 12:44 ` bot+bpf-ci
2026-10-30 11:52 ` [PATCH bpf-next v2 20/22] bpf: Remove the old instruction patching code Qiliang Yuan
2026-10-30 11:52 ` [PATCH bpf-next v2 21/22] selftests/bpf: Test jumps around patches of bpf_do_misc_fixups() Qiliang Yuan
2026-10-30 11:52 ` [PATCH bpf-next v2 22/22] selftests/bpf: Cover a backward may_goto pushed out of range by patches Qiliang Yuan
2026-10-30 12:13 ` sashiko-bot
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=sashiko-outbox-191010@kernel.org \
--to=sashiko-bot@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=odys.yuan@gmail.com \
--cc=sashiko-reviews@lists.linux.dev \
/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