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

[Namhyung Kim <namhyung@kernel.org>]

On Sat, Oct 10, 2026 at 06:34:56PM +0000, sashiko-bot@kernel.org wrote:
> 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]);

Right, I'll fix.

Thanks,
Namhyung

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