> resolve_btfids: Populate BTF link section
TL;DR: patch_btf_link() and main() in resolve_btfids mostly fail silently; a
kbuild vmlinux link step could stop with only "Error 234". Should pr_err()
messages be added, as patch_btfids() does?
> diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
> index dcd289d16faf..84cd9691f177 100644
> --- a/tools/bpf/resolve_btfids/main.c
> +++ b/tools/bpf/resolve_btfids/main.c
[ ... ]
> @@ -1036,6 +1095,110 @@ 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)
> +{
> [ ... ]
> + if (stat(link->btf_path, &st) || !S_ISREG(st.st_mode) ||
> + st.st_size <= 0 || st.st_size > UINT_MAX)
> + return -EINVAL;
> + raw_btf_size = st.st_size;
> + raw_btf_data = malloc(raw_btf_size);
> + if (!raw_btf_data)
> + return -ENOMEM;
> + btf_file = fopen(link->btf_path, "rb");
> + if (!btf_file)
> + goto out_data;
Should patch_btf_link() print a message when it fails?
Nearly every error path here returns without any diagnostic: the stat() and
S_ISREG() check, malloc(), fopen(), fread(), an unknown ELF class, a module
name that is too long, a section name that does not fit in section[], open(),
elf_begin(), elf_getshdrstrndx(), gelf_getshdr(), an unknown EI_DATA and
elf_update(). Only "section %s not found" and "section %s is not a writable
BTF link" call pr_err().
The sibling patch_btfids() calls pr_err() with strerror(errno) or
elf_errmsg() on the corresponding failures (open, elf_begin,
elf_getshdrstrndx, gelf_getshdr, stat, fopen, fread, elf_update), and
pr_err() prints regardless of verbosity.
> [ ... ]
> + if (!scn) {
> + pr_err("FAILED: section %s not found in %s\n", section,
> obj->path);
> + goto out_elf;
> + }
> [ ... ]
> + if (elf_update(elf, ELF_C_WRITE) >= 0)
> + err = 0;
> +out_elf:
> [ ... ]
[ ... ]
> @@ -1931,12 +2097,30 @@ int main(int argc, const char **argv)
> [ ... ]
> - 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;
> + }
> + for (u32 i = 0; i < btf_links.cnt; i++) {
> + struct btf_link *link = &btf_links.links[i];
>
> + err = patch_btf_link(&obj, link);
> + if (err)
> + break;
> + }
> + goto out;
> + }
main() does not print anything either when patch_btf_link() fails:
err = patch_btf_link(&obj, link);
if (err)
break;
...
goto out;
so resolve_btfids exits with status 234 (-EINVAL) or 255 (-1) and no
message.
This looks reachable from kbuild. f61c6417ac15 ("kbuild, btf: Expose
vmlinux inline BTF through sysfs") makes scripts/link-vmlinux.sh always pass:
--btf_link .BTF.inline:btf_vmlinux_inline:vmlinux.BTF.inline
when BTF_INLINE=m, while scripts/gen-btf.sh explicitly handles a missing
${ELF_FILE}.BTF.inline by running:
rm -f "${objtree}/vmlinux.BTF.inline"
In that case, or if writing the output fails, or if a module name is 56
characters or more on a 64-bit target, the vmlinux link step stops after
"BTFIDS vmlinux" with only make's "Error 234" and nothing that says which
check failed.
Could pr_err() messages be added, as patch_btfids() does, naming
link->btf_path, link->module, the section and obj->path?
---
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/37149955093
