Re: [PATCH v10 16/18] perf env: Add helper to lazily compute the os_release

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `perf_env__os_release()` helper incorrectly assumes an uninitialized `os_release` implies 'live mode' and populates the target environment with the host's OS release. This breaks cross-platform analysis of older `perf.data` files or pipe-mode streams.
- [Medium] Lockless lazy initialization of `env->os_release` creates a TOCTOU race condition, leading to potential memory leaks and unprotected data races in multi-threaded scenarios.
--

commit 3468e6592e2930724952882eef7d315b977f7343
Author: Ian Rogers <irogers@google.com>

perf env: Add helper to lazily compute the os_release

This commit introduces a helper to lazily initialize the os_release in
perf_env. When the os_release is uninitialized, the helper assumes the tool
is running in live mode and populates the field using the host's uname.

> diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
> index f61e108646133..b4fb7420b604b 100644
> --- a/tools/perf/util/env.c
> +++ b/tools/perf/util/env.c
[ ... ]
> @@ -358,6 +358,27 @@ bool perf_arch_is_big_endian(const char *arch)
>  	return false;
>  }
>  
> +const char *perf_env__os_release(struct perf_env *env)
> +{
> +	struct utsname uts;
> +	int ret;
> +
> +	if (!env)
> +		return perf_version_string;
> +
> +	if (env->os_release)
> +		return env->os_release;

[Severity: Medium]
Is this safe from concurrent execution in multi-threaded scenarios?

If multiple threads find env->os_release as NULL simultaneously, could they
independently call uname(), allocate strings via strdup(), and unconditionally
overwrite the shared pointer, leaking the memory allocated by the other
racing threads?

> +
> +	/*
> +	 * The os_release is being accessed but wasn't initialized from a data
> +	 * file, assume this is 'live' mode and use the release from uname. If
> +	 * uname or strdup fails then use the current perf tool version.
> +	 */
> +	ret = uname(&uts);
> +	env->os_release = strdup(ret < 0 ? perf_version_string : uts.release);

[Severity: Medium]
Does this break cross-platform and offline analysis for older data files?

When processing a perf.data file from an older tool version that lacks an
os_release header, this helper assumes live mode and mutates the target
environment with the current host's OS release.

Could this cause offline data converters, like output_headers() in
tools/perf/util/data-convert-json.c, to incorrectly export the analysis
host's kernel version instead of the target system's version?

> +	return env->os_release ?: perf_version_string;
> +}

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