On Tue, Jun 24, 2025 at 10:02 PM Adin Scannell <amscanne@xxxxxxxx> 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@xxxxxxxx>
> ---
> 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
>
