linux-kernel - Re: [PATCH] tracing/syscalls: Clang cfi enable syscall events

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: <CABCJKuecCch5v32GJ8Bt9wEJOWtp2aYanRT7qdrD6BHx5qDRNw@mail.gmail.com>
Date:   Mon, 26 Jul 2021 08:17:45 -0700
From:   Sami Tolvanen <samitolvanen@...gle.com>
To:     treasure4paddy@...il.com
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Ingo Molnar <mingo@...hat.com>,
        Padmanabha Srinivasaiah <padmanabha.srinivasaiah@...man.com>,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        LKML <linux-kernel@...r.kernel.org>,
        clang-built-linux <clang-built-linux@...glegroups.com>
Subject: Re: [PATCH] tracing/syscalls: Clang cfi enable syscall events

Hi,

On Tue, Jul 20, 2021 at 3:16 PM <treasure4paddy@...il.com> wrote:
>
> From: Padmanabha Srinivasaiah <padmanabha.srinivasaiah@...man.com>
>
> With clang cfi syscall symbols are appended with <syscall>.cfi_jt,
> hence syscall tracer can not find corresponding syscall name.
> And results in no syscall ftrace events with CFI.
>
> To fix this issue, this introduces custom cleanup_syscall_symbol_name()
> to strip postfix ".cfi_jt" before comparing syscall and symbol name.
>
> Signed-off-by: Padmanabha Srinivasaiah <padmanabha.srinivasaiah@...man.com>
> ---
>  kernel/trace/trace_syscalls.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
> index 8bfcd3b09422..dbc253fea34d 100644
> --- a/kernel/trace/trace_syscalls.c
> +++ b/kernel/trace/trace_syscalls.c
> @@ -79,6 +79,27 @@ trace_get_syscall_nr(struct task_struct *task, struct pt_regs *regs)
>  }
>  #endif /* ARCH_TRACE_IGNORE_COMPAT_SYSCALLS */
>
> +#if defined(CONFIG_CFI_CLANG) && defined(CONFIG_LTO_CLANG_THIN)
> +/*
> + * For a syscall symbol, clang generated non-canonical local jump tables
> + * will have entry as <syscall>.cfi_jt and address of this entry
> + * will be used to replace references to the syscall symbol.
> + * so we will strip the postfix from appended symbol name.
> + */
> +static inline bool cleanup_syscall_symbol_name(char *s)
> +{
> +       char *res;
> +
> +       res = strrchr(s, '.');
> +       if (res)
> +               *res = '\0';
> +
> +       return res != NULL;
> +}
> +#else
> +static inline bool cleanup_syscall_symbol_name(char *s) { return false; }
> +#endif
> +
>  static __init struct syscall_metadata *
>  find_syscall_meta(unsigned long syscall)
>  {
> @@ -90,6 +111,7 @@ find_syscall_meta(unsigned long syscall)
>         start = __start_syscalls_metadata;
>         stop = __stop_syscalls_metadata;
>         kallsyms_lookup(syscall, NULL, NULL, NULL, str);
> +       cleanup_syscall_symbol_name(str);

Shouldn't this happen in the kallsyms code instead? We already have
code there to clean up function names, and it seems like we should
just strip the postfix there before returning the name.

Sami