Re: [PATCH v2 2/8] perf debuginfo: Fetch debuginfo keyed by build ID using debuginfod

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Time-of-Check to Time-of-Use (TOCTOU) race condition in debuginfo__find_build_id() leading to redundant network fetches and memory leaks.
--

commit a2b8e0baf731e115b13a42e3b82a0348a1a8851a
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf debuginfo: Fetch debuginfo keyed by build ID using debuginfod

This commit introduces the ability to fetch debuginfo keyed by build ID
using the debuginfod client when it is not found locally. It adds mechanisms
for progress indication, user cancellation, and caching misses to prevent
repeated network fetches.

> diff --git a/tools/perf/util/debuginfo.c b/tools/perf/util/debuginfo.c
> index 84a78b30ceac1..6f3023c96375c 100644
> --- a/tools/perf/util/debuginfo.c
> +++ b/tools/perf/util/debuginfo.c
[ ... ]
> @@ -139,6 +149,443 @@ struct debuginfo *debuginfo__new(const char *path)
[ ... ]
> +int debuginfo__find_build_id(const struct build_id *bid, char **path)
> +{
> +	int err;
> +
> +	*path = NULL;
> +
> +	if (!build_id__is_defined(bid) || !symbol_conf.debuginfod)
> +		return -1;
> +
> +	if (debuginfod__cache_disabled()) {
> +		pr_debug("Build-id cache disabled (buildid dir is '%s'), not using debuginfod\n",
> +			 buildid_dir);
> +		return -1;
> +	}
> +
> +	if (debuginfod__missed(bid)) {
> +		char sbuild_id[SBUILD_ID_SIZE];
> +
> +		build_id__snprintf(bid, sbuild_id, sizeof(sbuild_id));
> +		pr_debug("Not searching build ID %s in debuginfod again, it was a miss earlier\n",
> +			 sbuild_id);
> +		return -1;
> +	}
> +
> +	debuginfod__fetch_lock_init();
> +	mutex_lock(&debuginfod__fetch_lock);
> +	err = debuginfod__fetch(bid, path);
> +	mutex_unlock(&debuginfod__fetch_lock);

[Severity: Medium]
Could this introduce a time-of-check to time-of-use (TOCTOU) race condition?

If two threads concurrently call debuginfo__find_build_id() for the same
build ID, both could check debuginfod__missed(bid) and symbol_conf.debuginfod
outside of debuginfod__fetch_lock. 

After the first thread acquires the lock, completes the fetch, and records a
miss (or processes a user cancellation), the second thread will acquire the
lock and unconditionally perform the same fetch again without re-evaluating
the conditions. This could lead to redundant blocking network requests,
ignored cancellation requests, and duplicate entries added to the
debuginfod__misses list.

> +
> +	return err;
> +}

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