| Message ID | 20191010151443.7399-4-maennich@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | export/modpost: avoid renaming __ksymtab entries for symbol namespaces | expand |
On Thu, Oct 10, 2019 at 04:14:42PM +0100, Matthias Maennich wrote: > The introduction Symbol Namespaces changed the naming schema of the Missing "of" ? > __ksymtab entries from __kysmtab__symbol to __ksymtab_NAMESPACE.symbol. > > That caused some breakages in tools that depend on the name layout in > either the binaries(vmlinux,*.ko) or in System.map. E.g. kmod's depmod > would not be able to read System.map without a patch to support symbol > namespaces. A warning reported by depmod for namespaced symbols would > look like > > depmod: WARNING: [...]/uas.ko needs unknown symbol usb_stor_adjust_quirks > > In order to address this issue, revert to the original naming scheme and > rather read the __kstrtabns_<symbol> entries and their corresponding > values from __ksymtab_strings to update the namespace values for > symbols. After having read all symbols and handled them in > handle_modversions(), the symbols are created. In a second pass, read > the __kstrtabns_ entries and update the namespaces accordingly. > > Suggested-by: Jessica Yu <jeyu@kernel.org> > Fixes: 8651ec01daed ("module: add support for symbol namespaces.") > Signed-off-by: Matthias Maennich <maennich@google.com> > --- > include/linux/export.h | 13 +++++-------- > scripts/mod/modpost.c | 33 ++++++++++++++++++--------------- > scripts/mod/modpost.h | 1 + > 3 files changed, 24 insertions(+), 23 deletions(-) Patch looks fine, and it would be good to have this fixed in 5.4: Acked-by: Will Deacon <will@kernel.org> Will
On Thu, Oct 10, 2019 at 04:14:42PM +0100, Matthias Maennich wrote: > The introduction Symbol Namespaces changed the naming schema of the > __ksymtab entries from __kysmtab__symbol to __ksymtab_NAMESPACE.symbol. > > That caused some breakages in tools that depend on the name layout in > either the binaries(vmlinux,*.ko) or in System.map. E.g. kmod's depmod > would not be able to read System.map without a patch to support symbol > namespaces. A warning reported by depmod for namespaced symbols would > look like > > depmod: WARNING: [...]/uas.ko needs unknown symbol usb_stor_adjust_quirks > > In order to address this issue, revert to the original naming scheme and > rather read the __kstrtabns_<symbol> entries and their corresponding > values from __ksymtab_strings to update the namespace values for > symbols. After having read all symbols and handled them in > handle_modversions(), the symbols are created. In a second pass, read > the __kstrtabns_ entries and update the namespaces accordingly. > > Suggested-by: Jessica Yu <jeyu@kernel.org> > Fixes: 8651ec01daed ("module: add support for symbol namespaces.") > Signed-off-by: Matthias Maennich <maennich@google.com> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
On Fri, Oct 11, 2019 at 12:16 AM Matthias Maennich <maennich@google.com> wrote: > > The introduction Symbol Namespaces changed the naming schema of the > __ksymtab entries from __kysmtab__symbol to __ksymtab_NAMESPACE.symbol. > > That caused some breakages in tools that depend on the name layout in > either the binaries(vmlinux,*.ko) or in System.map. E.g. kmod's depmod > would not be able to read System.map without a patch to support symbol > namespaces. A warning reported by depmod for namespaced symbols would > look like > > depmod: WARNING: [...]/uas.ko needs unknown symbol usb_stor_adjust_quirks > > In order to address this issue, revert to the original naming scheme and > rather read the __kstrtabns_<symbol> entries and their corresponding > values from __ksymtab_strings to update the namespace values for > symbols. After having read all symbols and handled them in > handle_modversions(), the symbols are created. In a second pass, read > the __kstrtabns_ entries and update the namespaces accordingly. > > Suggested-by: Jessica Yu <jeyu@kernel.org> > Fixes: 8651ec01daed ("module: add support for symbol namespaces.") > Signed-off-by: Matthias Maennich <maennich@google.com> According to https://lore.kernel.org/patchwork/patch/1135222/ was this problem reported by Stefan? Reported-by: Stefan Wahren <stefan.wahren@i2se.com> BTW, I initially suggested this way of fixing. Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com> > @@ -74,9 +72,8 @@ struct kernel_symbol { > int namespace_offset; > }; > #else > -#define __KSYMTAB_ENTRY_NS(sym, sec, ns) \ > - static const struct kernel_symbol __ksymtab_##sym##__##ns \ > - asm("__ksymtab_" #ns NS_SEPARATOR #sym) \ For consistency, you could also delete asm("__ksymtab_" #sym) by this patch instead of by 4/4. Not a big deal, though. Reviewed-by: Masahiro Yamada <yamada.masahiro@socionext.com>
diff --git a/include/linux/export.h b/include/linux/export.h index 621158ecd2e2..f24b86d7dd4d 100644 --- a/include/linux/export.h +++ b/include/linux/export.h @@ -18,8 +18,6 @@ extern struct module __this_module; #define THIS_MODULE ((struct module *)0) #endif -#define NS_SEPARATOR "." - #ifdef CONFIG_MODVERSIONS /* Mark the CRC weak since genksyms apparently decides not to * generate a checksums for some symbols */ @@ -48,11 +46,11 @@ extern struct module __this_module; * absolute relocations that require runtime processing on relocatable * kernels. */ -#define __KSYMTAB_ENTRY_NS(sym, sec, ns) \ +#define __KSYMTAB_ENTRY_NS(sym, sec) \ __ADDRESSABLE(sym) \ asm(" .section \"___ksymtab" sec "+" #sym "\", \"a\" \n" \ " .balign 4 \n" \ - "__ksymtab_" #ns NS_SEPARATOR #sym ": \n" \ + "__ksymtab_" #sym ": \n" \ " .long " #sym "- . \n" \ " .long __kstrtab_" #sym "- . \n" \ " .long __kstrtabns_" #sym "- . \n" \ @@ -74,9 +72,8 @@ struct kernel_symbol { int namespace_offset; }; #else -#define __KSYMTAB_ENTRY_NS(sym, sec, ns) \ - static const struct kernel_symbol __ksymtab_##sym##__##ns \ - asm("__ksymtab_" #ns NS_SEPARATOR #sym) \ +#define __KSYMTAB_ENTRY_NS(sym, sec) \ + static const struct kernel_symbol __ksymtab_##sym \ __attribute__((section("___ksymtab" sec "+" #sym), used)) \ __aligned(sizeof(void *)) \ = { (unsigned long)&sym, __kstrtab_##sym, __kstrtabns_##sym } @@ -115,7 +112,7 @@ struct kernel_symbol { static const char __kstrtabns_##sym[] \ __attribute__((section("__ksymtab_strings"), used, aligned(1))) \ = #ns; \ - __KSYMTAB_ENTRY_NS(sym, sec, ns) + __KSYMTAB_ENTRY_NS(sym, sec) #define ___EXPORT_SYMBOL(sym, sec) \ ___export_symbol_common(sym, sec); \ diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 46137b730447..7cf0065ac95f 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -348,18 +348,11 @@ static enum export export_from_sec(struct elf_info *elf, unsigned int sec) return export_unknown; } -static char *sym_extract_namespace(const char **symname) +static const char *namespace_from_kstrtabns(struct elf_info *info, + Elf_Sym *kstrtabns) { - char *namespace = NULL; - char *ns_separator; - - ns_separator = strchr(*symname, '.'); - if (ns_separator) { - namespace = NOFAIL(strndup(*symname, ns_separator - *symname)); - *symname = ns_separator + 1; - } - - return namespace; + char *value = info->ksymtab_strings + kstrtabns->st_value; + return value[0] ? value : NULL; } static void sym_update_namespace(const char *symname, const char *namespace) @@ -597,6 +590,10 @@ static int parse_elf(struct elf_info *info, const char *filename) info->export_unused_gpl_sec = i; else if (strcmp(secname, "__ksymtab_gpl_future") == 0) info->export_gpl_future_sec = i; + else if (strcmp(secname, "__ksymtab_strings") == 0) + info->ksymtab_strings = (void *)hdr + + sechdrs[i].sh_offset - + sechdrs[i].sh_addr; if (sechdrs[i].sh_type == SHT_SYMTAB) { unsigned int sh_link_idx; @@ -686,7 +683,6 @@ static void handle_modversions(struct module *mod, struct elf_info *info, enum export export; bool is_crc = false; const char *name; - char *namespace; if ((!is_vmlinux(mod->name) || mod->is_dot_o) && strstarts(symname, "__ksymtab")) @@ -759,10 +755,7 @@ static void handle_modversions(struct module *mod, struct elf_info *info, /* All exported symbols */ if (strstarts(symname, "__ksymtab_")) { name = symname + strlen("__ksymtab_"); - namespace = sym_extract_namespace(&name); sym_add_exported(name, mod, export); - sym_update_namespace(name, namespace); - free(namespace); } if (strcmp(symname, "init_module") == 0) mod->has_init = 1; @@ -2058,6 +2051,16 @@ static void read_symbols(const char *modname) handle_moddevtable(mod, &info, sym, symname); } + /* Apply symbol namespaces from __kstrtabns_<symbol> entries. */ + for (sym = info.symtab_start; sym < info.symtab_stop; sym++) { + symname = remove_dot(info.strtab + sym->st_name); + + if (strstarts(symname, "__kstrtabns_")) + sym_update_namespace(symname + strlen("__kstrtabns_"), + namespace_from_kstrtabns(&info, + sym)); + } + // check for static EXPORT_SYMBOL_* functions && global vars for (sym = info.symtab_start; sym < info.symtab_stop; sym++) { unsigned char bind = ELF_ST_BIND(sym->st_info); diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h index 92a926d375d2..ad271bc6c313 100644 --- a/scripts/mod/modpost.h +++ b/scripts/mod/modpost.h @@ -143,6 +143,7 @@ struct elf_info { Elf_Section export_gpl_sec; Elf_Section export_unused_gpl_sec; Elf_Section export_gpl_future_sec; + char *ksymtab_strings; char *strtab; char *modinfo; unsigned int modinfo_len;
The introduction Symbol Namespaces changed the naming schema of the __ksymtab entries from __kysmtab__symbol to __ksymtab_NAMESPACE.symbol. That caused some breakages in tools that depend on the name layout in either the binaries(vmlinux,*.ko) or in System.map. E.g. kmod's depmod would not be able to read System.map without a patch to support symbol namespaces. A warning reported by depmod for namespaced symbols would look like depmod: WARNING: [...]/uas.ko needs unknown symbol usb_stor_adjust_quirks In order to address this issue, revert to the original naming scheme and rather read the __kstrtabns_<symbol> entries and their corresponding values from __ksymtab_strings to update the namespace values for symbols. After having read all symbols and handled them in handle_modversions(), the symbols are created. In a second pass, read the __kstrtabns_ entries and update the namespaces accordingly. Suggested-by: Jessica Yu <jeyu@kernel.org> Fixes: 8651ec01daed ("module: add support for symbol namespaces.") Signed-off-by: Matthias Maennich <maennich@google.com> --- include/linux/export.h | 13 +++++-------- scripts/mod/modpost.c | 33 ++++++++++++++++++--------------- scripts/mod/modpost.h | 1 + 3 files changed, 24 insertions(+), 23 deletions(-)