Re: [PATCH v3] perf dso: Fix kallsyms DSO detection with fallback logic

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Use-After-Free vulnerability due to lockless access of `dso->long_name` in `dso__is_kallsyms`.
--

commit 9c2571a16a38ac5caaaeb737772bc4cd229c357c
Author: Tanushree Shah <tshah@linux.ibm.com>

perf dso: Fix kallsyms DSO detection with fallback logic

This commit fixes an issue where kallsyms build-id symlinks fail to be
created during perf record/inject because the dso_binary_type is not
yet explicitly set. It falls back to matching the long_name against
known kallsyms strings when the binary type is not found. A strict
validation function is also added for guest kallsyms to prevent path
traversal attacks.

> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
> index 2916b954a804b..884cca37753c4 100644
> --- a/tools/perf/util/dso.h
> +++ b/tools/perf/util/dso.h
[ ... ]
> @@ -924,8 +959,28 @@ static inline bool dso__is_kcore(const struct dso *dso)
>  static inline bool dso__is_kallsyms(const struct dso *dso)
>  {
>  	enum dso_binary_type bt = dso__binary_type(dso);
> +	const char *name;
> +
> +	if (bt == DSO_BINARY_TYPE__KALLSYMS || bt == DSO_BINARY_TYPE__GUEST_KALLSYMS)
> +		return true;
> +
> +	if (bt != DSO_BINARY_TYPE__NOT_FOUND)
> +		return false;
> +
> +	if (!RC_CHK_ACCESS(dso)->kernel)
> +		return false;
> +
> +	name = RC_CHK_ACCESS(dso)->long_name;
> +	if (!name)
> +		return false;
> +
> +	if (!strcmp(name, DSO__NAME_KALLSYMS))
> +		return true;

[Severity: High]
This isn't a bug introduced by this patch, but lockless access to 
dso->long_name is a systemic flaw that could lead to a use-after-free here.

If a concurrent thread modifies the DSO (e.g., the event processing thread in
perf top handling a ksymbol register event calls
machine__process_ksymbol_register() -> dso__set_long_name()), the old
long_name can be freed while the UI thread is executing dso__is_kallsyms()
and performing this strcmp().

Is it safe to expand the surface area of this pre-existing lockless string
dereference into a function that was previously safe?

> +
> +	if (!strcmp(name, DSO__NAME_GUEST_KALLSYMS))
> +		return true;
>  
> -	return bt == DSO_BINARY_TYPE__KALLSYMS || bt == DSO_BINARY_TYPE__GUEST_KALLSYMS;
> +	return is_guest_kallsyms_pid_name(name);
>  }
>  
>  bool dso__is_object_file(const struct dso *dso);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626161052.1024439-2-tshah@linux.ibm.com?part=1