Messages in this thread | | | From | Sami Tolvanen <> | Date | Tue, 3 Aug 2021 09:28:23 -0700 | Subject | Re: [PATCH v3] kallsyms: strip CLANG CFI postfix ".cfi_jt" |
| |
Hi,
On Thu, Jul 29, 2021 at 1:54 PM Padmanabha Srinivasaiah <treasure4paddy@gmail.com> wrote: > > Clang CFI adds a postfix ".cfi_jt" to a symbols of extern functions. > For example this breaks syscall tracer that doesn't expect such postfix, > so strip out the postfix from the expanded symbol. > > Signed-off-by: Padmanabha Srinivasaiah <treasure4paddy@gmail.com> > --- > > Change in v3: > - Modified commit message to indicate fix is for Clang CFI postfix > - Rebased on recent patch from ndesaulniers@google.com. > - Fix is enabled even for CONFIG_LTO_CLANG > > Change in v2: > - Use existing routine in kallsyms to strip postfix ".cfi_jt" from > extern function name. > - Modified the commit message accordingly > > kernel/kallsyms.c | 21 +++++++++++++++------ > 1 file changed, 15 insertions(+), 6 deletions(-) > > diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c > index 5cabe4dd3ff4..67d015854cbd 100644 > --- a/kernel/kallsyms.c > +++ b/kernel/kallsyms.c > @@ -174,7 +174,8 @@ static bool cleanup_symbol_name(char *s) > * foo.llvm.974640843467629774. This can break hooking of static > * functions with kprobes. > */ > - if (!IS_ENABLED(CONFIG_LTO_CLANG_THIN)) > + if (!(IS_ENABLED(CONFIG_LTO_CLANG) || > + IS_ENABLED(CONFIG_LTO_CLANG_THIN)))
This is redundant. LTO_CLANG is selected for both LTO modes, so there's no need to also check for LTO_CLANG_THIN here.
> return false; > > res = strstr(s, ".llvm.");
However, we should probably check for ".llvm." only with LTO_CLANG_THIN.
> @@ -184,16 +185,24 @@ static bool cleanup_symbol_name(char *s) > } > > /* > - * LLVM appends a hash to static function names when ThinLTO and CFI > - * are both enabled, i.e. foo() becomes > - * foo$707af9a22804d33c81801f27dcfe489b. This causes confusion and > - * potentially breaks user space tools, so we strip the suffix from > - * expanded symbol names. > + * LLVM appends a hash to static function names when both > + * (Thin/FULL) LTO and CFI are enabled, i.e. foo() becomes > + * foo$707af9a22804d33c81801f27dcfe489b.
That's not quite right, the hash is only appended with ThinLTO. I would leave this comment untouched.
> + * > + * In case of non static function symbol <funcsym>, > + * the local jump table will have entry as <funcsym>.cfi_jt. > + * > + * This causes confusion and potentially breaks > + * user space tools and some built-in components. > + * So we strip the suffix from expanded symbol names. > */ > if (!IS_ENABLED(CONFIG_CFI_CLANG)) > return false; > > res = strrchr(s, '$'); > + if (!res) > + res = strstr(s, ".cfi_jt");
And add a comment about stripping .cfi_jt from jump table symbols before this part.
> + > if (res) { > *res = '\0'; > return true; > -- > 2.17.1 >
Sami
| |