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

[Tanushree Shah <tshah@linux.ibm.com>]

Hello, Thanks for the review.
On 01/07/26 05:21, Namhyung Kim wrote:
> On Fri, Jun 26, 2026 at 09:40:53PM +0530, Tanushree Shah wrote:
>> The current kallsyms detection in dso__is_kallsyms() uses the
>> dso_binary_type enum which fixes the issue of kallsyms being cached in
>> the build-id cache for out-of-tree modules.
>>
>> However, during build-id injection in perf record/inject, dso_binary_type
>> has not been explicitly set yet,so dso__binary_type() returns
>> DSO_BINARY_TYPE__NOT_FOUND instead of DSO_BINARY_TYPE__KALLSYMS for the
>> kernel DSO. The current check then fails to identify it as kallsyms,
>> causing build-id symlinks to not be created in ~/.debug/.build-id/ and
>> perf archive to fail with "Cannot stat" errors.
>>
>> Steps to reproduce the issue:
>> 1. rm -rf ~/.debug/.build-id
>> 2. perf record sleep 1
>> 3. perf archive
>>
>> Fix by falling back to matching long_name against the known kallsyms
>> strings explicitly when binary_type is not yet set
>> (== DSO_BINARY_TYPE__NOT_FOUND). Use strcmp() for exact matching of
>> fixed names and strict validation for guest kallsyms with embedded PID
>> to prevent path traversal attacks.
>>
>> Fixes: ebf0b332732d ("perf dso: fix dso__is_kallsyms() check")
>> Signed-off-by: Tanushree Shah <tshah@linux.ibm.com>
>> ---
>> v2 -> v3: Replace strncmp() prefix matching with strcmp() for fixed
>>            kallsyms names and add is_guest_kallsyms_pid_name() to
>>            strictly validate guest kallsyms with PID format, preventing
>>            path traversal attacks.
>>
>> v1 -> v2: Rename DSO__NAME_GUEST_KALLSYMS to DSO__PREFIX_GUEST_KALLSYMS
>>            to reflect that it is a prefix, not a full name.
>>
>> v1: https://lore.kernel.org/all/20260410071225.708005-2-tshah@linux.ibm.com/
>>
>>   tools/perf/util/dso.h | 57 ++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 56 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
>> index ede691e9a249..8763e6f65316 100644
>> --- a/tools/perf/util/dso.h
>> +++ b/tools/perf/util/dso.h
>> @@ -9,6 +9,7 @@
>>   #include <stdbool.h>
>>   #include <stdio.h>
>>   #include <linux/bitops.h>
>> +#include <string.h>
>>   #include "build-id.h"
>>   #include "debuginfo.h"
>>   #include "mutex.h"
>> @@ -20,6 +21,40 @@ struct perf_env;
>>   
>>   #define DSO__NAME_KALLSYMS	"[kernel.kallsyms]"
>>   #define DSO__NAME_KCORE		"[kernel.kcore]"
>> +#define DSO__NAME_GUEST_KALLSYMS		"[guest.kernel.kallsyms]"
>> +#define DSO__NAME_GUEST_KALLSYMS_PID_PREFIX	"[guest.kernel.kallsyms."
>> +
>> +/*
>> + * Validate names of the form "[guest.kernel.kallsyms.<pid>]", where
>> + * <pid> is the PID of the guest VM and varies per guest, so it
>> + * cannot be matched with strcmp() against a fixed string.
>> + *
>> + * Every character after the fixed prefix must be a decimal digit,
>> + * with ']' immediately terminating the digit run and nothing
>> + * following it. This rules out '/', "..", or any other character
>> + * being smuggled into the name.
>> + */
>> +static inline bool is_guest_kallsyms_pid_name(const char *name)
>> +{
>> +	const size_t prefix_len = sizeof(DSO__NAME_GUEST_KALLSYMS_PID_PREFIX) - 1;
>> +	size_t digits;
>> +
>> +	if (strncmp(name, DSO__NAME_GUEST_KALLSYMS_PID_PREFIX, prefix_len) != 0)
>> +		return false;
>> +
>> +	digits = strspn(name + prefix_len, "0123456789");
>> +	if (digits == 0)
>> +		return false;
>> +
>> +	/* ']' must terminate the digit run, with nothing trailing it */
>> +	if (name[prefix_len + digits] != ']')
>> +		return false;
>> +
>> +	if (name[prefix_len + digits + 1] != '\0')
>> +		return false;
>> +
>> +	return true;
>> +}
>>   
>>   /**
>>    * enum dso_binary_type - The kind of DSO generally associated with a memory
>> @@ -914,8 +949,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);
> 
> I have to check its usage carefully but any chance dso__symtab_type(dso)
> instead produces better results?
> 
I did some additional logging and found that symtab_type is not getting 
set while running perf record, so it seems dso__symtab_type(dso) would 
likely have the same issue here.
> 
>> +	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)
> 
> I think the proper wrapper is dso__kernel().
> 
Thanks for the pointer. Will use this and send v4.
> 
>> +		return false;
>> +
>> +	name = RC_CHK_ACCESS(dso)->long_name;
> 
> And dso__long_name().

Sure, will use this and send v4.
> 
> Thanks,
> Namhyung
> 
> 
>> +	if (!name)
>> +		return false;
>> +
>> +	if (!strcmp(name, DSO__NAME_KALLSYMS))
>> +		return true;
>> +
>> +	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);
>> -- 
>> 2.47.3
>>
> 
Thanks
Tanushree Shah