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

Thank you for the suggestion. The reason strcmp() was not used before 
was due to different variations of guest kallsyms. I further did a bit 
more analysis and identified that kallsyms can be in three formats:

"[kernel.kallsyms]"
"[guest.kernel.kallsyms]"
"[guest.kernel.kallsyms.<pid>]"

I will implement v3 based on this.

Thanks,
Tanushree Shah

On 25/06/26 01:24, Namhyung Kim wrote:
> On Thu, Jun 11, 2026 at 06:04:23PM +0530, Tanushree Shah wrote:
>>
>> On 16/04/26 16:39,[email protected] wrote:
>>> Sashiko AI review found 2 potential issue(s):
>>> - [High] Path Traversal and Information Disclosure due to `strncmp` matching any prefix.
>>> - [Medium] Incomplete fix: Fallback logic is missing from `dso__is_kcore()` and `dso__is_vmlinux()`.
>>> --
>>>
>>> commit 9ee80283f13c6a0a5235a2add7a94736c1dfcbba
>>> Author: Tanushree Shah<[email protected]>
>>>
>>> perf dso: Fix kallsyms DSO detection with fallback logic
>>>
>>> This commit fixes an issue where kallsyms DSOs are not correctly identified
>>> during build-id injection because the dso_binary_type is not yet set. It
>>> adds a fallback string matching check against the DSO's long_name.
>>>
>>>> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
>>>> index ede691e9a2495..515ab6e95b0e4 100644
>>>> --- a/tools/perf/util/dso.h
>>>> +++ b/tools/perf/util/dso.h
>>> [ ... ]
>>>
>>>> @@ -915,6 +917,14 @@ static inline bool dso__is_kallsyms(const struct dso *dso)
>>>>    {
>>>>    	enum dso_binary_type bt = dso__binary_type(dso);
>>>> +	if (bt == DSO_BINARY_TYPE__NOT_FOUND) {
>>> Since the fallback logic was added here because dso_binary_type(dso) is
>>> DSO_BINARY_TYPE__NOT_FOUND during perf record and inject, do dso__is_kcore()
>>> and dso__is_vmlinux() require a similar fallback?
>>>
>>> Looking at dso__cache_build_id(), it relies on dso__is_kcore() to flag the
>>> DSO for special caching logic. If dso__is_kcore() returns false during
>>> build-id caching, kcore DSOs are treated as regular files, which causes
>>> nsinfo__realpath() to fail and breaks perf archive for kcore sessions.
>>>
>>>> +		return RC_CHK_ACCESS(dso)->kernel &&
>>>> +			((strncmp(RC_CHK_ACCESS(dso)->long_name, DSO__NAME_KALLSYMS,
>>>> +				strlen(DSO__NAME_KALLSYMS)) == 0) ||
>>> Does using strncmp() here allow a path traversal if long_name is
>>> intentionally crafted?
>>>
>>> If a malicious perf.data file contains an MMAP event with a filename like
>>> "[kernel.kallsyms]/../../../../tmp/leak", this prefix check evaluates
>>> to true.
>>>
>>> Could this allow build_id_cache__cachedir() to construct a cache directory
>>> using this malicious path, causing build_id_cache__add() to unconditionally
>>> copy the host's /proc/kallsyms into an attacker-controlled directory?
>> Hello,
>> The current code relies on dso_binary_type to check if its a kallsyms dso.
>> While running perf archive, the code calls dso__is_kallsyms check to create
>> a build-id dir. But till that time, dso_binary_type is not getting set yet.
>> Hence, this patch introduced usage of dso->long_name to check if it is
>> kallsyms. But based on the above review from Sashiko, it seems that this may
>> create a security concern.
>>
>> I tried to check for the possibility to set the dso binary type before
>> dso__is_kallsyms gets called. But, I found that dso binary type is getting
>> set based on dso__is_kallsyms only.
>>
>> So, I am looking for suggestions/approach that we can take here.
> Can we simply check with strcmp() for the whole string?
> 
> Thanks,
> Namhyung
> 
>