Re: [PATCH v2] libbpf: poison unresolved weak kfuncs in light skeletons
Sid Nayyar <[email protected]>
| Newsgroups | org.kernel.vger.bpf,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CA+OvW8aDu_-d3YCEtcJ5muGYhKMiRC2q6QQ5rG5r1x-64OKRZg@mail.gmail.com> |
On Tue, Jul 28, 2026 at 12:25 AM Andrii Nakryiko <[email protected]> wrote: > > On Wed, Jul 22, 2026 at 4:41 AM Siddharth Nayyar <[email protected]> wrote: > > However, the generator fails to clear src_reg (which is set to > > BPF_PSEUDO_KFUNC_CALL). This leaves the instruction looking like a valid > > pseudo kfunc call with a zero BTF ID. When the target verifier's > > add_subprog_and_kfunc encounters this, it unconditionally scans all > > BPF_PSEUDO_KFUNC_CALL instructions, sees imm == 0, and panics or > > panics? as in can crash the kernel? overstatement or it's actually that bad? I meant the verifier rejects loading the BPF program, sorry if panic was an overstatement. > > fails the load (e.g. bpf_unspec#0 or -EINVAL). This entirely breaks > > also, I think we intentionally do not fail for kfunc with btf ID zero > eagerly, so if subsequent dead code elimination logic proves this > instruction is unreachable, it should be ignored anyways. > > can you please double check, this sounds suspicious. invalid helper vs > invalid kfunc call shouldn't matter much (and if it does, we should > fix *that*) I investigated further and it seems like handling of unresolved weak kfuncs in add_subprog_and_kfunc() works fine. This change doesn't seem to add any functional benefit to BPF program loading. Apologies for the noise. Self-NACK Regards, Siddharth Nayyar > > the verifier's dead-code elimination logic which expects to cleanly prune > > branches protected by bpf_ksym_exists(). > > > > Furthermore, when the generator processes subsequent references to the > > same unresolved weak kfunc, it copies the imm and off fields from > > the first occurrence but skips the src_reg field, meaning subsequent > > calls also retain the poisonous BPF_PSEUDO_KFUNC_CALL flag. > > > > This patch fixes the issue by explicitly clearing src_reg for both the > > initial occurrence and all subsequent occurrences of unresolved weak > > kfuncs, converting them into standard invalid helper calls that the > > verifier's dead-code eliminator can safely recognize and discard. > > > > Fixes: 18f4fccbf314 ("libbpf: Update gen_loader to emit BTF_KIND_FUNC relocations") > > Signed-off-by: Siddharth Nayyar <[email protected]> > > Acked-by: Yonghong Song <[email protected]> > > --- > > Changes in v2: > > - Rebased and added Fixes and Acked-by tags > > - Link to v1: https://lore.kernel.org/r/[email protected] > > --- > > tools/lib/bpf/gen_loader.c | 15 ++++++++++++--- > > 1 file changed, 12 insertions(+), 3 deletions(-) > > > > diff --git a/tools/lib/bpf/gen_loader.c b/tools/lib/bpf/gen_loader.c > > index d79695f01c87..a08071114347 100644 > > --- a/tools/lib/bpf/gen_loader.c > > +++ b/tools/lib/bpf/gen_loader.c > > @@ -783,10 +783,17 @@ static void emit_relo_kfunc_btf(struct bpf_gen *gen, struct ksym_relo_desc *relo > > return; > > /* try to copy from existing bpf_insn */ > > if (kdesc->ref > 1) { > > - move_blob2blob(gen, insn + offsetof(struct bpf_insn, imm), 4, > > - kdesc->insn + offsetof(struct bpf_insn, imm)); > > move_blob2blob(gen, insn + offsetof(struct bpf_insn, off), 2, > > kdesc->insn + offsetof(struct bpf_insn, off)); > > + move_blob2blob(gen, insn + offsetof(struct bpf_insn, imm), 4, > > + kdesc->insn + offsetof(struct bpf_insn, imm)); > > + /* > > + * jump over src_reg adjustment if imm (btf_id) is not 0, reuse BPF_REG_0 from > > + * move_blob2blob. If btf_id is zero, clear BPF_PSEUDO_KFUNC_CALL flag in src_reg > > + * of call insn. > > + */ > > + emit(gen, BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1)); > > + emit(gen, BPF_ST_MEM(BPF_B, BPF_REG_8, 1, 0)); > > goto log; > > } > > /* remember insn offset, so we can copy BTF ID and FD later */ > > @@ -804,10 +811,12 @@ static void emit_relo_kfunc_btf(struct bpf_gen *gen, struct ksym_relo_desc *relo > > } > > kdesc->off = btf_fd_idx; > > /* jump to success case */ > > - emit(gen, BPF_JMP_IMM(BPF_JSGE, BPF_REG_7, 0, 3)); > > + emit(gen, BPF_JMP_IMM(BPF_JSGE, BPF_REG_7, 0, 4)); > > /* set value for imm, off as 0 */ > > emit(gen, BPF_ST_MEM(BPF_W, BPF_REG_8, offsetof(struct bpf_insn, imm), 0)); > > emit(gen, BPF_ST_MEM(BPF_H, BPF_REG_8, offsetof(struct bpf_insn, off), 0)); > > + /* clear src_reg (and dst_reg) to convert pseudo kfunc call into invalid helper call */ > > + emit(gen, BPF_ST_MEM(BPF_B, BPF_REG_8, 1, 0)); > > /* skip success case for ret < 0 */ > > emit(gen, BPF_JMP_IMM(BPF_JA, 0, 0, 10)); > > /* store btf_id into insn[insn_idx].imm */ > > > > --- > > base-commit: 248951ddc14de84de3910f9b13f51491a8cd91df > > change-id: 20260622-bpf-lskel-fixes-2-758036ae9911 > > > > Best regards, > > -- > > Siddharth Nayyar <[email protected]> > >