BPF List
 help / color / mirror / Atom feed
* [PATCH] libbpf: fix double-free of distilled base BTF on .BTF.ext parse error
@ 2026-07-07 20:18 Naveed Khan
  2026-07-07 20:33 ` sashiko-bot
  0 siblings, 1 reply; 4+ messages in thread
From: Naveed Khan @ 2026-07-07 20:18 UTC (permalink / raw)
  To: bpf

When btf_parse_elf() is called without a caller-supplied base_btf (i.e.
via the public btf__parse_elf()) and the object file carries a .BTF.base
(distilled base) section, a dist_base_btf object is created and used as
the base of the split BTF built from the .BTF section.  Because base_btf
is NULL, the relocation block that would otherwise free and clear
dist_base_btf is skipped, and ownership of dist_base_btf is instead
transferred to the split btf by setting btf->owns_base = true.

That ownership transfer was performed before the fallible btf_ext__new()
call that parses the .BTF.ext section.  If .BTF.ext is malformed,
btf_ext__new() fails and the function jumps to the error path, which
frees dist_base_btf directly and then frees btf.  Since owns_base is
already set, btf__free(btf) also frees btf->base_btf, which is the same
dist_base_btf object.  The result is a use-after-free read followed by a
double free of the base BTF, driven entirely by a crafted object file
(a .BTF + .BTF.base + malformed .BTF.ext combination) passed to
btf__parse_elf(), as used by bpftool, pahole and similar tools.

Transfer ownership only after .BTF.ext has been parsed successfully, so
that any earlier failure leaves dist_base_btf owned solely by the local
cleanup path and it is freed exactly once.

Signed-off-by: Naveed Khan <naveed@digiscrypt.com>
---
diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 823bce8951..35695f33c6 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -1506,9 +1506,6 @@ static struct btf *btf_parse_elf(const char *path, struct btf *base_btf,
 		dist_base_btf = NULL;
 	}
 
-	if (dist_base_btf)
-		btf->owns_base = true;
-
 	switch (gelf_getclass(elf)) {
 	case ELFCLASS32:
 		btf__set_pointer_size(btf, 4);
@@ -1530,6 +1527,9 @@ static struct btf *btf_parse_elf(const char *path, struct btf *base_btf,
 	} else if (btf_ext) {
 		*btf_ext = NULL;
 	}
+
+	if (dist_base_btf)
+		btf->owns_base = true;
 done:
 	if (elf)
 		elf_end(elf);

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] libbpf: fix double-free of distilled base BTF on .BTF.ext parse error
  2026-07-07 20:18 [PATCH] libbpf: fix double-free of distilled base BTF on .BTF.ext parse error Naveed Khan
@ 2026-07-07 20:33 ` sashiko-bot
  2026-07-10 21:53   ` Andrii Nakryiko
  0 siblings, 1 reply; 4+ messages in thread
From: sashiko-bot @ 2026-07-07 20:33 UTC (permalink / raw)
  To: Naveed Khan; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] btf_parse_elf() incorrectly uses IS_ERR() to check the return value of the public API btf_ext__new(), rendering the error path and the patched double-free fix unreachable.
- [High] In `btf_parse_elf`, the out-parameter `btf_ext` is not initialized to `NULL` at the beginning of the function. If the function fails early, the error path reads the uninitialized pointer from the caller and passes it to `btf_ext__free()`, causing a crash.
--

--- Patch [1]: [PATCH] libbpf: fix double-free of distilled base BTF on .BTF.ext parse error ---
commit 343ec621fb0622b822c4fef99c23ee0947343232
Author: Naveed Khan <naveed@digiscrypt.com>
Subject: libbpf: fix double-free of distilled base BTF on .BTF.ext parse error

This patch fixes a double-free in the error path of btf_parse_elf(). By
deferring the ownership transfer of the distilled base BTF until after
.BTF.ext is parsed successfully, it ensures the base BTF is freed exactly
once on failure.

> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index 823bce895178d..35695f33c6942 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
[ ... ]
> @@ -1530,6 +1527,9 @@ static struct btf *btf_parse_elf(const char *path, struct btf *base_btf,
>  	} else if (btf_ext) {

[Severity: High]
This isn't a bug introduced by this patch, but immediately before this
else-if block, btf_ext__new() is called and checked using IS_ERR():

	*btf_ext = btf_ext__new(secs.btf_ext_data->d_buf, secs.btf_ext_data->d_size);
	if (IS_ERR(*btf_ext)) {
		err = PTR_ERR(*btf_ext);
		goto done;
	}

Since btf_ext__new() is a public libbpf API, doesn't it return NULL on
error rather than an ERR_PTR?

If IS_ERR(NULL) is false, the error path is never taken for malformed
.BTF.ext sections. Does this effectively make the double-free issue
that this patch fixes structurally unreachable?

>  		*btf_ext = NULL;
>  	}
> +
> +	if (dist_base_btf)
> +		btf->owns_base = true;
>  done:
>  	if (elf)
>  		elf_end(elf);

[Severity: High]
This is a pre-existing issue, but I have a question about the cleanup
path below this done label:

	if (btf_ext)
		btf_ext__free(*btf_ext);

If a caller provides an uninitialized pointer (e.g., struct btf_ext *ext;
btf__parse_elf(path, &ext);) and btf_parse_elf() fails early,
isn't the out-parameter *btf_ext left uninitialized?

Could this cause btf_ext__free() to read uninitialized memory from the
caller's stack and crash? Should *btf_ext be initialized to NULL at the
beginning of btf_parse_elf()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/178345549172.94179.7948304165383170781@digiscrypt.com?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] libbpf: fix double-free of distilled base BTF on .BTF.ext parse error
  2026-07-07 20:33 ` sashiko-bot
@ 2026-07-10 21:53   ` Andrii Nakryiko
  2026-07-11 16:48     ` [PATCH v2] libbpf: initialize btf_ext out-parameter early in btf_parse_elf() Naveed Khan
  0 siblings, 1 reply; 4+ messages in thread
From: Andrii Nakryiko @ 2026-07-10 21:53 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: Naveed Khan, bpf

On Tue, Jul 7, 2026 at 1:38 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] btf_parse_elf() incorrectly uses IS_ERR() to check the return value of the public API btf_ext__new(), rendering the error path and the patched double-free fix unreachable.
> - [High] In `btf_parse_elf`, the out-parameter `btf_ext` is not initialized to `NULL` at the beginning of the function. If the function fails early, the error path reads the uninitialized pointer from the caller and passes it to `btf_ext__free()`, causing a crash.
> --
>
> --- Patch [1]: [PATCH] libbpf: fix double-free of distilled base BTF on .BTF.ext parse error ---
> commit 343ec621fb0622b822c4fef99c23ee0947343232
> Author: Naveed Khan <naveed@digiscrypt.com>
> Subject: libbpf: fix double-free of distilled base BTF on .BTF.ext parse error
>
> This patch fixes a double-free in the error path of btf_parse_elf(). By
> deferring the ownership transfer of the distilled base BTF until after
> .BTF.ext is parsed successfully, it ensures the base BTF is freed exactly
> once on failure.
>
> > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> > index 823bce895178d..35695f33c6942 100644
> > --- a/tools/lib/bpf/btf.c
> > +++ b/tools/lib/bpf/btf.c
> [ ... ]
> > @@ -1530,6 +1527,9 @@ static struct btf *btf_parse_elf(const char *path, struct btf *base_btf,
> >       } else if (btf_ext) {
>
> [Severity: High]
> This isn't a bug introduced by this patch, but immediately before this
> else-if block, btf_ext__new() is called and checked using IS_ERR():
>
>         *btf_ext = btf_ext__new(secs.btf_ext_data->d_buf, secs.btf_ext_data->d_size);
>         if (IS_ERR(*btf_ext)) {
>                 err = PTR_ERR(*btf_ext);
>                 goto done;
>         }
>
> Since btf_ext__new() is a public libbpf API, doesn't it return NULL on
> error rather than an ERR_PTR?
>
> If IS_ERR(NULL) is false, the error path is never taken for malformed
> .BTF.ext sections. Does this effectively make the double-free issue
> that this patch fixes structurally unreachable?

this is a bug in existing code, IS_ERR() shouldn't be used, I fixed
this up while applying as well.

>
> >               *btf_ext = NULL;
> >       }
> > +
> > +     if (dist_base_btf)
> > +             btf->owns_base = true;
> >  done:
> >       if (elf)
> >               elf_end(elf);
>
> [Severity: High]
> This is a pre-existing issue, but I have a question about the cleanup
> path below this done label:
>
>         if (btf_ext)
>                 btf_ext__free(*btf_ext);
>
> If a caller provides an uninitialized pointer (e.g., struct btf_ext *ext;
> btf__parse_elf(path, &ext);) and btf_parse_elf() fails early,
> isn't the out-parameter *btf_ext left uninitialized?
>
> Could this cause btf_ext__free() to read uninitialized memory from the
> caller's stack and crash? Should *btf_ext be initialized to NULL at the
> beginning of btf_parse_elf()?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/178345549172.94179.7948304165383170781@digiscrypt.com?part=1
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2] libbpf: initialize btf_ext out-parameter early in btf_parse_elf()
  2026-07-10 21:53   ` Andrii Nakryiko
@ 2026-07-11 16:48     ` Naveed Khan
  0 siblings, 0 replies; 4+ messages in thread
From: Naveed Khan @ 2026-07-11 16:48 UTC (permalink / raw)
  To: bpf

btf_parse_elf() only stores to *btf_ext once it reaches the .BTF.ext
parsing step.  If the function fails before that point (open() aside,
e.g. elf_begin() failure, missing .BTF section, or a malformed .BTF /
.BTF.base section), the cleanup path still runs

	if (btf_ext)
		btf_ext__free(*btf_ext);

and reads a value that was never written.  A caller that passes an
uninitialized pointer, e.g.

	struct btf_ext *ext;
	btf = btf__parse_elf(path, &ext);

ends up with btf_ext__free() operating on stack garbage, which can
crash or corrupt the heap.  Current in-tree callers happen to
NULL-initialize their pointer, but the API contract should not rely
on that.

Initialize *btf_ext to NULL on entry so every exit path leaves the
out-parameter in a defined state.

Reported-by: sashiko-bot@kernel.org
Signed-off-by: Naveed Khan <naveed@digiscrypt.com>
---
>From 4dc7d1c8a2a2ec4c00ec038a94a86e26ab6cf75a Mon Sep 17 00:00:00 2001
From: Naveed Khan <naveed@digiscrypt.com>
Date: Sat, 11 Jul 2026 16:19:36 +0530
Subject: [PATCH] libbpf: initialize btf_ext out-parameter early in
 btf_parse_elf()

btf_parse_elf() only stores to *btf_ext once it reaches the .BTF.ext
parsing step.  If the function fails before that point (open() aside,
e.g. elf_begin() failure, missing .BTF section, or a malformed .BTF /
.BTF.base section), the cleanup path still runs

	if (btf_ext)
		btf_ext__free(*btf_ext);

and reads a value that was never written.  A caller that passes an
uninitialized pointer, e.g.

	struct btf_ext *ext;
	btf = btf__parse_elf(path, &ext);

ends up with btf_ext__free() operating on stack garbage, which can
crash or corrupt the heap.  Current in-tree callers happen to
NULL-initialize their pointer, but the API contract should not rely
on that.

Initialize *btf_ext to NULL on entry so every exit path leaves the
out-parameter in a defined state.

Reported-by: sashiko-bot@kernel.org
Signed-off-by: Naveed Khan <naveed@digiscrypt.com>
---
 tools/lib/bpf/btf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 823bce8951..cabe8286f0 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -1453,6 +1453,9 @@ static struct btf *btf_parse_elf(const char *path, struct btf *base_btf,
 	int err = 0, fd = -1;
 	Elf *elf = NULL;
 
+	if (btf_ext)
+		*btf_ext = NULL;
+
 	if (elf_version(EV_CURRENT) == EV_NONE) {
 		pr_warn("failed to init libelf for %s\n", path);
 		return ERR_PTR(-LIBBPF_ERRNO__LIBELF);
-- 
2.52.0

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-07-11 16:48 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-07 20:18 [PATCH] libbpf: fix double-free of distilled base BTF on .BTF.ext parse error Naveed Khan
2026-07-07 20:33 ` sashiko-bot
2026-07-10 21:53   ` Andrii Nakryiko
2026-07-11 16:48     ` [PATCH v2] libbpf: initialize btf_ext out-parameter early in btf_parse_elf() Naveed Khan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox