Re: [PATCH v6 2/3] perf env: Add helper to lazily compute the os_release

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [Medium] Offline traces missing the OS_RELEASE header are polluted with the host's kernel version.
- [Medium] Unhandled `strdup()` memory allocation failure leads to NULL pointer dereference in `snprintf()`.
--

commit 291bf2cc1c91c985c162c953f24ca34e304792c1
Author: Ian Rogers <irogers@google.com>

perf env: Add helper to lazily compute the os_release

This commit adds a lazy initialization helper for the os_release string in
the perf environment, falling back to uname if it is not initialized.

> 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
[ ... ]
> @@ -330,6 +330,27 @@ int perf_env__kernel_is_64_bit(struct perf_env *env)
>  	return env->kernel_is_64_bit;
>  }
>  
> +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;
> +
> +	/*
> +	 * 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 fails then use the current perf tool version.
> +	 */
> +	ret = uname(&uts);
> +	env->os_release = strdup(ret < 0 ? perf_version_string : uts.release);
> +	return env->os_release;
> +}

If we are processing an offline perf.data file with a tool like perf report
or perf data convert, and the file is missing the HEADER_OSRELEASE feature,
env->os_release will legitimately be NULL.

Does this unconditional fallback to uname risk polluting the offline
trace's metadata with the local analysis host's kernel version?

This could potentially cause tools to incorrectly search for the target
vmlinux using the host's kernel version, or embed the host's release into
exported traces as the target's release.

> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
[ ... ]
> @@ -2226,7 +2226,7 @@ static int vmlinux_path__init(struct perf_env *env)
>  		return 0;
>  
>  	if (env) {
> -		kernel_version = env->os_release;
> +		kernel_version = perf_env__os_release(env);

If the strdup call in perf_env__os_release fails, it will return NULL.

Since kernel_version is assigned the result directly, could this result in
passing a NULL pointer to snprintf slightly further down in this function:

	snprintf(bf, sizeof(bf), vmlinux_paths_upd[i], kernel_version);

Passing NULL to a %s format specifier can lead to undefined behavior and
potential segmentation faults on certain C libraries like musl.

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