[PATCH v8 3/6] module: use relative references for __ksymtab entries

Martijn Coenen maco at android.com
Mon Jun 25 18:56:46 AEST 2018


Hi Ard,

> --- a/arch/x86/include/asm/export.h
> +++ /dev/null
> @@ -1,5 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -#ifdef CONFIG_64BIT
> -#define KSYM_ALIGN 16
> -#endif

Why remove the 16-byte alignment here? I was working on some other
code and ran into this 16-byte alignment, but I'm not sure why it's
needed on x86_64 in the first place. Possibly because the ABI requires
the stack to be 16-byte aligned when using sse instructions?

Thanks,
Martijn

> -#include <asm-generic/export.h>
> diff --git a/include/asm-generic/export.h b/include/asm-generic/export.h
> index 719db1968d81..97ce606459ae 100644
> --- a/include/asm-generic/export.h
> +++ b/include/asm-generic/export.h
> @@ -5,12 +5,10 @@
>  #define KSYM_FUNC(x) x
>  #endif
>  #ifdef CONFIG_64BIT
> -#define __put .quad
>  #ifndef KSYM_ALIGN
>  #define KSYM_ALIGN 8
>  #endif
>  #else
> -#define __put .long
>  #ifndef KSYM_ALIGN
>  #define KSYM_ALIGN 4
>  #endif
> @@ -25,6 +23,16 @@
>  #define KSYM(name) name
>  #endif
>
> +.macro __put, val, name
> +#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
> +       .long   \val - ., \name - .
> +#elif defined(CONFIG_64BIT)
> +       .quad   \val, \name
> +#else
> +       .long   \val, \name
> +#endif
> +.endm
> +
>  /*
>   * note on .section use: @progbits vs %progbits nastiness doesn't matter,
>   * since we immediately emit into those sections anyway.
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index ab4711c63601..0a9328ea9dbd 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -280,6 +280,25 @@ unsigned long read_word_at_a_time(const void *addr)
>
>  #endif /* __KERNEL__ */
>
> +/*
> + * Force the compiler to emit 'sym' as a symbol, so that we can reference
> + * it from inline assembler. Necessary in case 'sym' could be inlined
> + * otherwise, or eliminated entirely due to lack of references that are
> + * visible to the compiler.
> + */
> +#define __ADDRESSABLE(sym) \
> +       static void * const __attribute__((section(".discard"), used))  \
> +               __PASTE(__addressable_##sym, __LINE__) = (void *)&sym;
> +
> +/**
> + * offset_to_ptr - convert a relative memory offset to an absolute pointer
> + * @off:       the address of the 32-bit offset value
> + */
> +static inline void *offset_to_ptr(const int *off)
> +{
> +       return (void *)((unsigned long)off + *off);
> +}
> +
>  #endif /* __ASSEMBLY__ */
>
>  #ifndef __optimize
> diff --git a/include/linux/export.h b/include/linux/export.h
> index 25005b55b079..04c78e6bfec9 100644
> --- a/include/linux/export.h
> +++ b/include/linux/export.h
> @@ -24,12 +24,6 @@
>  #define VMLINUX_SYMBOL_STR(x) __VMLINUX_SYMBOL_STR(x)
>
>  #ifndef __ASSEMBLY__
> -struct kernel_symbol
> -{
> -       unsigned long value;
> -       const char *name;
> -};
> -
>  #ifdef MODULE
>  extern struct module __this_module;
>  #define THIS_MODULE (&__this_module)
> @@ -60,17 +54,47 @@ extern struct module __this_module;
>  #define __CRC_SYMBOL(sym, sec)
>  #endif
>
> +#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
> +#include <linux/compiler.h>
> +/*
> + * Emit the ksymtab entry as a pair of relative references: this reduces
> + * the size by half on 64-bit architectures, and eliminates the need for
> + * absolute relocations that require runtime processing on relocatable
> + * kernels.
> + */
> +#define __KSYMTAB_ENTRY(sym, sec)                                      \
> +       __ADDRESSABLE(sym)                                              \
> +       asm("   .section \"___ksymtab" sec "+" #sym "\", \"a\"  \n"     \
> +           "   .balign 8                                       \n"     \
> +           VMLINUX_SYMBOL_STR(__ksymtab_##sym) ":              \n"     \
> +           "   .long " VMLINUX_SYMBOL_STR(sym) "- .            \n"     \
> +           "   .long " VMLINUX_SYMBOL_STR(__kstrtab_##sym) "- .\n"     \
> +           "   .previous                                       \n")
> +
> +struct kernel_symbol {
> +       int value_offset;
> +       int name_offset;
> +};
> +#else
> +#define __KSYMTAB_ENTRY(sym, sec)                                      \
> +       static const struct kernel_symbol __ksymtab_##sym               \
> +       __attribute__((section("___ksymtab" sec "+" #sym), used))       \
> +       = { (unsigned long)&sym, __kstrtab_##sym }
> +
> +struct kernel_symbol {
> +       unsigned long value;
> +       const char *name;
> +};
> +#endif
> +
>  /* For every exported symbol, place a struct in the __ksymtab section */
>  #define ___EXPORT_SYMBOL(sym, sec)                                     \
>         extern typeof(sym) sym;                                         \
>         __CRC_SYMBOL(sym, sec)                                          \
>         static const char __kstrtab_##sym[]                             \
> -       __attribute__((section("__ksymtab_strings"), aligned(1)))       \
> +       __attribute__((section("__ksymtab_strings"), used, aligned(1))) \
>         = VMLINUX_SYMBOL_STR(sym);                                      \
> -       static const struct kernel_symbol __ksymtab_##sym               \
> -       __used                                                          \
> -       __attribute__((section("___ksymtab" sec "+" #sym), used))       \
> -       = { (unsigned long)&sym, __kstrtab_##sym }
> +       __KSYMTAB_ENTRY(sym, sec)
>
>  #if defined(__DISABLE_EXPORTS)
>
> diff --git a/kernel/module.c b/kernel/module.c
> index ad2d420024f6..b4782cfbb79b 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -549,12 +549,30 @@ static bool check_symbol(const struct symsearch *syms,
>         return true;
>  }
>
> +static unsigned long kernel_symbol_value(const struct kernel_symbol *sym)
> +{
> +#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
> +       return (unsigned long)offset_to_ptr(&sym->value_offset);
> +#else
> +       return sym->value;
> +#endif
> +}
> +
> +static const char *kernel_symbol_name(const struct kernel_symbol *sym)
> +{
> +#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
> +       return offset_to_ptr(&sym->name_offset);
> +#else
> +       return sym->name;
> +#endif
> +}
> +
>  static int cmp_name(const void *va, const void *vb)
>  {
>         const char *a;
>         const struct kernel_symbol *b;
>         a = va; b = vb;
> -       return strcmp(a, b->name);
> +       return strcmp(a, kernel_symbol_name(b));
>  }
>
>  static bool find_symbol_in_section(const struct symsearch *syms,
> @@ -2198,7 +2216,7 @@ void *__symbol_get(const char *symbol)
>                 sym = NULL;
>         preempt_enable();
>
> -       return sym ? (void *)sym->value : NULL;
> +       return sym ? (void *)kernel_symbol_value(sym) : NULL;
>  }
>  EXPORT_SYMBOL_GPL(__symbol_get);
>
> @@ -2228,10 +2246,12 @@ static int verify_export_symbols(struct module *mod)
>
>         for (i = 0; i < ARRAY_SIZE(arr); i++) {
>                 for (s = arr[i].sym; s < arr[i].sym + arr[i].num; s++) {
> -                       if (find_symbol(s->name, &owner, NULL, true, false)) {
> +                       if (find_symbol(kernel_symbol_name(s), &owner, NULL,
> +                                       true, false)) {
>                                 pr_err("%s: exports duplicate symbol %s"
>                                        " (owned by %s)\n",
> -                                      mod->name, s->name, module_name(owner));
> +                                      mod->name, kernel_symbol_name(s),
> +                                      module_name(owner));
>                                 return -ENOEXEC;
>                         }
>                 }
> @@ -2280,7 +2300,7 @@ static int simplify_symbols(struct module *mod, const struct load_info *info)
>                         ksym = resolve_symbol_wait(mod, info, name);
>                         /* Ok if resolved.  */
>                         if (ksym && !IS_ERR(ksym)) {
> -                               sym[i].st_value = ksym->value;
> +                               sym[i].st_value = kernel_symbol_value(ksym);
>                                 break;
>                         }
>
> @@ -2540,7 +2560,7 @@ static int is_exported(const char *name, unsigned long value,
>                 ks = lookup_symbol(name, __start___ksymtab, __stop___ksymtab);
>         else
>                 ks = lookup_symbol(name, mod->syms, mod->syms + mod->num_syms);
> -       return ks != NULL && ks->value == value;
> +       return ks != NULL && kernel_symbol_value(ks) == value;
>  }
>
>  /* As per nm */
> --
> 2.15.1
>


More information about the Linuxppc-dev mailing list