From: "Kumar Kartikeya Dwivedi" <memxor@gmail.com>
To: "Alexei Starovoitov" <alexei.starovoitov@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: Fri, 25 Sep 2026 07:22:15 +0200 [thread overview]
Message-ID: <DLO59414RWGN.382APGRD4P0YG@gmail.com> (raw)
In-Reply-To: <DLNUODSRIPTF.1MRZX3443BJ19@gmail.com>
On Thu Sep 24, 2026 at 11:05 PM CEST, Alexei Starovoitov wrote:
> 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
Only find_subprog_properties() is moving ahead to let us see func/line info for
formatting messages.
I think you missed v3, that was the most recent posting. Already dropped CO-RE
change there.
https://lore.kernel.org/bpf/20260924170646.2366016-7-memxor@gmail.com
Should I resend last 3 commits again (or just patch 6)?
next prev parent reply other threads:[~2026-09-25 5:22 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
2026-09-25 5:22 ` Kumar Kartikeya Dwivedi [this message]
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=DLO59414RWGN.382APGRD4P0YG@gmail.com \
--to=memxor@gmail.com \
--cc=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 \
/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