Messages in this thread | | | From | Sami Tolvanen <> | Date | Thu, 12 Aug 2021 16:05:25 -0700 | Subject | Re: [PATCH v4] kallsyms: strip CLANG CFI postfix ".cfi_jt" |
| |
On Thu, Aug 5, 2021 at 4:28 PM Padmanabha Srinivasaiah <treasure4paddy@gmail.com> wrote: > > Clang CFI adds a postfix ".cfi_jt" to a symbols of extern functions. > For e.g. 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 v4: > - Remove redundant check; irrespective of LTO type (THIN/FULL), > LTO_CLANG will be always enabled. Hence will be used as entry flag > to check various postfix patterns. > - And prior to stripping postfix ".cfi_jt", added a comment to > justify why we are doing so. > > Change in v3: > - Modified commit message to indicate fix is for Clang CFI postfix > - Rebased on recent patch from ndesaulniers@google.com. > https://lore.kernel.org/lkml/ > 20210707181814.365496-1-ndesaulniers@google.com/#t > - 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 | 23 ++++++++++++++++++----- > 1 file changed, 18 insertions(+), 5 deletions(-) > > diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c > index 5cabe4dd3ff4..1b40bcf20fe6 100644 > --- a/kernel/kallsyms.c > +++ b/kernel/kallsyms.c > @@ -174,13 +174,15 @@ 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)) > return false; > > - res = strstr(s, ".llvm."); > - if (res) { > - *res = '\0'; > - return true; > + if (IS_ENABLED(CONFIG_LTO_CLANG_THIN)) { > + res = strstr(s, ".llvm."); > + if (res) { > + *res = '\0'; > + return true; > + } > }
I confirmed that LLVM renames these also with full LTO, so the config check can be dropped here.
> > /* > @@ -194,6 +196,17 @@ static bool cleanup_symbol_name(char *s) > return false; > > res = strrchr(s, '$'); > + if (!res) { > + /* > + * In case of non static function symbol <funcsym>, > + * the local jump table will have entry as <funcsym>.cfi_jt. > + * > + * Such expansion breaks some built-in components, > + * e.g. syscall tracer. Hence remove postfix ".cfi_jt". > + */ > + res = strstr(s, ".cfi_jt"); > + } > + > if (res) { > *res = '\0'; > return true;
Otherwise, the logic looks pretty good to me. Nick, are you planning to resend your earlier patch? Should this be just folded into the next version?
Sami
| |