Re: [PATCH 04/12] perf debuginfo: Let the user skip and disable debuginfod fetches

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] perf debuginfo: thread-unsafe setenv and config modification in fetch callback
--

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

perf debuginfo: Let the user skip and disable debuginfod fetches

When a fetch is in progress, it prints progress and allows users to abort
it using 's' to skip the current fetch or 'd' to additionally disable
debuginfod for the rest of the session.

> diff --git a/tools/perf/util/debuginfo.c b/tools/perf/util/debuginfo.c
> --- a/tools/perf/util/debuginfo.c
> +++ b/tools/perf/util/debuginfo.c
[ ... ]
> +static void debuginfod__cancel_key(int key)
> +{
> +	if (key == 's' || key == 'S') {
> +		debuginfod_fetch_cancelled = true;
> +		debuginfod__skipped("Skipping this debuginfod fetch, this build ID will not be fetched again in this session, press 'd' to also disable it for the other ones");
> +	} else if (key == 'd' || key == 'D') {
> +		debuginfod_fetch_cancelled = true;
> +		symbol_conf.debuginfod = false;
> +		/*
> +		 * libdwfl's own debuginfod client reads DEBUGINFOD_URLS
> +		 * in every query, so clear it to stop it as well.
> +		 */
> +		setenv("DEBUGINFOD_URLS", "", 1);
> +		if (perf_config__set_variable("core.debuginfod", "false"))

[Severity: High]
Are setenv() and perf_config__set_variable() safe to call here?

When running perf top, the background process_thread may fetch debuginfo
and invoke debuginfod_progress_fn(). If the user hits 'd' to cancel,
debuginfod__cancel_key() calls setenv() and perf_config__set_variable().

At the same time, the concurrent display_thread can call getenv() (e.g.
implicitly via libc functions) or perf_config() (e.g. via script_browse()).

This could cause a data race on environ and on the unprotected static global
parsing state variables in config.c, potentially leading to memory corruption
or a crash.

> +			debuginfod__skipped("Skipping this debuginfod fetch and disabling debuginfod for this session, run 'perf config core.debuginfod=false' to also disable it permanently");
> +		else if (config_exclusive_filename)
> +			debuginfod__skipped("Skipping this debuginfod fetch and disabling debuginfod for this session and in the configuration file");
> +		else
> +			debuginfod__skipped("Skipping this debuginfod fetch and disabling debuginfod for this session and in ~/.perfconfig");
> +	}
> +}

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