linux-kernel - Re: [PATCH] powerpc/ftrace: fix syscall tracing on PPC64_ELF_ABI

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]

Date:   Tue, 6 Dec 2022 23:08:40 +0000
From:   Christophe Leroy <christophe.leroy@...roup.eu>
To:     Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Michael Ellerman <mpe@...erman.id.au>,
        Michael Jeanson <mjeanson@...icios.com>,
        Steven Rostedt <rostedt@...dmis.org>
CC:     "stable@...r.kernel.org" <stable@...r.kernel.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Nicholas Piggin <npiggin@...il.com>,
        Michal Suchanek <msuchanek@...e.de>,
        "linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] powerpc/ftrace: fix syscall tracing on PPC64_ELF_ABI_V1



Le 06/12/2022 à 15:38, Mathieu Desnoyers a écrit :
> On 2022-12-05 17:50, Michael Ellerman wrote:
>> Michael Jeanson <mjeanson@...icios.com> writes:
>>> On 2022-12-05 15:11, Michael Jeanson wrote:
>>>>>>> Michael Jeanson <mjeanson@...icios.com> writes:
>>>>>>>> In v5.7 the powerpc syscall entry/exit logic was rewritten in C, on
>>>>>>>> PPC64_ELF_ABI_V1 this resulted in the symbols in the syscall table
>>>>>>>> changing from their dot prefixed variant to the non-prefixed ones.
>>>>>>>>
>>>>>>>> Since ftrace prefixes a dot to the syscall names when matching 
>>>>>>>> them to
>>>>>>>> build its syscall event list, this resulted in no syscall events 
>>>>>>>> being
>>>>>>>> available.
>>>>>>>>
>>>>>>>> Remove the PPC64_ELF_ABI_V1 specific version of
>>>>>>>> arch_syscall_match_sym_name to have the same behavior across all 
>>>>>>>> powerpc
>>>>>>>> variants.
>>>>>>>
>>>>>>> This doesn't seem to work for me.
>>>>>>>
>>>>>>> Event with it applied I still don't see anything in
>>>>>>> /sys/kernel/debug/tracing/events/syscalls
>>>>>>>
>>>>>>> Did we break it in some other way recently?
>>>>>>>
>>>>>>> cheers
>>>
>>> I did some further testing, my config also enabled KALLSYMS_ALL, when 
>>> I remove
>>> it there is indeed no syscall events.
>>
>> Aha, OK that explains it I guess.
>>
>> I was using ppc64_guest_defconfig which has ABI_V1 and FTRACE_SYSCALLS,
>> but does not have KALLSYMS_ALL. So I guess there's some other bug
>> lurking in there.
> 
> I don't have the setup handy to validate it, but I suspect it is caused 
> by the way scripts/kallsyms.c:symbol_valid() checks whether a symbol 
> entry needs to be integrated into the assembler output when 
> --all-symbols is not specified. It only keeps symbols which addresses 
> are in the text range. On PPC64_ELF_ABI_V1, this means only the 
> dot-prefixed symbols will be kept (those point to the function begin), 
> leaving out the non-dot-prefixed symbols (those point to the function 
> descriptors).
> 
> So I see two possible solutions there: either we ensure that 
> FTRACE_SYSCALLS selects KALLSYMS_ALL on PPC64_ELF_ABI_V1, or we modify 
> scripts/kallsyms.c:symbol_valid() to also include function descriptor 
> symbols. This would mean accepting symbols pointing into the .opd ELF 
> section.
> 
> IMHO the second option would be better because it does not increase the 
> kernel image size as much as KALLSYMS_ALL.
> 

Yes, seems to be the best solution.

Maybe the only thing to do is to add a new range to text_ranges, 
something like (untested):

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 03fa07ad45d9..decf31c497f5 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -64,6 +64,7 @@ static unsigned long long relative_base;
  static struct addr_range text_ranges[] = {
  	{ "_stext",     "_etext"     },
  	{ "_sinittext", "_einittext" },
+	{ "__start_opd", "__end_opd" },
  };
  #define text_range_text     (&text_ranges[0])
  #define text_range_inittext (&text_ranges[1])

---
Christophe