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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.