Re: [PATCH v2 1/3] perf libdw: Fix Dwfl discovery with split files

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf libbfd: TOCTOU race populating cache with stale dso_name
--

commit 6a983b5d3f6eb6250b40233e72f23d9a6454ca97
Author: Ian Rogers <irogers@google.com>

perf libdw: Fix Dwfl discovery with split files

This commit updates Dwfl initialization to use the symsrc filename when
available, falling back to the long name. It drops the cached Dwfl and
addr2line cache when the symsrc filename is set, using the dso lock to
safely handle concurrent teardown and access.

> diff --git a/tools/perf/util/libbfd.c b/tools/perf/util/libbfd.c
> --- a/tools/perf/util/libbfd.c
> +++ b/tools/perf/util/libbfd.c
> @@ -226,7 +226,10 @@ int libbfd__addr2line(const char *dso_name, u64 addr,
>  		      struct symbol *sym)
>  {
>  	int ret = 0;
> -	struct a2l_data *a2l = dso__a2l(dso);
> +	struct a2l_data *a2l;
> +
> +	mutex_lock(dso__lock(dso));
> +	a2l = dso__a2l(dso);
>  
>  	if (!a2l) {
>  		a2l = addr2line_init(dso_name);
>  		dso__set_a2l(dso, a2l);
>  	}

[Severity: Medium]
Is there a Time-of-Check-to-Time-of-Use race here when populating the a2l
cache?

In __get_srcline(), dso_name is fetched before the lock is acquired in
libbfd__addr2line():

tools/perf/util/srcline.c:__get_srcline() {
    ...
    dso_name = srcline_dso_name(dso);
    if (dso_name == NULL)
        goto out_err;

    if (!addr2line(dso_name, addr, &file, &line, dso, ...
}

If another thread calls dso__set_symsrc_filename(), which updates the
debuginfo file and clears the cache, before libbfd__addr2line() acquires
its mutex:

Thread 1
dso__set_symsrc_filename()
    dso__free_a2l(dso);

Thread 2 would see a NULL cache in libbfd__addr2line() and populate it using
the stale dso_name argument:

    if (!a2l) {
        a2l = addr2line_init(dso_name);
        dso__set_a2l(dso, a2l);
    }

Could this permanently poison the cache with a BFD instance for a file
lacking debuginfo by not verifying it against the current symsrc_filename
under the lock?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916063545.3103314-1-irogers@google.com?part=1