Re: [PATCH v12 17/19] perf env: Add helper to lazily compute the os_release

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Premature lazy initialization in pipe mode poisons `perf_env` with the host's `os_release`, corrupting JSON/CTF exports and offline symbol resolution.
--

commit 1b9df6eca5b172acf3ec3492723767468d3b5ec6
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
string in the perf environment, falling back to the host's release via
uname when the environment indicates live mode.

> diff --git a/tools/perf/util/data-convert-json.c b/tools/perf/util/data-convert-json.c
> --- a/tools/perf/util/data-convert-json.c
> +++ b/tools/perf/util/data-convert-json.c
[ ... ]
> @@ -296,7 +297,8 @@ static void output_headers(struct perf_session *session, struct convert_json *c)
>  	output_json_key_format(out, true, 2, "feat-offset", "%" PRIu64, header->feat_offset);
>  
>  	output_json_key_string(out, true, 2, "hostname", env->hostname);
> -	output_json_key_string(out, true, 2, "os-release", env->os_release);
> +	output_json_key_string(out, true, 2, "os-release",
> +			       perf_env__os_release(env));

[Severity: High]
In pipe mode, feature headers are received as interleaved events in the data
stream, so the environment is not fully populated until
perf_session__process_events() runs. 

Because output_headers() is called before the stream events are processed,
will calling perf_env__os_release() here trigger the uname fallback and
cause a regression that permanently writes the host's release string to
the json output instead of the guest's?

> diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
> --- a/tools/perf/util/env.c
> +++ b/tools/perf/util/env.c
[ ... ]
> @@ -361,6 +361,44 @@ bool perf_arch_is_big_endian(const char *arch)
[ ... ]
> +const char *perf_env__os_release(struct perf_env *env)
> +{
[ ... ]
> +	/*
> +	 * If env->arch is set, this is an offline target environment.
> +	 * If the os_release is not populated in the file, we do not want
> +	 * to poison it with the host's release which would break guest checks.
> +	 */
> +	if (env->arch) {
> +		release = NULL;
> +		goto out;
> +	}
> +
> +	/*
> +	 * 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: High]
Does this caching behavior cause a regression in offline symbol resolution
for unparsed pipe streams?

If the architecture string is not yet set because the pipe stream hasn't
been parsed yet, this helper assumes live mode and caches the host's release
in the environment structure. 

Could this cause early callers, such as vmlinux_path__init() during symbol
initialization, to search for the host's kernel symbols instead of the
target's?

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