From: Padmanabha Srinivasaiah <[email protected]>
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 <[email protected]>
---
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);
if (arch_syscall_match_sym_name(str, "sys_ni_syscall"))
return NULL;
--
2.17.1
Hi,
On Tue, Jul 20, 2021 at 3:16 PM <[email protected]> wrote:
>
> From: Padmanabha Srinivasaiah <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
On Mon, Jul 26, 2021 at 08:17:45AM -0700, Sami Tolvanen wrote:
> Hi,
>
> On Tue, Jul 20, 2021 at 3:16 PM <[email protected]> wrote:
> >
> > From: Padmanabha Srinivasaiah <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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
Thanks Sami for the input, yes have regenrated patch:
https://lore.kernel.org/lkml/[email protected]/T/#u