Re: [PATCH v4 bpf-next 2/7] resolve_btfids: Populate BTF link section
flat view
From: bot+bpf-ci@kernel.org
Date: 2026-10-09 12:24:34
Also in:
bpf
resolve_btfids: Populate BTF link section
TL;DR: Nits only, no bugs found: patch_btf_link() duplicates much of patch_btfids() and reopens vmlinux repeatedly, runs a full elf_collect() just for class/encoding, and could use ensure_mem().
quoted hunk ↗ jump to hunk
diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c index 15e7be13061f..f737fac60fa2 100644 --- a/tools/bpf/resolve_btfids/main.c +++ b/tools/bpf/resolve_btfids/main.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -267,6 +282,49 @@ static int __ensure_mem(void **data, u32 *cap, u32 cnt, size_t elem_sz) #define ensure_mem(arr_ptr, cap_ptr, cnt) \ __ensure_mem((void **)(arr_ptr), (cap_ptr), (cnt), sizeof(**(arr_ptr))) +static int parse_btf_link(const struct option *opt, const char *arg, int unset) +{ + struct btf_links *links = opt->value; + struct btf_link *link; + char *separator; + + if (unset) + return -EINVAL; + if (__ensure_mem((void **)&links->links, &links->cap, links->cnt + 1, + sizeof(*links->links))) + return -ENOMEM;
this isn't a bug, but could parse_btf_link() use the ensure_mem() macro defined right above it, the way the other callers in main.c do? ensure_mem(&links->links, &links->cap, links->cnt + 1) would expand to the same __ensure_mem() call. [ ... ]
quoted hunk ↗ jump to hunk
@@ -1030,6 +1089,153 @@ static int dump_raw_btf(struct btf *btf, const char *out_path) return 0; } +static int patch_btf_link(struct object *obj, const struct btf_link *link) +{ + char section[128];
[ ... ]
quoted hunk ↗ jump to hunk
+ fd = open(obj->path, O_RDWR); + if (fd < 0) { + err = -errno; + pr_err("FAILED to open %s: %s\n", obj->path, strerror(errno)); + goto out; + } + elf = elf_begin(fd, ELF_C_RDWR_MMAP, NULL); + if (!elf) { + err = -EINVAL; + pr_err("FAILED to create ELF descriptor for %s: %s\n", + obj->path, elf_errmsg(-1)); + goto out; + } + elf_flagelf(elf, ELF_C_SET, ELF_F_LAYOUT); + if (elf_getshdrstrndx(elf, &shdrstrndx)) { + pr_err("FAILED to get section name index from %s: %s\n", + obj->path, elf_errmsg(-1)); + goto out; + } + while ((scn = elf_nextscn(elf, scn))) { + if (gelf_getshdr(scn, &sh) != &sh) { + pr_err("FAILED to get section header from %s: %s\n", + obj->path, elf_errmsg(-1)); + goto out; + } + name = elf_strptr(elf, shdrstrndx, sh.sh_name); + if (name && !strcmp(name, section)) + break; + } + if (!scn) { + pr_err("FAILED: section %s not found in %s\n", section, obj->path); + goto out; + } + data = elf_getdata(scn, NULL); + if (sh.sh_type != SHT_PROGBITS || !data || !data->d_buf || + data->d_size != module_name_len + + BTF_SHA256_DIGEST_LENGTH + sizeof(u32)) { + pr_err("FAILED: section %s is not a writable BTF link\n", section); + goto out; + }
this isn't a bug, but most of patch_btf_link() repeats what patch_btfids() already does: the open(O_RDWR) / elf_begin(ELF_C_RDWR_MMAP) / elf_getshdrstrndx() setup, the loop that looks up a section by name, elf_getdata(), and the elf_flagdata(ELF_F_DIRTY) / elf_update(ELF_C_WRITE) / elf_end() / close() teardown. This also means the final pass opens and rewrites vmlinux once for .BTF_ids and then again for each --btf_link. Could patch_btfids() and patch_btf_link() share a helper that opens the ELF and finds a section by name, or even use a single RDWR descriptor? [ ... ]
quoted hunk ↗ jump to hunk
@@ -1916,12 +2125,30 @@ int main(int argc, const char **argv)
[ ... ]
quoted hunk ↗ jump to hunk
obj.path = argv[0]; - if (btfids_path) - return patch_btfids(btfids_path, obj.path); + if (btfids_path) { + err = patch_btfids(btfids_path, obj.path); + if (err || !btf_links.cnt) + goto out; + if (elf_collect(&obj)) { + err = -EINVAL; + goto out; + }
this isn't a bug, but the --btf_link path runs the whole elf_collect() pass only to get EI_CLASS and EI_DATA for patch_btf_link(). elf_collect() opens a second descriptor and mmap of the target, walks every section, and byte-swaps .BTF_ids in its private map. patch_btf_link() then opens the same file again with its own Elf handle. Since patch_btf_link() already calls elf_begin(), could it read the class and data encoding directly with gelf_getehdr() / gelf_getclass(), so that this elf_collect() call and the new efile.elf_class field aren't needed? --- AI reviewed your patch. Please fix the bug or email reply why it's not a bug. See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md CI run summary: https://github.com/kernel-patches/bpf/actions/runs/37924786344