linux-kernel - Re: [PATCH] perf symbol: ignore $a/$d symbols for ARM modules

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c7dfbd17-85fd-b914-b90f-082abc64c9d1@arm.com>
Date:   Wed, 27 Oct 2021 16:10:29 +0100
From:   James Clark <james.clark@....com>
To:     Lexi Shao <shaolexi@...wei.com>
Cc:     acme@...nel.org, alexander.shishkin@...ux.intel.com,
        jolsa@...hat.com, linux-kernel@...r.kernel.org,
        linux-perf-users@...r.kernel.org, mark.rutland@....com,
        mingo@...hat.com, namhyung@...nel.org, nixiaoming@...wei.com,
        peterz@...radead.org, qiuxi1@...wei.com, wangbing6@...wei.com
Subject: Re: [PATCH] perf symbol: ignore $a/$d symbols for ARM modules



On 27/10/2021 13:31, Lexi Shao wrote:
> On 27/10/2021 18:24, James Clark wrote:
>> On 27/10/2021 10:52, Lexi Shao wrote:
>>> On ARM machine, kernel symbols from modules can be resolved to $a
>>> instead of printing the actual symbol name. Ignore symbols starting with
>>> "$" when building kallsyms rbtree.
>>>
>>> A sample stacktrace is shown as follows:
>>>
>>> c0f2e39c schedule_hrtimeout+0x14 ([kernel.kallsyms])
>>> bf4a66d8 $a+0x78 ([test_module])
>>> c0a4f5f4 kthread+0x15c ([kernel.kallsyms])
>>> c0a001f8 ret_from_fork+0x14 ([kernel.kallsyms])
>>>
>>> On ARM machine, $a/$d symbols are used by the compiler to mark the
>>> beginning of code/data part in code section. These symbols are filtered
>>> out when linking vmlinux(see scripts/kallsyms.c ignored_prefixes), but
>>> are left on modules. So there are $a symbols in /proc/kallsyms which
>>> share the same addresses with the actual module symbols and confuses perf
>>> when resolving symbols.
>>
>> Hi Lexi,
>>
>> Is it worth using or re-implementing the entire is_ignored_symbol() function
>>from scripts/kallsyms.c? It seems like this change only fixes one occurrence,
>> but is_ignored_symbol() has a big list of other cases.
>>
>> Unless those cases are different?
>>
>> Thanks
>> James
> 
> Hi James,
> 
> I don't think it's necessary to cover all the cases listed in
> is_ignored_symbol(). As long as the symbols are unique and not overlapping
> with each other, they should't cause problems in resolving symbols. So most
> of the cases in is_ignored_symbol() should be irrelevant.

Is it possible to do the filtering of the module symbols somewhere else like
in kernel/kallsyms.c? I'm not that familiar with it but it seems like
at one point when populating kallsyms '$...' functions are filtered out, but
when modules are loaded the symbols are not filtered because another code path is
used?

If kallsyms has some duplicate addresses in there then isn't the bug in kallsyms
rather than perf? And another tool could get confused too.

James

> 
> Lexi
> 
>>
>>>
>>> After this patch, the module symbol name is printed:
>>>
>>> c0f2e39c schedule_hrtimeout+0x14 ([kernel.kallsyms])
>>> bf4a66d8 test_func+0x78 ([test_module])
>>> c0a4f5f4 kthread+0x15c ([kernel.kallsyms])
>>> c0a001f8 ret_from_fork+0x14 ([kernel.kallsyms])
>>>
>>> Signed-off-by: Lexi Shao <shaolexi@...wei.com>
>>> ---
>>>  tools/perf/util/symbol.c | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
>>> index 0fc9a5410739..35116aed74eb 100644
>>> --- a/tools/perf/util/symbol.c
>>> +++ b/tools/perf/util/symbol.c
>>> @@ -702,6 +702,10 @@ static int map__process_kallsym_symbol(void *arg, const char *name,
>>>  	if (!symbol_type__filter(type))
>>>  		return 0;
>>>  
>>> +	/* Ignore local symbols for ARM modules */
>>> +	if (name[0] == '$')
>>> +		return 0;
>>> +
>>>  	/*
>>>  	 * module symbols are not sorted so we add all
>>>  	 * symbols, setting length to 0, and rely on
>>>
>>