From a2b11184389489dcca4fa899b290fc2751ea55ac Mon Sep 17 00:00:00 2001 From: Matthias Maennich Date: Fri, 18 Oct 2019 10:31:40 +0100 Subject: modpost: delegate updating namespaces to separate function Let the function 'sym_update_namespace' take care of updating the namespace for a symbol. While this currently only replaces one single location where namespaces are updated, in a following patch, this function will get more call sites. The function signature is intentionally close to sym_update_crc and taking the name by char* seems like unnecessary work as the symbol has to be looked up again. In a later patch of this series, this concern will be addressed. This function ensures that symbol::namespace is either NULL or has a valid non-empty value. Previously, the empty string was considered 'no namespace' as well and this lead to confusion. Acked-by: Will Deacon Reviewed-by: Greg Kroah-Hartman Reviewed-by: Masahiro Yamada Signed-off-by: Matthias Maennich Signed-off-by: Jessica Yu --- scripts/mod/modpost.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) (limited to 'scripts/mod/modpost.c') diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 936d3ad23c83..4041a265d2b6 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -362,6 +362,25 @@ static char *sym_extract_namespace(const char **symname) return namespace; } +static void sym_update_namespace(const char *symname, const char *namespace) +{ + struct symbol *s = find_symbol(symname); + + /* + * That symbol should have been created earlier and thus this is + * actually an assertion. + */ + if (!s) { + merror("Could not update namespace(%s) for symbol %s\n", + namespace, symname); + return; + } + + free(s->namespace); + s->namespace = + namespace && namespace[0] ? NOFAIL(strdup(namespace)) : NULL; +} + /** * Add an exported symbol - it may have already been added without a * CRC, in this case just update the CRC @@ -383,8 +402,7 @@ static struct symbol *sym_add_exported(const char *name, const char *namespace, s->module = mod; } } - free(s->namespace); - s->namespace = namespace ? strdup(namespace) : NULL; + sym_update_namespace(name, namespace); s->preloaded = 0; s->vmlinux = is_vmlinux(mod->name); s->kernel = 0; @@ -2196,7 +2214,7 @@ static int check_exports(struct module *mod) else basename = mod->name; - if (exp->namespace && exp->namespace[0]) { + if (exp->namespace) { add_namespace(&mod->required_namespaces, exp->namespace); -- cgit v1.2.3 From 9ae5bd1847566e079ffb7607394389ac54815f2b Mon Sep 17 00:00:00 2001 From: Matthias Maennich Date: Fri, 18 Oct 2019 10:31:41 +0100 Subject: modpost: make updating the symbol namespace explicit Setting the symbol namespace of a symbol within sym_add_exported feels displaced and lead to issues in the current implementation of symbol namespaces. This patch makes updating the namespace an explicit call to decouple it from adding a symbol to the export list. Acked-by: Will Deacon Reviewed-by: Greg Kroah-Hartman Reviewed-by: Masahiro Yamada Signed-off-by: Matthias Maennich Signed-off-by: Jessica Yu --- scripts/mod/modpost.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'scripts/mod/modpost.c') diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 4041a265d2b6..c43f1b1532f7 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -385,8 +385,8 @@ static void sym_update_namespace(const char *symname, const char *namespace) * Add an exported symbol - it may have already been added without a * CRC, in this case just update the CRC **/ -static struct symbol *sym_add_exported(const char *name, const char *namespace, - struct module *mod, enum export export) +static struct symbol *sym_add_exported(const char *name, struct module *mod, + enum export export) { struct symbol *s = find_symbol(name); @@ -402,7 +402,6 @@ static struct symbol *sym_add_exported(const char *name, const char *namespace, s->module = mod; } } - sym_update_namespace(name, namespace); s->preloaded = 0; s->vmlinux = is_vmlinux(mod->name); s->kernel = 0; @@ -764,7 +763,8 @@ static void handle_modversions(struct module *mod, struct elf_info *info, if (strstarts(symname, "__ksymtab_")) { name = symname + strlen("__ksymtab_"); namespace = sym_extract_namespace(&name); - sym_add_exported(name, namespace, mod, export); + sym_add_exported(name, mod, export); + sym_update_namespace(name, namespace); free(namespace); } if (strcmp(symname, "init_module") == 0) @@ -2472,12 +2472,12 @@ static void read_dump(const char *fname, unsigned int kernel) mod = new_module(modname); mod->skip = 1; } - s = sym_add_exported(symname, namespace, mod, - export_no(export)); + s = sym_add_exported(symname, mod, export_no(export)); s->kernel = kernel; s->preloaded = 1; s->is_static = 0; sym_update_crc(symname, mod, crc, export_no(export)); + sym_update_namespace(symname, namespace); } release_file(file, size); return; -- cgit v1.2.3 From 69923208431e097ce3830647aee98e5bd3e889c8 Mon Sep 17 00:00:00 2001 From: Matthias Maennich Date: Fri, 18 Oct 2019 10:31:42 +0100 Subject: symbol namespaces: revert to previous __ksymtab name scheme The introduction of 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_ 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. Fixes: 8651ec01daed ("module: add support for symbol namespaces.") Reported-by: Stefan Wahren Suggested-by: Masahiro Yamada Acked-by: Will Deacon Reviewed-by: Greg Kroah-Hartman Reviewed-by: Masahiro Yamada Signed-off-by: Matthias Maennich Signed-off-by: Jessica Yu --- include/linux/export.h | 14 +++++--------- scripts/mod/modpost.c | 33 ++++++++++++++++++--------------- scripts/mod/modpost.h | 1 + 3 files changed, 24 insertions(+), 24 deletions(-) (limited to 'scripts/mod/modpost.c') diff --git a/include/linux/export.h b/include/linux/export.h index 621158ecd2e2..941d075f03d6 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,16 +72,14 @@ 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 } #define __KSYMTAB_ENTRY(sym, sec) \ static const struct kernel_symbol __ksymtab_##sym \ - asm("__ksymtab_" #sym) \ __attribute__((section("___ksymtab" sec "+" #sym), used)) \ __aligned(sizeof(void *)) \ = { (unsigned long)&sym, __kstrtab_##sym, NULL } @@ -115,7 +111,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 c43f1b1532f7..d2a30a7b3f07 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) @@ -600,6 +593,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; @@ -689,7 +686,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")) @@ -762,10 +758,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; @@ -2061,6 +2054,16 @@ static void read_symbols(const char *modname) handle_moddevtable(mod, &info, sym, symname); } + /* Apply symbol namespaces from __kstrtabns_ 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; -- cgit v1.2.3