* [PATCH bpf v2] libbpf: fix possible use-after-free for externs
@ 2025-06-25 5:02 Adin Scannell
2025-06-25 19:28 ` Andrii Nakryiko
2025-06-25 19:40 ` patchwork-bot+netdevbpf
0 siblings, 2 replies; 3+ messages in thread
From: Adin Scannell @ 2025-06-25 5:02 UTC (permalink / raw)
To: bpf, andrii; +Cc: ast, Adin Scannell
The `name` field in `obj->externs` points into the BTF data at initial
open time. However, some functions may invalidate this after opening and
before loading (e.g. `bpf_map__set_value_size`), which results in
pointers into freed memory and undefined behavior.
The simplest solution is to simply `strdup` these strings, similar to
the `essent_name`, and free them at the same time.
In order to test this path, the `global_map_resize` BPF selftest is
modified slightly to ensure the presence of an extern, which causes this
test to fail prior to the fix. Given there isn't an obvious API or error
to test against, I opted to add this to the existing test as an aspect
of the resizing feature rather than duplicate the test.
Fixes: 9d0a23313b1a ("libbpf: Add capability for resizing datasec maps")
Signed-off-by: Adin Scannell <amscanne@meta.com>
---
tools/lib/bpf/libbpf.c | 10 +++++++---
.../selftests/bpf/progs/test_global_map_resize.c | 16 ++++++++++++++++
2 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index e9c641a2fb20..52e353368f58 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -597,7 +597,7 @@ struct extern_desc {
int sym_idx;
int btf_id;
int sec_btf_id;
- const char *name;
+ char *name;
char *essent_name;
bool is_set;
bool is_weak;
@@ -4259,7 +4259,9 @@ static int bpf_object__collect_externs(struct bpf_object *obj)
return ext->btf_id;
}
t = btf__type_by_id(obj->btf, ext->btf_id);
- ext->name = btf__name_by_offset(obj->btf, t->name_off);
+ ext->name = strdup(btf__name_by_offset(obj->btf, t->name_off));
+ if (!ext->name)
+ return -ENOMEM;
ext->sym_idx = i;
ext->is_weak = ELF64_ST_BIND(sym->st_info) == STB_WEAK;
@@ -9138,8 +9140,10 @@ void bpf_object__close(struct bpf_object *obj)
zfree(&obj->btf_custom_path);
zfree(&obj->kconfig);
- for (i = 0; i < obj->nr_extern; i++)
+ for (i = 0; i < obj->nr_extern; i++) {
+ zfree(&obj->externs[i].name);
zfree(&obj->externs[i].essent_name);
+ }
zfree(&obj->externs);
obj->nr_extern = 0;
diff --git a/tools/testing/selftests/bpf/progs/test_global_map_resize.c b/tools/testing/selftests/bpf/progs/test_global_map_resize.c
index a3f220ba7025..ee65bad0436d 100644
--- a/tools/testing/selftests/bpf/progs/test_global_map_resize.c
+++ b/tools/testing/selftests/bpf/progs/test_global_map_resize.c
@@ -32,6 +32,16 @@ int my_int_last SEC(".data.array_not_last");
int percpu_arr[1] SEC(".data.percpu_arr");
+/* at least one extern is included, to ensure that a specific
+ * regression is tested whereby resizing resulted in a free-after-use
+ * bug after type information is invalidated by the resize operation.
+ *
+ * There isn't a particularly good API to test for this specific condition,
+ * but by having externs for the resizing tests it will cover this path.
+ */
+extern int LINUX_KERNEL_VERSION __kconfig;
+long version_sink;
+
SEC("tp/syscalls/sys_enter_getpid")
int bss_array_sum(void *ctx)
{
@@ -44,6 +54,9 @@ int bss_array_sum(void *ctx)
for (size_t i = 0; i < bss_array_len; ++i)
sum += array[i];
+ /* see above; ensure this is not optimized out */
+ version_sink = LINUX_KERNEL_VERSION;
+
return 0;
}
@@ -59,6 +72,9 @@ int data_array_sum(void *ctx)
for (size_t i = 0; i < data_array_len; ++i)
sum += my_array[i];
+ /* see above; ensure this is not optimized out */
+ version_sink = LINUX_KERNEL_VERSION;
+
return 0;
}
--
2.48.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH bpf v2] libbpf: fix possible use-after-free for externs
2025-06-25 5:02 [PATCH bpf v2] libbpf: fix possible use-after-free for externs Adin Scannell
@ 2025-06-25 19:28 ` Andrii Nakryiko
2025-06-25 19:40 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: Andrii Nakryiko @ 2025-06-25 19:28 UTC (permalink / raw)
To: Adin Scannell; +Cc: bpf, andrii, ast
On Tue, Jun 24, 2025 at 10:02 PM Adin Scannell <amscanne@meta.com> wrote:
>
> The `name` field in `obj->externs` points into the BTF data at initial
> open time. However, some functions may invalidate this after opening and
> before loading (e.g. `bpf_map__set_value_size`), which results in
> pointers into freed memory and undefined behavior.
>
> The simplest solution is to simply `strdup` these strings, similar to
> the `essent_name`, and free them at the same time.
>
> In order to test this path, the `global_map_resize` BPF selftest is
> modified slightly to ensure the presence of an extern, which causes this
> test to fail prior to the fix. Given there isn't an obvious API or error
> to test against, I opted to add this to the existing test as an aspect
> of the resizing feature rather than duplicate the test.
>
> Fixes: 9d0a23313b1a ("libbpf: Add capability for resizing datasec maps")
> Signed-off-by: Adin Scannell <amscanne@meta.com>
> ---
> tools/lib/bpf/libbpf.c | 10 +++++++---
> .../selftests/bpf/progs/test_global_map_resize.c | 16 ++++++++++++++++
> 2 files changed, 23 insertions(+), 3 deletions(-)
>
LGTM, thanks, applied to bpf tree. But for the future, please split
out selftests into separate patch from both libbpf and kernel changes.
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index e9c641a2fb20..52e353368f58 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -597,7 +597,7 @@ struct extern_desc {
> int sym_idx;
> int btf_id;
> int sec_btf_id;
> - const char *name;
> + char *name;
> char *essent_name;
> bool is_set;
> bool is_weak;
> @@ -4259,7 +4259,9 @@ static int bpf_object__collect_externs(struct bpf_object *obj)
> return ext->btf_id;
> }
> t = btf__type_by_id(obj->btf, ext->btf_id);
> - ext->name = btf__name_by_offset(obj->btf, t->name_off);
> + ext->name = strdup(btf__name_by_offset(obj->btf, t->name_off));
> + if (!ext->name)
> + return -ENOMEM;
> ext->sym_idx = i;
> ext->is_weak = ELF64_ST_BIND(sym->st_info) == STB_WEAK;
>
> @@ -9138,8 +9140,10 @@ void bpf_object__close(struct bpf_object *obj)
> zfree(&obj->btf_custom_path);
> zfree(&obj->kconfig);
>
> - for (i = 0; i < obj->nr_extern; i++)
> + for (i = 0; i < obj->nr_extern; i++) {
> + zfree(&obj->externs[i].name);
> zfree(&obj->externs[i].essent_name);
> + }
>
> zfree(&obj->externs);
> obj->nr_extern = 0;
> diff --git a/tools/testing/selftests/bpf/progs/test_global_map_resize.c b/tools/testing/selftests/bpf/progs/test_global_map_resize.c
> index a3f220ba7025..ee65bad0436d 100644
> --- a/tools/testing/selftests/bpf/progs/test_global_map_resize.c
> +++ b/tools/testing/selftests/bpf/progs/test_global_map_resize.c
> @@ -32,6 +32,16 @@ int my_int_last SEC(".data.array_not_last");
>
> int percpu_arr[1] SEC(".data.percpu_arr");
>
> +/* at least one extern is included, to ensure that a specific
> + * regression is tested whereby resizing resulted in a free-after-use
> + * bug after type information is invalidated by the resize operation.
> + *
> + * There isn't a particularly good API to test for this specific condition,
> + * but by having externs for the resizing tests it will cover this path.
> + */
> +extern int LINUX_KERNEL_VERSION __kconfig;
> +long version_sink;
> +
> SEC("tp/syscalls/sys_enter_getpid")
> int bss_array_sum(void *ctx)
> {
> @@ -44,6 +54,9 @@ int bss_array_sum(void *ctx)
> for (size_t i = 0; i < bss_array_len; ++i)
> sum += array[i];
>
> + /* see above; ensure this is not optimized out */
> + version_sink = LINUX_KERNEL_VERSION;
> +
> return 0;
> }
>
> @@ -59,6 +72,9 @@ int data_array_sum(void *ctx)
> for (size_t i = 0; i < data_array_len; ++i)
> sum += my_array[i];
>
> + /* see above; ensure this is not optimized out */
> + version_sink = LINUX_KERNEL_VERSION;
> +
> return 0;
> }
>
> --
> 2.48.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH bpf v2] libbpf: fix possible use-after-free for externs
2025-06-25 5:02 [PATCH bpf v2] libbpf: fix possible use-after-free for externs Adin Scannell
2025-06-25 19:28 ` Andrii Nakryiko
@ 2025-06-25 19:40 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-06-25 19:40 UTC (permalink / raw)
To: Adin Scannell; +Cc: bpf, andrii, ast
Hello:
This patch was applied to bpf/bpf.git (master)
by Andrii Nakryiko <andrii@kernel.org>:
On Tue, 24 Jun 2025 22:02:15 -0700 you wrote:
> The `name` field in `obj->externs` points into the BTF data at initial
> open time. However, some functions may invalidate this after opening and
> before loading (e.g. `bpf_map__set_value_size`), which results in
> pointers into freed memory and undefined behavior.
>
> The simplest solution is to simply `strdup` these strings, similar to
> the `essent_name`, and free them at the same time.
>
> [...]
Here is the summary with links:
- [bpf,v2] libbpf: fix possible use-after-free for externs
https://git.kernel.org/bpf/bpf/c/fa6f092cc0a0
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-06-25 19:39 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-25 5:02 [PATCH bpf v2] libbpf: fix possible use-after-free for externs Adin Scannell
2025-06-25 19:28 ` Andrii Nakryiko
2025-06-25 19:40 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox