Re: [PATCH v3 bpf-next 07/11] bpftool: Handle multi-split BTF by supporting multiple base BTFs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] bpf: evaluation of freed pointer in bpftool cleanup
- [Low] bpf: unbalanced braces in bpftool btf help string
--

commit 43f26ee1c8d2e52438682ffa46a3b61f16ff7512
Author: Alan Maguire <alan.maguire@oracle.com>

bpftool: Handle multi-split BTF by supporting multiple base BTFs

This commit updates bpftool to support multi-split BTF by allowing multiple
base BTF files to be specified via the -B argument. It updates the help text
and handles loading and freeing multiple nested BTF bases.

> diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
> index bca0a3982f09b..bbe8f9ea144fa 100644
> --- a/tools/bpf/bpftool/btf.c
> +++ b/tools/bpf/bpftool/btf.c
> @@ -1545,7 +1545,7 @@ static int do_help(int argc, char **argv)
>  		"       " HELP_SPEC_MAP "\n"
>  		"       " HELP_SPEC_PROGRAM "\n"
>  		"       " HELP_SPEC_OPTIONS " |\n"
> -		"                    {-B|--base-btf} }\n"
> +		"                    {[{-B|--base-btf} FILE]... }\n"

[Severity: Low]
Does this leave the opening brace from the HELP_SPEC_OPTIONS block unclosed?

The HELP_SPEC_OPTIONS macro starts a brace-enclosed block, but replacing the
single closing brace with a balanced group {[...]} might leave the overarching
list unclosed when the help output is printed.

> diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
> index 5ababd8f7d0a0..cff76be7d2e86 100644
> --- a/tools/bpf/bpftool/main.c
> +++ b/tools/bpf/bpftool/main.c

[ ... ]

> @@ -567,7 +572,18 @@ int main(int argc, char **argv)
>  	if (json_output)
>  		jsonw_destroy(&json_wtr);
>  
> -	btf__free(base_btf);
> +	while (base_btf) {
> +		new_base_btf = (struct btf *)btf__base_btf(base_btf);
> +		btf__free(base_btf);
> +		/*
> +		 * Do not free base BTF that is an owned .BTF.base ; leads
> +		 * to a double-free, so only free as far as the root base
> +		 * we explicitly read with -B above.
> +		 */
> +		if (base_btf == root_base_btf)
> +			break;

[Severity: Low]
Are we evaluating a freed pointer in this break condition?

Since btf__free(base_btf) is called just before this check, evaluating
base_btf against root_base_btf could invoke undefined behavior by reading the
dangling pointer, even if it is just an address comparison in a teardown path.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916074118.1007116-1-alan.maguire@oracle.com?part=7