Re: [PATCH v2 2/3] perf symbol: Add missing symsrc__destroy() for libbfd

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] perf: double free and UAF in libbfd fallback logic
--

commit 27e7b4b66a3038201c422641092c060f7b466328
Author: Namhyung Kim <namhyung@kernel.org>

perf symbol: Add missing symsrc__destroy() for libbfd

This commit aims to prevent memory leaks and symbol overwrites by destroying
any pending syms_ss when libbfd successfully loads symbols.

Closes: https://lore.kernel.org/r/sashiko-outbox-162540@kernel.org

> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 6e327eaec8c0e..5e63c3477570c 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -1925,6 +1925,10 @@ int dso__load(struct dso *dso, struct map *map)
>  			nsinfo__mountns_enter(dso__nsinfo(dso), &nsc);
>  
>  		if (bfdrc > 0) {
> +			if (syms_ss) {
> +				symsrc__destroy(syms_ss);
> +				syms_ss = NULL;
> +			}

[Severity: High]
Can this sequence lead to a use-after-free and double free later in the
function?

If syms_ss and runtime_ss were both set to point to the same symsrc object
in an earlier iteration of the candidate debug images loop, this block
destroys the object and clears syms_ss, but leaves runtime_ss pointing to
the now freed memory. The object also remains tracked in the ss_ array.

After the loop breaks, could the fallback logic in dso__load() resurrect the
destroyed object:

	if (runtime_ss && !syms_ss) {
		syms_ss = runtime_ss;
	}

This appears to lead to dso__load_sym() being called with the destroyed 
syms_ss object, which would cause a use-after-free and overwrite the 
successful ret = 1 result from BFD:

	if (syms_ss)
		ret = dso__load_sym(dso, map, syms_ss, runtime_ss, kmod);

Finally, since the destroyed object was never removed from the ss_ array, 
would the cleanup block at the end of the function trigger a double-free?

	for (; ss_pos > 0; ss_pos--)
		symsrc__destroy(&ss_[ss_pos - 1]);

>  			ret = 1;
>  			break;
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261010182439.9732-1-namhyung@kernel.org?part=2