[PATCH v2 1/3] perf symbol: Fix symbol loading from libbfd

[PATCH v2 1/3] perf symbol: Fix symbol loading from libbfd

[Namhyung Kim]

The dso__load() should return a positive number (number of symbols
loaded) when it succeded to load symbols.  But dso__load_bfd_symbols()
only returns 0 and it's hard to check if it actually loaded symbols or
not.  Let's return the value of dso__has_symbol() and check it in
dso__load() so that it can return 1 for success.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/r/sashiko-outbox-162540@kernel.org
Fixes: eac9a4342e5447cad ("perf symbols: Try reading the symbol table with libbfd")
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/libbfd.c | 2 +-
 tools/perf/util/symbol.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/libbfd.c b/tools/perf/util/libbfd.c
index a907d7c1ac948808..a4dcb406c79e9cee 100644
--- a/tools/perf/util/libbfd.c
+++ b/tools/perf/util/libbfd.c
@@ -463,7 +463,7 @@ int dso__load_bfd_symbols(struct dso *dso, const char *debugfile)
 	symbols__fixup_duplicate(dso__symbols(dso));
 	dso__set_adjust_symbols(dso, true);
 
-	err = 0;
+	err = dso__has_symbols(dso);
 out_free:
 	free(symbols);
 out_close:
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 5d98888d068c741a..a394e742f9b18812 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1930,8 +1930,8 @@ int dso__load(struct dso *dso, struct map *map)
 		if (nsexit)
 			nsinfo__mountns_enter(dso__nsinfo(dso), &nsc);
 
-		if (bfdrc == 0) {
-			ret = 0;
+		if (bfdrc > 0) {
+			ret = 1;
 			break;
 		}
 
-- 
2.55.0

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

[Namhyung Kim]

When libbfd loaded symbols and there's a pending syms_ss, it'd be lost
or overwrite existing symbols.  Let's destroy it before breaking the
loop so that it can safely return from the function.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/r/sashiko-outbox-162540@kernel.org
Fixes: eac9a4342e5447cad ("perf symbols: Try reading the symbol table with libbfd")
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/symbol.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index a394e742f9b18812..93045123cbbefdbd 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1931,6 +1931,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;
+			}
 			ret = 1;
 			break;
 		}
-- 
2.55.0

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

[sashiko-bot]

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

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

[Namhyung Kim]

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

[PATCH v2 3/3] perf symbol: Set dso symtab type for libbfd

[Namhyung Kim]

It missed to set the dso symtab type when the symbols are loaded by
libbfd.  When it happens both syms_ss and runtime_ss may not be set
and won't call dso__load_sym().

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/symbol.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 93045123cbbefdbd..5c2ba61ff8d9de51 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1935,6 +1935,7 @@ int dso__load(struct dso *dso, struct map *map)
 				symsrc__destroy(syms_ss);
 				syms_ss = NULL;
 			}
+			dso__set_symtab_type(dso, symtab_type);
 			ret = 1;
 			break;
 		}
-- 
2.55.0

Re: [PATCH v2 3/3] perf symbol: Set dso symtab type for libbfd

[sashiko-bot]

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

commit 8791c82124897f35132a0fe029d993b2dcd4a6bc
Author: Namhyung Kim <namhyung@kernel.org>

perf symbol: Set dso symtab type for libbfd

It missed to set the dso symtab type when the symbols are loaded by
libbfd.  When it happens both syms_ss and runtime_ss may not be set
and won't call dso__load_sym().

> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 5e63c3477570c..debd0a66f9669 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -1929,6 +1929,7 @@ int dso__load(struct dso *dso, struct map *map)
>  				symsrc__destroy(syms_ss);
>  				syms_ss = NULL;
>  			}
> +			dso__set_symtab_type(dso, symtab_type);
>  			ret = 1;
>  			break;
>  		}

[Severity: High]
When dso__load_bfd_symbols() succeeds and sets ret = 1, this explicit
destruction of syms_ss does not remove the struct from the ss_ array because
ss_pos remains unmodified.

Does this lead to a double-free later in the cleanup loop?

tools/perf/util/symbol.c:dso__load() {
    ...
	for (; ss_pos > 0; ss_pos--)
		symsrc__destroy(&ss_[ss_pos - 1]);
    ...
}

Additionally, since runtime_ss is not cleared here, can execution incorrectly
fall through to dso__load_sym() and overwrite the successful ret = 1 value
from libbfd?

tools/perf/util/symbol.c:dso__load() {
    ...
	if (runtime_ss && !syms_ss) {
		syms_ss = runtime_ss;
	}

	/* We'll have to hope for the best */
	if (!runtime_ss && syms_ss)
		runtime_ss = syms_ss;

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

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

Re: [PATCH v2 3/3] perf symbol: Set dso symtab type for libbfd

[Namhyung Kim]

On Sat, Oct 10, 2026 at 06:34:40PM +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 state confusion in libbfd fallback path
> --
> 
> commit 8791c82124897f35132a0fe029d993b2dcd4a6bc
> Author: Namhyung Kim <namhyung@kernel.org>
> 
> perf symbol: Set dso symtab type for libbfd
> 
> It missed to set the dso symtab type when the symbols are loaded by
> libbfd.  When it happens both syms_ss and runtime_ss may not be set
> and won't call dso__load_sym().
> 
> > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> > index 5e63c3477570c..debd0a66f9669 100644
> > --- a/tools/perf/util/symbol.c
> > +++ b/tools/perf/util/symbol.c
> > @@ -1929,6 +1929,7 @@ int dso__load(struct dso *dso, struct map *map)
> >  				symsrc__destroy(syms_ss);
> >  				syms_ss = NULL;
> >  			}
> > +			dso__set_symtab_type(dso, symtab_type);
> >  			ret = 1;
> >  			break;
> >  		}
> 
> [Severity: High]
> When dso__load_bfd_symbols() succeeds and sets ret = 1, this explicit
> destruction of syms_ss does not remove the struct from the ss_ array because
> ss_pos remains unmodified.
> 
> Does this lead to a double-free later in the cleanup loop?
> 
> tools/perf/util/symbol.c:dso__load() {
>     ...
> 	for (; ss_pos > 0; ss_pos--)
> 		symsrc__destroy(&ss_[ss_pos - 1]);
>     ...
> }
> 
> Additionally, since runtime_ss is not cleared here, can execution incorrectly
> fall through to dso__load_sym() and overwrite the successful ret = 1 value
> from libbfd?

I don't expect it sets runtime_ss, but I'll make sure it's cleared.

Thanks,
Namhyung

> 
> tools/perf/util/symbol.c:dso__load() {
>     ...
> 	if (runtime_ss && !syms_ss) {
> 		syms_ss = runtime_ss;
> 	}
> 
> 	/* We'll have to hope for the best */
> 	if (!runtime_ss && syms_ss)
> 		runtime_ss = syms_ss;
> 
> 	if (syms_ss)
> 		ret = dso__load_sym(dso, map, syms_ss, runtime_ss, kmod);
>     ...
> }
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261010182439.9732-1-namhyung@kernel.org?part=3

Re: [PATCH v2 1/3] perf symbol: Fix symbol loading from libbfd

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf test: pe-file-parsing breaks due to dso__load_bfd_symbols() return value change
--

commit 5ad4f779ca47fdb7586a43ebe4942962c3038922
Author: Namhyung Kim <namhyung@kernel.org>

perf symbol: Fix symbol loading from libbfd

This commit modifies dso__load_bfd_symbols() to return a positive number
representing the number of loaded symbols instead of 0 on success. The caller
in dso__load() is also updated to check for this positive return value.

Closes: https://lore.kernel.org/r/sashiko-outbox-162540@kernel.org
Fixes: eac9a4342e5447cad ("perf symbols: Try reading the symbol table with libbfd")

> diff --git a/tools/perf/util/libbfd.c b/tools/perf/util/libbfd.c
> index 33dc6158b2b1f..c2462b54b0e56 100644
> --- a/tools/perf/util/libbfd.c
> +++ b/tools/perf/util/libbfd.c
> @@ -411,7 +411,7 @@ int dso__load_bfd_symbols(struct dso *dso, const char *debugfile)
>  	symbols__fixup_duplicate(dso__symbols(dso));
>  	dso__set_adjust_symbols(dso, true);
>  
> -	err = 0;
> +	err = dso__has_symbols(dso);

[Severity: Medium]
Does this change break the pe-file-parsing perf test?

Looking at the pe-file-parsing test in tools/perf/tests/pe-file-parsing.c
inside run_dir(), it still expects the old 0 return value for success:

    ret = dso__load_bfd_symbols(dso, debugfile);
    TEST_ASSERT_VAL("Failed to load symbols", ret == 0);

Since dso__has_symbols() evaluates to a positive number when symbols exist,
won't this cause the test suite to unconditionally fail when analyzing valid
PE files?

[ ... ]

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

Re: [PATCH v2 1/3] perf symbol: Fix symbol loading from libbfd

[Namhyung Kim]

On Sat, Oct 10, 2026 at 06:36:17PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] perf test: pe-file-parsing breaks due to dso__load_bfd_symbols() return value change
> --
> 
> commit 5ad4f779ca47fdb7586a43ebe4942962c3038922
> Author: Namhyung Kim <namhyung@kernel.org>
> 
> perf symbol: Fix symbol loading from libbfd
> 
> This commit modifies dso__load_bfd_symbols() to return a positive number
> representing the number of loaded symbols instead of 0 on success. The caller
> in dso__load() is also updated to check for this positive return value.
> 
> Closes: https://lore.kernel.org/r/sashiko-outbox-162540@kernel.org
> Fixes: eac9a4342e5447cad ("perf symbols: Try reading the symbol table with libbfd")
> 
> > diff --git a/tools/perf/util/libbfd.c b/tools/perf/util/libbfd.c
> > index 33dc6158b2b1f..c2462b54b0e56 100644
> > --- a/tools/perf/util/libbfd.c
> > +++ b/tools/perf/util/libbfd.c
> > @@ -411,7 +411,7 @@ int dso__load_bfd_symbols(struct dso *dso, const char *debugfile)
> >  	symbols__fixup_duplicate(dso__symbols(dso));
> >  	dso__set_adjust_symbols(dso, true);
> >  
> > -	err = 0;
> > +	err = dso__has_symbols(dso);
> 
> [Severity: Medium]
> Does this change break the pe-file-parsing perf test?
> 
> Looking at the pe-file-parsing test in tools/perf/tests/pe-file-parsing.c
> inside run_dir(), it still expects the old 0 return value for success:
> 
>     ret = dso__load_bfd_symbols(dso, debugfile);
>     TEST_ASSERT_VAL("Failed to load symbols", ret == 0);
> 
> Since dso__has_symbols() evaluates to a positive number when symbols exist,
> won't this cause the test suite to unconditionally fail when analyzing valid
> PE files?

Oops, completely missed that.  Will update.

Thanks,
Namhyung