| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAEf4BzYz9jBG7njY4Vsu53aspzfp+1B++SdY5zYya0Sq_PEP8w@mail.gmail.com>
Date: Fri, 5 Dec 2025 17:13:12 -0800
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Ihor Solodrai <ihor.solodrai@...ux.dev>
Cc: Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>,
Eduard Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>,
Yonghong Song <yonghong.song@...ux.dev>, John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>, Hao Luo <haoluo@...gle.com>,
Jiri Olsa <jolsa@...nel.org>, Andrew Morton <akpm@...ux-foundation.org>,
Nathan Chancellor <nathan@...nel.org>, Nicolas Schier <nsc@...nel.org>, Tejun Heo <tj@...nel.org>,
David Vernet <void@...ifault.com>, Andrea Righi <arighi@...dia.com>,
Changwoo Min <changwoo@...lia.com>, Shuah Khan <shuah@...nel.org>,
Nick Desaulniers <nick.desaulniers+lkml@...il.com>, Bill Wendling <morbo@...gle.com>,
Justin Stitt <justinstitt@...gle.com>, Alan Maguire <alan.maguire@...cle.com>,
Donglin Peng <dolinux.peng@...il.com>, bpf@...r.kernel.org, dwarves@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-kbuild@...r.kernel.org
Subject: Re: [PATCH bpf-next v3 6/6] resolve_btfids: change in-place update
with raw binary output
On Fri, Dec 5, 2025 at 2:36 PM Ihor Solodrai <ihor.solodrai@...ux.dev> wrote:
>
> Currently resolve_btfids updates .BTF_ids section of an ELF file
> in-place, based on the contents of provided BTF, usually within the
> same input file, and optionally a BTF base.
>
> Change resolve_btfids behavior to enable BTF transformations as part
> of its main operation. To achieve this, in-place ELF write in
> resolve_btfids is replaced with generation of the following binaries:
> * ${1}.BTF with .BTF section data
> * ${1}.BTF_ids with .BTF_ids section data if it existed in ${1}
> * ${1}.BTF.base with .BTF.base section data for out-of-tree modules
>
> The execution of resolve_btfids and consumption of its output is
> orchestrated by scripts/gen-btf.sh introduced in this patch.
>
> The motivation for emitting binary data is that it allows simplifying
> resolve_btfids implementation by delegating ELF update to the $OBJCOPY
> tool [1], which is already widely used across the codebase.
>
> There are two distinct paths for BTF generation and resolve_btfids
> application in the kernel build: for vmlinux and for kernel modules.
>
> For the vmlinux binary a .BTF section is added in a roundabout way to
> ensure correct linking. The patch doesn't change this approach, only
> the implementation is a little different.
>
> Before this patch it worked as follows:
>
> * pahole consumed .tmp_vmlinux1 [2] and added .BTF section with
> llvm-objcopy [3] to it
> * then everything except the .BTF section was stripped from .tmp_vmlinux1
> into a .tmp_vmlinux1.bpf.o object [2], later linked into vmlinux
> * resolve_btfids was executed later on vmlinux.unstripped [4],
> updating it in-place
>
> After this patch gen-btf.sh implements the following:
>
> * pahole consumes .tmp_vmlinux1 and produces a *detached* file with
> raw BTF data
> * resolve_btfids consumes .tmp_vmlinux1 and detached BTF to produce
> (potentially modified) .BTF, and .BTF_ids sections data
> * a .tmp_vmlinux1.bpf.o object is then produced with objcopy copying
> BTF output of resolve_btfids
> * .BTF_ids data gets embedded into vmlinux.unstripped in
> link-vmlinux.sh by objcopy --update-section
>
> For kernel modules, creating a special .bpf.o file is not necessary,
> and so embedding of sections data produced by resolve_btfids is
> straightforward with objcopy.
>
> With this patch an ELF file becomes effectively read-only within
> resolve_btfids, which allows deleting elf_update() call and satellite
> code (like compressed_section_fix [5]).
>
> Endianness handling of .BTF_ids data is also changed. Previously the
> "flags" part of the section was bswapped in sets_patch() [6], and then
> Elf_Type was modified before elf_update() to signal to libelf that
> bswap may be necessary. With this patch we explicitly bswap entire
> data buffer on load and on dump.
>
> [1] https://lore.kernel.org/bpf/131b4190-9c49-4f79-a99d-c00fac97fa44@linux.dev/
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/link-vmlinux.sh?h=v6.18#n110
> [3] https://git.kernel.org/pub/scm/devel/pahole/pahole.git/tree/btf_encoder.c?h=v1.31#n1803
> [4] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/link-vmlinux.sh?h=v6.18#n284
> [5] https://lore.kernel.org/bpf/20200819092342.259004-1-jolsa@kernel.org/
> [6] https://lore.kernel.org/bpf/cover.1707223196.git.vmalik@redhat.com/
>
> Signed-off-by: Ihor Solodrai <ihor.solodrai@...ux.dev>
> ---
> MAINTAINERS | 1 +
> scripts/Makefile.btf | 17 +-
> scripts/Makefile.modfinal | 5 +-
> scripts/Makefile.vmlinux | 2 +-
> scripts/gen-btf.sh | 157 ++++++++++++
> scripts/link-vmlinux.sh | 46 +---
> tools/bpf/resolve_btfids/main.c | 228 +++++++++++-------
> tools/testing/selftests/bpf/.gitignore | 3 +
> tools/testing/selftests/bpf/Makefile | 9 +-
> .../selftests/bpf/prog_tests/resolve_btfids.c | 4 +-
> 10 files changed, 338 insertions(+), 134 deletions(-)
> create mode 100755 scripts/gen-btf.sh
>
Overall it looks good, but I'd like another pair of eyes on this :)
See some more minore nits below as well.
pw-bot: cr
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e36689cd7cc7..fe6141c69708 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4673,6 +4673,7 @@ F: net/sched/act_bpf.c
> F: net/sched/cls_bpf.c
> F: samples/bpf/
> F: scripts/bpf_doc.py
> +F: scripts/gen-btf.sh
> F: scripts/Makefile.btf
> F: scripts/pahole-version.sh
> F: tools/bpf/
> diff --git a/scripts/Makefile.btf b/scripts/Makefile.btf
> index 7c1cd6c2ff75..d067e91049cb 100644
> --- a/scripts/Makefile.btf
> +++ b/scripts/Makefile.btf
> @@ -1,5 +1,10 @@
> # SPDX-License-Identifier: GPL-2.0
>
> +gen-btf-y =
> +gen-btf-$(CONFIG_DEBUG_INFO_BTF) = $(srctree)/scripts/gen-btf.sh
> +
> +export GEN_BTF := $(gen-btf-y)
> +
What's the point of GEN_BTF? It's just so that you don't have to have
$(srctree)/scripts/gen-btf.sh specified in three places? Between
obscure $(GEN_BTF) (and having to understand where it is set and how
it's exported) and explicit $(srctree)/scripts/gen-btf.sh in a few
places, I'd prefer the latter, as it is way more greppable and it's
not like we are going to rename or move this script frequently
> pahole-ver := $(CONFIG_PAHOLE_VERSION)
> pahole-flags-y :=
>
[...]
> @@ -371,7 +348,7 @@ static int elf_collect(struct object *obj)
>
> elf_version(EV_CURRENT);
>
> - elf = elf_begin(fd, ELF_C_RDWR_MMAP, NULL);
> + elf = elf_begin(fd, ELF_C_READ_MMAP_PRIVATE, NULL);
> if (!elf) {
> close(fd);
> pr_err("FAILED cannot create ELF descriptor: %s\n",
> @@ -434,21 +411,20 @@ static int elf_collect(struct object *obj)
> obj->efile.symbols_shndx = idx;
> obj->efile.strtabidx = sh.sh_link;
> } else if (!strcmp(name, BTF_IDS_SECTION)) {
> + /*
> + * If target endianness differs from host, we need to bswap32
> + * the .BTF_ids section data on load, because .BTF_ids has
> + * Elf_Type = ELF_T_BYTE, and so libelf returns data buffer in
> + * the target endiannes. We repeat this on dump.
gmail screams at me for "endianness"
> + */
> + if (obj->efile.encoding != ELFDATANATIVE) {
> + pr_debug("bswap_32 .BTF_ids data from target to host endianness\n");
> + bswap_32_data(data->d_buf, data->d_size);
this looks like a violation of ELF_C_READ_MMAP_PRIVATE promise, no?...
would it be too create a copy here? for simplicity we can just always
malloc() a copy, regardless of bswap(), it can never be a huge amount
of data
> + }
> obj->efile.idlist = data;
> obj->efile.idlist_shndx = idx;
> obj->efile.idlist_addr = sh.sh_addr;
> - } else if (!strcmp(name, BTF_BASE_ELF_SEC)) {
> - /* If a .BTF.base section is found, do not resolve
> - * BTF ids relative to vmlinux; resolve relative
> - * to the .BTF.base section instead. btf__parse_split()
> - * will take care of this once the base BTF it is
> - * passed is NULL.
> - */
> - obj->base_btf_path = NULL;
> }
> -
> - if (compressed_section_fix(elf, scn, &sh))
> - return -1;
> }
>
> return 0;
> @@ -552,6 +528,13 @@ static int symbols_collect(struct object *obj)
> return 0;
> }
>
> +static inline bool is_envvar_set(const char *var_name)
> +{
> + const char *value = getenv(var_name);
> +
> + return value && value[0] != '\0';
> +}
> +
leftovers?
> static int load_btf(struct object *obj)
> {
> struct btf *base_btf = NULL, *btf = NULL;
> @@ -578,6 +561,19 @@ static int load_btf(struct object *obj)
> obj->base_btf = base_btf;
> obj->btf = btf;
>
> + if (obj->base_btf && obj->distill_base) {
> + err = btf__distill_base(obj->btf, &base_btf, &btf);
> + if (err) {
> + pr_err("FAILED to distill base BTF: %s\n", strerror(errno));
> + goto out_err;
> + }
> +
> + btf__free(obj->btf);
> + btf__free(obj->base_btf);
> + obj->btf = btf;
> + obj->base_btf = base_btf;
> + }
> +
> return 0;
>
> out_err:
[...]
> +static int dump_raw_btf_ids(struct object *obj, const char *out_path)
> +{
> + Elf_Data *data = obj->efile.idlist;
> + int fd, err;
> +
> + if (!data || !data->d_buf) {
> + pr_debug("%s has no BTF_ids data to dump\n", obj->path);
> + return 0;
> + }
> +
> + /*
> + * If target endianness differs from host, we need to bswap32 the
> + * .BTF_ids section data before dumping so that the output is in
> + * target endianness.
> + */
> + if (obj->efile.encoding != ELFDATANATIVE) {
> + pr_debug("bswap_32 .BTF_ids data from host to target endianness\n");
> + bswap_32_data(data->d_buf, data->d_size);
same about modifying ELF data in-place for what is supposed to be read-only use
> + }
> +
> + err = dump_raw_data(out_path, data->d_buf, data->d_size);
> + if (err)
> + return -1;
> +
> + return 0;
> +}
> +
> +static int dump_raw_btf(struct btf *btf, const char *out_path)
> +{
> + const void *raw_btf_data;
> + u32 raw_btf_size;
> + int fd, err;
> +
> + raw_btf_data = btf__raw_data(btf, &raw_btf_size);
> + if (!raw_btf_data) {
> + pr_err("btf__raw_data() failed\n");
> + return -1;
> + }
did you check that libbpf does proper byte swap as well?
> +
> + err = dump_raw_data(out_path, raw_btf_data, raw_btf_size);
> + if (err)
> + return -1;
> +
> + return 0;
> +}
> +
> +static inline int make_out_path(char *buf, const char *in_path, const char *suffix)
> +{
> + int len = snprintf(buf, PATH_MAX, "%s%s", in_path, suffix);
nit: normally you pass buffer and its size as input arguments instead
of assuming and hard-coding common PATH_MAX constant in two separate
places
> +
> + if (len < 0 || len >= PATH_MAX) {
> + pr_err("Output path is too long: %s%s\n", in_path, suffix);
> + return -E2BIG;
> }
>
> - pr_debug("update %s for %s\n",
> - err >= 0 ? "ok" : "failed", obj->path);
> - return err < 0 ? -1 : 0;
> + return 0;
> }
>
> static const char * const resolve_btfids_usage[] = {
[...]
Powered by blists - more mailing lists