From: "Alexei Starovoitov" <alexei.starovoitov@gmail.com>
To: "Kumar Kartikeya Dwivedi" <memxor@gmail.com>, <bpf@vger.kernel.org>
Cc: "Alexei Starovoitov" <ast@kernel.org>,
"Andrii Nakryiko" <andrii@kernel.org>,
"Daniel Borkmann" <daniel@iogearbox.net>,
"Eduard Zingerman" <eddyz87@gmail.com>,
"Emil Tsalapatis" <emil@etsalapatis.com>, <kkd@meta.com>,
<kernel-team@meta.com>
Subject: Re: [PATCH bpf-next v2 6/7] bpf: Correct Program Structure diagnostic context
Date: Thu, 24 Sep 2026 21:05:00 +0000 [thread overview]
Message-ID: <DLNUODSRIPTF.1MRZX3443BJ19@gmail.com> (raw)
In-Reply-To: <20260924092941.3174809-7-memxor@gmail.com>
On Thu Sep 24, 2026 at 9:29 AM UTC, Kumar Kartikeya Dwivedi wrote:
> Program Structure reports have two attribution gaps. A missing jump
> table is reported at the beginning of its subprogram rather than at
> the gotox that needs the table, and the early subprogram-layout checks
> run before BTF line information is installed.
>
> Pass the failing gotox instruction into the jump-table lookup.
>
> The BTF validator needs the discovered subprogram boundaries together
> with the LD_ABS and tail-call properties collected during the layout
> scan. Collect those properties, along with the program's callx marker,
> with nested subprogram and instruction loops, then validate BTF before
> reporting layout errors. This makes validated source information
> available to the jump-boundary and fallthrough reports without changing
> either check.
>
> Moving BTF validation ahead of normal instruction validation also lets
> CO-RE see a truncated final LD_IMM64. Reject a relocation targeting
> that instruction before bpf_core_apply() can inspect or patch its
> missing second half.
>
> Link: https://lore.kernel.org/bpf/cf2f420c2b21de440a7dc51b1565c0f06d4b539640ee5c03384e5d77bcfb5686@mail.kernel.org/
> Fixes: a8f427835394 ("bpf: Report Program Structure CFG errors")
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---
> kernel/bpf/cfg.c | 6 ++---
> kernel/bpf/check_btf.c | 15 +++++++++---
> kernel/bpf/verifier.c | 54 +++++++++++++++++++++++++++++-------------
> 3 files changed, 52 insertions(+), 23 deletions(-)
>
> diff --git a/kernel/bpf/cfg.c b/kernel/bpf/cfg.c
> index 0de2f634ef67..33b98285e802 100644
> --- a/kernel/bpf/cfg.c
> +++ b/kernel/bpf/cfg.c
> @@ -288,7 +288,7 @@ static struct bpf_iarray *jt_from_map(struct bpf_map *map)
> * combined jump table in jt->items (allocated with kvcalloc)
> */
> static struct bpf_iarray *jt_from_subprog(struct bpf_verifier_env *env,
> - int subprog_start, int subprog_end)
> + int insn_idx, int subprog_start, int subprog_end)
> {
> struct bpf_iarray *jt = NULL;
> struct bpf_map *map;
> @@ -328,7 +328,7 @@ static struct bpf_iarray *jt_from_subprog(struct bpf_verifier_env *env,
> if (!jt) {
> verbose(env, "no jump tables found for subprog starting at %u\n", subprog_start);
> bpf_diag_program_structure(
> - env, subprog_start, "missing jump table",
> + env, insn_idx, "missing jump table",
> "Make sure subprograms containing gotox instructions are accompanied by jump tables referencing these subprograms.",
> "No jump table was found for the subprogram that starts at instruction %u.",
> subprog_start);
> @@ -350,7 +350,7 @@ create_jt(int t, struct bpf_verifier_env *env)
> subprog = bpf_find_containing_subprog(env, t);
> subprog_start = subprog->start;
> subprog_end = (subprog + 1)->start;
> - jt = jt_from_subprog(env, subprog_start, subprog_end);
> + jt = jt_from_subprog(env, t, subprog_start, subprog_end);
> if (IS_ERR(jt))
> return jt;
>
> diff --git a/kernel/bpf/check_btf.c b/kernel/bpf/check_btf.c
> index 0e8b3ccc7a5b..81f4dbfbf146 100644
> --- a/kernel/bpf/check_btf.c
> +++ b/kernel/bpf/check_btf.c
> @@ -373,6 +373,8 @@ static int check_core_relo(struct bpf_verifier_env *env,
> * relocation record one at a time.
> */
> for (i = 0; i < nr_core_relo; i++) {
> + u32 insn_idx;
> +
> /* future proofing when sizeof(bpf_core_relo) changes */
> err = bpf_check_uarg_tail_zero(u_core_relo, expected_size, rec_size);
> if (err) {
> @@ -391,15 +393,22 @@ static int check_core_relo(struct bpf_verifier_env *env,
> break;
> }
>
> - if (core_relo.insn_off % 8 || core_relo.insn_off / 8 >= prog->len) {
> + insn_idx = core_relo.insn_off / 8;
> + if (core_relo.insn_off % 8 || insn_idx >= prog->len) {
> verbose(env, "Invalid core_relo[%u].insn_off:%u prog->len:%u\n",
> i, core_relo.insn_off, prog->len);
> err = -EINVAL;
> break;
> }
> + if (insn_idx == prog->len - 1 &&
> + prog->insnsi[insn_idx].code == (BPF_LD | BPF_IMM | BPF_DW)) {
> + verbose(env, "Invalid core_relo[%u] targets truncated bpf_ld_imm64 insn\n",
> + i);
> + err = -EINVAL;
> + break;
> + }
>
> - err = bpf_core_apply(&ctx, &core_relo, i,
> - &prog->insnsi[core_relo.insn_off / 8]);
> + err = bpf_core_apply(&ctx, &core_relo, i, &prog->insnsi[insn_idx]);
> if (err)
> break;
> bpfptr_add(&u_core_relo, rec_size);
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 1c52e7bd570d..63b32a39a0f7 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -3098,6 +3098,34 @@ static int add_kfuncs(struct bpf_verifier_env *env)
> return 0;
> }
>
> +static void find_subprog_properties(struct bpf_verifier_env *env)
> +{
> + struct bpf_subprog_info *subprog = env->subprog_info;
> + struct bpf_insn *insn = env->prog->insnsi;
> + int cur_subprog;
> +
> + for (cur_subprog = 0; cur_subprog < env->subprog_cnt; cur_subprog++) {
> + int i;
> +
> + for (i = subprog[cur_subprog].start;
> + i < subprog[cur_subprog + 1].start; i++) {
> + u8 code = insn[i].code;
> +
> + if (code == (BPF_JMP | BPF_CALL) &&
> + insn[i].src_reg == 0 &&
> + insn[i].imm == BPF_FUNC_tail_call) {
> + subprog[cur_subprog].has_tail_call = true;
> + subprog[cur_subprog].tail_call_reachable = true;
> + }
> + if (BPF_CLASS(code) == BPF_LD &&
> + (BPF_MODE(code) == BPF_ABS || BPF_MODE(code) == BPF_IND))
> + subprog[cur_subprog].has_ld_abs = true;
> + if (bpf_is_callx(&insn[i]))
> + env->has_callx = true;
> + }
> + }
> +}
> +
> static int check_subprogs(struct bpf_verifier_env *env)
> {
> int i, subprog_start, subprog_end, off, cur_subprog = 0;
> @@ -3111,17 +3139,6 @@ static int check_subprogs(struct bpf_verifier_env *env)
> for (i = 0; i < insn_cnt; i++) {
> u8 code = insn[i].code;
>
> - if (code == (BPF_JMP | BPF_CALL) &&
> - insn[i].src_reg == 0 &&
> - insn[i].imm == BPF_FUNC_tail_call) {
> - subprog[cur_subprog].has_tail_call = true;
> - subprog[cur_subprog].tail_call_reachable = true;
> - }
> - if (BPF_CLASS(code) == BPF_LD &&
> - (BPF_MODE(code) == BPF_ABS || BPF_MODE(code) == BPF_IND))
> - subprog[cur_subprog].has_ld_abs = true;
> - if (bpf_is_callx(&insn[i]))
> - env->has_callx = true;
> if (BPF_CLASS(code) != BPF_JMP && BPF_CLASS(code) != BPF_JMP32)
> goto next;
> if (BPF_OP(code) == BPF_CALL)
> @@ -3143,9 +3160,10 @@ static int check_subprogs(struct bpf_verifier_env *env)
> }
> next:
> if (i == subprog_end - 1) {
> - /* to avoid fall-through from one subprog into another
> + /*
> + * To avoid fall-through from one subprog into another,
> * the last insn of the subprog should be either exit
> - * or unconditional jump back or bpf_throw call
> + * or unconditional jump back or bpf_throw call.
> */
> if (code != (BPF_JMP | BPF_EXIT) &&
> code != (BPF_JMP32 | BPF_JA) &&
> @@ -22410,17 +22428,19 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr,
> if (ret < 0)
> goto skip_full_check;
>
> - /* Discover all subprograms before validating their layout and BTF. */
> + /* Discover all subprograms and collect the properties needed by BTF validation. */
> ret = add_subprogs(env);
> if (ret < 0)
> goto skip_full_check;
>
> - ret = check_subprogs(env);
> + find_subprog_properties(env);
> +
> + /* Validate BTF and apply CO-RE before reporting subprogram layout errors. */
> + ret = bpf_check_btf_info(env, attr, uattr);
> if (ret < 0)
> goto skip_full_check;
>
> - /* Validate BTF against the complete subprogram layout and apply CO-RE. */
> - ret = bpf_check_btf_info(env, attr, uattr);
> + ret = check_subprogs(env);
I think this is undoing your own fix
commit c26e97721b172163
next prev parent reply other threads:[~2026-09-24 21:05 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-24 9:29 [PATCH bpf-next v2 0/7] Follow ups for verifier errors set Kumar Kartikeya Dwivedi
2026-09-24 9:29 ` [PATCH bpf-next v2 1/7] bpf: Correct verifier diagnostic attribution for stack reads Kumar Kartikeya Dwivedi
2026-09-24 9:29 ` [PATCH bpf-next v2 2/7] selftests/bpf: Test verifier stack-read diagnostic attribution Kumar Kartikeya Dwivedi
2026-09-24 9:29 ` [PATCH bpf-next v2 3/7] bpf: Drop dead spill diagnostic condition Kumar Kartikeya Dwivedi
2026-09-24 9:29 ` [PATCH bpf-next v2 4/7] bpf: Report non-sleepable kfunc programs accurately Kumar Kartikeya Dwivedi
2026-09-24 9:29 ` [PATCH bpf-next v2 5/7] selftests/bpf: Test non-sleepable kfunc context Kumar Kartikeya Dwivedi
2026-09-24 9:29 ` [PATCH bpf-next v2 6/7] bpf: Correct Program Structure diagnostic context Kumar Kartikeya Dwivedi
2026-09-24 21:05 ` Alexei Starovoitov [this message]
2026-09-25 5:22 ` Kumar Kartikeya Dwivedi
2026-09-24 9:29 ` [PATCH bpf-next v2 7/7] selftests/bpf: Test " 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=DLNUODSRIPTF.1MRZX3443BJ19@gmail.com \
--to=alexei.starovoitov@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=emil@etsalapatis.com \
--cc=kernel-team@meta.com \
--cc=kkd@meta.com \
--cc=memxor@gmail.com \
/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.