Re: [PATCH] tools/bpf/bpftool: Fix vmlinux BTF leak in cgroup commands

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] bpftool map and struct_ops commands in batch mode cause a Use-After-Free due to missing NULL assignment of the btf_vmlinux global after freeing.
--

commit 0812ae583a5122b98ffb4e25bf0e712dcec5d4d5
Author: Yichong Chen <chenyichong@uniontech.com>

tools/bpf/bpftool: Fix vmlinux BTF leak in cgroup commands

This commit fixes a vmlinux BTF leak in the bpftool cgroup show and tree
commands. It ensures the BTF object is loaded only once during a tree walk
and properly released with the global pointer reset, preventing issues in
batch mode.

> diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
> index ec356deb27c9e..ce69d1e5468e3 100644
> --- a/tools/bpf/bpftool/cgroup.c
> +++ b/tools/bpf/bpftool/cgroup.c
> @@ -78,6 +78,13 @@ static unsigned int query_flags;
>  static struct btf *btf_vmlinux;
>  static __u32 btf_vmlinux_id;
>  
> +static void free_btf_vmlinux(void)
> +{
> +	btf__free(btf_vmlinux);
> +	btf_vmlinux = NULL;
> +	btf_vmlinux_id = 0;
> +}
> +

[Severity: High]
This isn't a bug introduced by this patch, but seeing your proper fix for
resetting the btf_vmlinux pointer here makes me wonder if batch mode is
still broken for other commands.

Do map and struct_ops commands have a use-after-free in batch mode?

Looking at tools/bpf/bpftool/map.c in do_dump(), it frees the global
btf_vmlinux but doesn't set it to NULL:

tools/bpf/bpftool/map.c:do_dump() {
    ...
exit_free:
    free(fds);
    btf__free(btf_vmlinux);
    return err;
}

In batch mode, a subsequent map command calling get_map_kv_btf() would see
a non-NULL dangling pointer and reuse it instead of reloading the BTF:

tools/bpf/bpftool/map.c:get_map_kv_btf() {
    ...
    if (!btf_vmlinux) {
        btf_vmlinux = libbpf_find_kernel_btf();
    ...
    *btf = btf_vmlinux;
}

The same pattern appears in tools/bpf/bpftool/struct_ops.c where
do_struct_ops() calls btf__free(btf_vmlinux) without setting it to NULL,
and get_btf_vmlinux() reuses it if it's not NULL.

Should these other files also be updated to correctly reset btf_vmlinux
to NULL when freeing, similar to your fix for cgroup commands?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/24357C69B4405079+20260617090117.280222-1-chenyichong@uniontech.com?part=1