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

From: Martijn Coenen
Date: Mon Jun 25 2018 - 04:56:54 EST

Next message: Naga Sureshkumar Relli: "RE: [[LINUX PATCH v10] 3/4] Documentation: nand: pl353: Add documentation for controller and driver"
Previous message: Peter Zijlstra: "Re: [PATCH v12 10/11] sched: early boot clock"
Next in thread: Ard Biesheuvel: "Re: [PATCH v8 3/6] module: use relative references for __ksymtab entries"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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
>

Next message: Naga Sureshkumar Relli: "RE: [[LINUX PATCH v10] 3/4] Documentation: nand: pl353: Add documentation for controller and driver"
Previous message: Peter Zijlstra: "Re: [PATCH v12 10/11] sched: early boot clock"
Next in thread: Ard Biesheuvel: "Re: [PATCH v8 3/6] module: use relative references for __ksymtab entries"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]