From 39808e451fdf30d20099a92e5185a0acb028d826 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Thu, 3 Oct 2019 19:29:14 +0900 Subject: kbuild: do not read $(KBUILD_EXTMOD)/Module.symvers Since commit 040fcc819a2e ("kbuild: improved modversioning support for external modules"), the external module build reads Module.symvers in the directory of the module itself, then dumps symbols back into it. It accumulates stale symbols in the file when you build an external module incrementally. The idea behind it was, as the commit log explained, you can copy Modules.symvers from one module to another when you need to pass symbol information between two modules. However, the manual copy of the file sounds questionable to me, and containing stale symbols is a downside. Some time later, commit 0d96fb20b7ed ("kbuild: Add new Kbuild variable KBUILD_EXTRA_SYMBOLS") introduced a saner approach. So, this commit removes the former one. Going forward, the external module build dumps symbols into Module.symvers to be carried via KBUILD_EXTRA_SYMBOLS, but never reads it automatically. With the -I option removed, there is no one to set the external_module flag unless KBUILD_EXTRA_SYMBOLS is passed. Now the -i option does it instead. Signed-off-by: Masahiro Yamada --- scripts/mod/modpost.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) (limited to 'scripts/mod/modpost.c') diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index d2a30a7b3f07..37fa1c65ee4d 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -2561,7 +2561,7 @@ int main(int argc, char **argv) { struct module *mod; struct buffer buf = { }; - char *kernel_read = NULL, *module_read = NULL; + char *kernel_read = NULL; char *dump_write = NULL, *files_source = NULL; int opt; int err; @@ -2569,13 +2569,10 @@ int main(int argc, char **argv) struct ext_sym_list *extsym_iter; struct ext_sym_list *extsym_start = NULL; - while ((opt = getopt(argc, argv, "i:I:e:mnsT:o:awEd")) != -1) { + while ((opt = getopt(argc, argv, "i:e:mnsT:o:awEd")) != -1) { switch (opt) { case 'i': kernel_read = optarg; - break; - case 'I': - module_read = optarg; external_module = 1; break; case 'e': @@ -2620,8 +2617,6 @@ int main(int argc, char **argv) if (kernel_read) read_dump(kernel_read, 1); - if (module_read) - read_dump(module_read, 0); while (extsym_start) { read_dump(extsym_start->file, 0); extsym_iter = extsym_start->next; -- cgit v1.2.3 From bff9c62b5d20d26f54bab81b33b6d9d1f9afcdf6 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Tue, 29 Oct 2019 21:38:06 +0900 Subject: modpost: do not invoke extra modpost for nsdeps 'make nsdeps' invokes the modpost three times at most; before linking vmlinux, before building modules, and finally for generating .ns_deps files. Running the modpost again and again is not efficient. The last two can be unified. When the -d option is given, the modpost still does the usual job, and in addition, generates .ns_deps files. Signed-off-by: Masahiro Yamada Tested-by: Matthias Maennich Reviewed-by: Matthias Maennich --- Makefile | 5 ++--- scripts/Makefile.modpost | 8 +++----- scripts/mod/modpost.c | 9 ++------- 3 files changed, 7 insertions(+), 15 deletions(-) (limited to 'scripts/mod/modpost.c') diff --git a/Makefile b/Makefile index 8a1e1ce15672..107a52d170ff 100644 --- a/Makefile +++ b/Makefile @@ -1684,10 +1684,9 @@ tags TAGS cscope gtags: FORCE # --------------------------------------------------------------------------- PHONY += nsdeps - +nsdeps: export KBUILD_NSDEPS=1 nsdeps: modules - $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost nsdeps - $(Q)$(CONFIG_SHELL) $(srctree)/scripts/$@ + $(Q)$(CONFIG_SHELL) $(srctree)/scripts/nsdeps # Scripts to check various things for consistency # --------------------------------------------------------------------------- diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost index 20359c7887b3..0089417760f9 100644 --- a/scripts/Makefile.modpost +++ b/scripts/Makefile.modpost @@ -53,8 +53,7 @@ MODPOST = scripts/mod/modpost \ $(if $(KBUILD_EXTMOD),$(addprefix -e ,$(KBUILD_EXTRA_SYMBOLS))) \ $(if $(KBUILD_EXTMOD),-o $(modulesymfile)) \ $(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E) \ - $(if $(KBUILD_MODPOST_WARN),-w) \ - $(if $(filter nsdeps,$(MAKECMDGOALS)),-d) + $(if $(KBUILD_MODPOST_WARN),-w) ifdef MODPOST_VMLINUX @@ -66,7 +65,8 @@ __modpost: else -MODPOST += $(subst -i,-n,$(filter -i,$(MAKEFLAGS))) -s -T - +MODPOST += $(subst -i,-n,$(filter -i,$(MAKEFLAGS))) -s -T - \ + $(if $(KBUILD_NSDEPS),-d) ifeq ($(KBUILD_EXTMOD),) MODPOST += $(wildcard vmlinux) @@ -96,8 +96,6 @@ ifneq ($(KBUILD_MODPOST_NOFINAL),1) $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modfinal endif -nsdeps: __modpost - endif .PHONY: $(PHONY) diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 37fa1c65ee4d..1de983d9a05d 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -2221,8 +2221,7 @@ static int check_exports(struct module *mod) add_namespace(&mod->required_namespaces, exp->namespace); - if (!write_namespace_deps && - !module_imports_namespace(mod, exp->namespace)) { + if (!module_imports_namespace(mod, exp->namespace)) { warn("module %s uses symbol %s from namespace %s, but does not import it.\n", basename, exp->name, exp->namespace); } @@ -2642,8 +2641,6 @@ int main(int argc, char **argv) err |= check_modname_len(mod); err |= check_exports(mod); - if (write_namespace_deps) - continue; add_header(&buf, mod); add_intree_flag(&buf, !external_module); @@ -2658,10 +2655,8 @@ int main(int argc, char **argv) write_if_changed(&buf, fname); } - if (write_namespace_deps) { + if (write_namespace_deps) write_namespace_deps_files(); - return 0; - } if (dump_write) write_dump(dump_write); -- cgit v1.2.3 From 0241ea8cae19b49fc1b1459f7bbe9a77f4f9cc89 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Thu, 7 Nov 2019 00:19:59 +0900 Subject: modpost: free ns_deps_buf.p after writing ns_deps files buf_write() allocates memory. Free it. Signed-off-by: Masahiro Yamada --- scripts/mod/modpost.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'scripts/mod/modpost.c') diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 1de983d9a05d..95f440d217e5 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -2549,6 +2549,8 @@ static void write_namespace_deps_files(void) sprintf(fname, "%s.ns_deps", mod->name); write_if_changed(&ns_deps_buf, fname); } + + free(ns_deps_buf.p); } struct ext_sym_list { -- cgit v1.2.3 From bbc55bded4aaf47d6f2bd9389fc8d3a3821d18c0 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Tue, 29 Oct 2019 21:38:07 +0900 Subject: modpost: dump missing namespaces into a single modules.nsdeps file The modpost, with the -d option given, generates per-module .ns_deps files. Kbuild generates per-module .mod files to carry module information. This is convenient because Make handles multiple jobs in parallel when the -j option is given. On the other hand, the modpost always runs as a single thread. I do not see a strong reason to produce separate .ns_deps files. This commit changes the modpost to generate just one file, modules.nsdeps, each line of which has the following format: : Please note it contains *missing* namespaces instead of required ones. So, modules.nsdeps is empty if the namespace dependency is all good. This will work more efficiently because spatch will no longer process already imported namespaces. I removed the '(if needed)' from the nsdeps log since spatch is invoked only when needed. This also solves the stale .ns_deps problem reported by Jessica Yu: https://lkml.org/lkml/2019/10/28/467 Signed-off-by: Masahiro Yamada Tested-by: Jessica Yu Acked-by: Jessica Yu Reviewed-by: Matthias Maennich Tested-by: Matthias Maennich --- .gitignore | 2 +- Documentation/dontdiff | 1 + Makefile | 4 ++-- scripts/Makefile.modpost | 2 +- scripts/mod/modpost.c | 42 +++++++++++++++++------------------------- scripts/mod/modpost.h | 4 ++-- scripts/nsdeps | 21 ++++++++++----------- 7 files changed, 34 insertions(+), 42 deletions(-) (limited to 'scripts/mod/modpost.c') diff --git a/.gitignore b/.gitignore index 70580bdd352c..72ef86a5570d 100644 --- a/.gitignore +++ b/.gitignore @@ -32,7 +32,6 @@ *.lzo *.mod *.mod.c -*.ns_deps *.o *.o.* *.patch @@ -61,6 +60,7 @@ modules.order /System.map /Module.markers /modules.builtin.modinfo +/modules.nsdeps # # RPM spec file (make rpm-pkg) diff --git a/Documentation/dontdiff b/Documentation/dontdiff index 9f4392876099..72fc2e9e2b63 100644 --- a/Documentation/dontdiff +++ b/Documentation/dontdiff @@ -179,6 +179,7 @@ mkutf8data modpost modules.builtin modules.builtin.modinfo +modules.nsdeps modules.order modversions.h* nconf diff --git a/Makefile b/Makefile index 107a52d170ff..084016405f69 100644 --- a/Makefile +++ b/Makefile @@ -1357,7 +1357,7 @@ endif # CONFIG_MODULES # Directories & files removed with 'make clean' CLEAN_DIRS += include/ksym -CLEAN_FILES += modules.builtin.modinfo +CLEAN_FILES += modules.builtin.modinfo modules.nsdeps # Directories & files removed with 'make mrproper' MRPROPER_DIRS += include/config include/generated \ @@ -1662,7 +1662,7 @@ clean: $(clean-dirs) -o -name '*.ko.*' \ -o -name '*.dtb' -o -name '*.dtb.S' -o -name '*.dt.yaml' \ -o -name '*.dwo' -o -name '*.lst' \ - -o -name '*.su' -o -name '*.mod' -o -name '*.ns_deps' \ + -o -name '*.su' -o -name '*.mod' \ -o -name '.*.d' -o -name '.*.tmp' -o -name '*.mod.c' \ -o -name '*.lex.c' -o -name '*.tab.[ch]' \ -o -name '*.asn1.[ch]' \ diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost index 0089417760f9..62e127028b48 100644 --- a/scripts/Makefile.modpost +++ b/scripts/Makefile.modpost @@ -66,7 +66,7 @@ __modpost: else MODPOST += $(subst -i,-n,$(filter -i,$(MAKEFLAGS))) -s -T - \ - $(if $(KBUILD_NSDEPS),-d) + $(if $(KBUILD_NSDEPS),-d modules.nsdeps) ifeq ($(KBUILD_EXTMOD),) MODPOST += $(wildcard vmlinux) diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 95f440d217e5..2cdbcdf197a3 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -38,8 +38,6 @@ static int sec_mismatch_count = 0; static int sec_mismatch_fatal = 0; /* ignore missing files */ static int ignore_missing_files; -/* write namespace dependencies */ -static int write_namespace_deps; enum export { export_plain, export_unused, export_gpl, @@ -2217,14 +2215,11 @@ static int check_exports(struct module *mod) else basename = mod->name; - if (exp->namespace) { - add_namespace(&mod->required_namespaces, - exp->namespace); - - if (!module_imports_namespace(mod, exp->namespace)) { - warn("module %s uses symbol %s from namespace %s, but does not import it.\n", - basename, exp->name, exp->namespace); - } + if (exp->namespace && + !module_imports_namespace(mod, exp->namespace)) { + warn("module %s uses symbol %s from namespace %s, but does not import it.\n", + basename, exp->name, exp->namespace); + add_namespace(&mod->missing_namespaces, exp->namespace); } if (!mod->gpl_compatible) @@ -2526,30 +2521,26 @@ static void write_dump(const char *fname) free(buf.p); } -static void write_namespace_deps_files(void) +static void write_namespace_deps_files(const char *fname) { struct module *mod; struct namespace_list *ns; struct buffer ns_deps_buf = {}; for (mod = modules; mod; mod = mod->next) { - char fname[PATH_MAX]; - if (mod->skip) + if (mod->skip || !mod->missing_namespaces) continue; - ns_deps_buf.pos = 0; - - for (ns = mod->required_namespaces; ns; ns = ns->next) - buf_printf(&ns_deps_buf, "%s\n", ns->namespace); + buf_printf(&ns_deps_buf, "%s.ko:", mod->name); - if (ns_deps_buf.pos == 0) - continue; + for (ns = mod->missing_namespaces; ns; ns = ns->next) + buf_printf(&ns_deps_buf, " %s", ns->namespace); - sprintf(fname, "%s.ns_deps", mod->name); - write_if_changed(&ns_deps_buf, fname); + buf_printf(&ns_deps_buf, "\n"); } + write_if_changed(&ns_deps_buf, fname); free(ns_deps_buf.p); } @@ -2563,6 +2554,7 @@ int main(int argc, char **argv) struct module *mod; struct buffer buf = { }; char *kernel_read = NULL; + char *missing_namespace_deps = NULL; char *dump_write = NULL, *files_source = NULL; int opt; int err; @@ -2570,7 +2562,7 @@ int main(int argc, char **argv) struct ext_sym_list *extsym_iter; struct ext_sym_list *extsym_start = NULL; - while ((opt = getopt(argc, argv, "i:e:mnsT:o:awEd")) != -1) { + while ((opt = getopt(argc, argv, "i:e:mnsT:o:awEd:")) != -1) { switch (opt) { case 'i': kernel_read = optarg; @@ -2609,7 +2601,7 @@ int main(int argc, char **argv) sec_mismatch_fatal = 1; break; case 'd': - write_namespace_deps = 1; + missing_namespace_deps = optarg; break; default: exit(1); @@ -2657,8 +2649,8 @@ int main(int argc, char **argv) write_if_changed(&buf, fname); } - if (write_namespace_deps) - write_namespace_deps_files(); + if (missing_namespace_deps) + write_namespace_deps_files(missing_namespace_deps); if (dump_write) write_dump(dump_write); diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h index ad271bc6c313..fe6652535e4b 100644 --- a/scripts/mod/modpost.h +++ b/scripts/mod/modpost.h @@ -126,8 +126,8 @@ struct module { struct buffer dev_table_buf; char srcversion[25]; int is_dot_o; - // Required namespace dependencies - struct namespace_list *required_namespaces; + // Missing namespace dependencies + struct namespace_list *missing_namespaces; // Actual imported namespaces struct namespace_list *imported_namespaces; }; diff --git a/scripts/nsdeps b/scripts/nsdeps index 04cea0921673..f802c6f7b860 100644 --- a/scripts/nsdeps +++ b/scripts/nsdeps @@ -27,15 +27,14 @@ generate_deps_for_ns() { } generate_deps() { - local mod_name=`basename $@ .ko` - local mod_file=`echo $@ | sed -e 's/\.ko/\.mod/'` - local ns_deps_file=`echo $@ | sed -e 's/\.ko/\.ns_deps/'` - if [ ! -f "$ns_deps_file" ]; then return; fi - local mod_source_files="`cat $mod_file | sed -n 1p \ + local mod=${1%.ko:} + shift + local namespaces="$*" + local mod_source_files="`cat $mod.mod | sed -n 1p \ | sed -e 's/\.o/\.c/g' \ | sed "s|[^ ]* *|${srctree}/&|g"`" - for ns in `cat $ns_deps_file`; do - echo "Adding namespace $ns to module $mod_name (if needed)." + for ns in $namespaces; do + echo "Adding namespace $ns to module $mod.ko." generate_deps_for_ns $ns "$mod_source_files" # sort the imports for source_file in $mod_source_files; do @@ -52,7 +51,7 @@ generate_deps() { done } -for f in `cat $objtree/modules.order`; do - generate_deps $f -done - +while read line +do + generate_deps $line +done < modules.nsdeps -- cgit v1.2.3 From 76b54cf033c9f2effc70066a2bbb2331013889a1 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Tue, 29 Oct 2019 21:38:09 +0900 Subject: modpost: remove unneeded local variable in contains_namespace() The local variable, ns_entry, is unneeded. While I was here, I also cleaned up the comparison with NULL or 0. Signed-off-by: Masahiro Yamada Reviewed-by: Matthias Maennich --- scripts/mod/modpost.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'scripts/mod/modpost.c') diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 2cdbcdf197a3..46d7f695fe7f 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -239,10 +239,8 @@ static struct symbol *find_symbol(const char *name) static bool contains_namespace(struct namespace_list *list, const char *namespace) { - struct namespace_list *ns_entry; - - for (ns_entry = list; ns_entry != NULL; ns_entry = ns_entry->next) - if (strcmp(ns_entry->namespace, namespace) == 0) + for (; list; list = list->next) + if (!strcmp(list->namespace, namespace)) return true; return false; -- cgit v1.2.3 From afa0459daa7b08c7b2c879705b69d39b734a11d0 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Fri, 15 Nov 2019 02:42:21 +0900 Subject: modpost: add a helper to get data pointed by a symbol When CONFIG_MODULE_REL_CRCS is enabled, the value of __crc_* is not an absolute value, but the address to the CRC data embedded in the .rodata section. Getting the data pointed by the symbol value is somewhat complex. Split it out into a new helper, sym_get_data(). I will reuse it to refactor namespace_from_kstrtabns() in the next commit. Signed-off-by: Masahiro Yamada --- scripts/mod/modpost.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) (limited to 'scripts/mod/modpost.c') diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 46d7f695fe7f..cd885573daaf 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -308,6 +308,18 @@ static const char *sec_name(struct elf_info *elf, int secindex) return sech_name(elf, &elf->sechdrs[secindex]); } +static void *sym_get_data(const struct elf_info *info, const Elf_Sym *sym) +{ + Elf_Shdr *sechdr = &info->sechdrs[sym->st_shndx]; + unsigned long offset; + + offset = sym->st_value; + if (info->hdr->e_type != ET_REL) + offset -= sechdr->sh_addr; + + return (void *)info->hdr + sechdr->sh_offset + offset; +} + #define strstarts(str, prefix) (strncmp(str, prefix, strlen(prefix)) == 0) static enum export export_from_secname(struct elf_info *elf, unsigned int sec) @@ -697,10 +709,7 @@ static void handle_modversions(struct module *mod, struct elf_info *info, unsigned int *crcp; /* symbol points to the CRC in the ELF object */ - crcp = (void *)info->hdr + sym->st_value + - info->sechdrs[sym->st_shndx].sh_offset - - (info->hdr->e_type != ET_REL ? - info->sechdrs[sym->st_shndx].sh_addr : 0); + crcp = sym_get_data(info, sym); crc = TO_NATIVE(*crcp); } sym_update_crc(symname + strlen("__crc_"), mod, crc, -- cgit v1.2.3 From e84f9fbbece1585f45a03ccc11eeabe121cadc1b Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Fri, 15 Nov 2019 02:42:22 +0900 Subject: modpost: refactor namespace_from_kstrtabns() to not hard-code section name Currently, namespace_from_kstrtabns() relies on the fact that namespace strings are recorded in the __ksymtab_strings section. Actually, it is coded in include/linux/export.h, but modpost does not need to hard-code the section name. Elf_Sym::st_shndx holds the index of the relevant section. Using it is a more portable way to get the namespace string. Make namespace_from_kstrtabns() simply call sym_get_data(), and delete the info->ksymtab_strings . While I was here, I added more 'const' qualifiers to pointers. Signed-off-by: Masahiro Yamada --- scripts/mod/modpost.c | 10 +++------- scripts/mod/modpost.h | 1 - 2 files changed, 3 insertions(+), 8 deletions(-) (limited to 'scripts/mod/modpost.c') diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index cd885573daaf..d9418c58a8c0 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -356,10 +356,10 @@ static enum export export_from_sec(struct elf_info *elf, unsigned int sec) return export_unknown; } -static const char *namespace_from_kstrtabns(struct elf_info *info, - Elf_Sym *kstrtabns) +static const char *namespace_from_kstrtabns(const struct elf_info *info, + const Elf_Sym *sym) { - char *value = info->ksymtab_strings + kstrtabns->st_value; + const char *value = sym_get_data(info, sym); return value[0] ? value : NULL; } @@ -601,10 +601,6 @@ 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; diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h index fe6652535e4b..64a82d2d85f6 100644 --- a/scripts/mod/modpost.h +++ b/scripts/mod/modpost.h @@ -143,7 +143,6 @@ 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 From 9bd2a099d7224281dd7756efa5c79df4f3fe8daf Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Fri, 15 Nov 2019 02:42:23 +0900 Subject: modpost: rename handle_modversions() to handle_symbol() This function handles not only modversions, but also unresolved symbols, export symbols, etc. Rename it to a more proper function name. While I was here, I also added the 'const' qualifier to *sym. Signed-off-by: Masahiro Yamada --- scripts/mod/modpost.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'scripts/mod/modpost.c') diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index d9418c58a8c0..6735ae3da4c2 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -683,8 +683,8 @@ static int ignore_undef_symbol(struct elf_info *info, const char *symname) return 0; } -static void handle_modversions(struct module *mod, struct elf_info *info, - Elf_Sym *sym, const char *symname) +static void handle_symbol(struct module *mod, struct elf_info *info, + const Elf_Sym *sym, const char *symname) { unsigned int crc; enum export export; @@ -2051,7 +2051,7 @@ static void read_symbols(const char *modname) for (sym = info.symtab_start; sym < info.symtab_stop; sym++) { symname = remove_dot(info.strtab + sym->st_name); - handle_modversions(mod, &info, sym, symname); + handle_symbol(mod, &info, sym, symname); handle_moddevtable(mod, &info, sym, symname); } -- cgit v1.2.3 From 1743694eb2357b47cd9951079f9ab0d728c916bf Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Fri, 15 Nov 2019 02:42:24 +0900 Subject: modpost: stop symbol preloading for modversion CRC It is complicated to add mocked-up symbols for pre-handling CRC. Handle CRC after all the export symbols in the relevant module are registered. Call handle_modversion() after the handle_symbol() iteration. In some cases, I see atand-alone __crc_* without __ksymtab_*. For example, ARCH=arm allyesconfig produces __crc_ccitt_veneer and __crc_itu_t_veneer. I guess they come from crc_ccitt, crc_itu_t, respectively. Since __*_veneer are auto-generated symbols, just ignore them. Signed-off-by: Masahiro Yamada --- scripts/mod/modpost.c | 71 ++++++++++++++++++++++++++++----------------------- 1 file changed, 39 insertions(+), 32 deletions(-) (limited to 'scripts/mod/modpost.c') diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 6735ae3da4c2..1c22cf7aa732 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -169,7 +169,7 @@ struct symbol { unsigned int vmlinux:1; /* 1 if symbol is defined in vmlinux */ unsigned int kernel:1; /* 1 if symbol is from kernel * (only for external modules) **/ - unsigned int preloaded:1; /* 1 if symbol from Module.symvers, or crc */ + unsigned int preloaded:1; /* 1 if symbol from Module.symvers */ unsigned int is_static:1; /* 1 if symbol is not global */ enum export export; /* Type of export */ char name[0]; @@ -410,16 +410,17 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod, return s; } -static void sym_update_crc(const char *name, struct module *mod, - unsigned int crc, enum export export) +static void sym_set_crc(const char *name, unsigned int crc) { struct symbol *s = find_symbol(name); - if (!s) { - s = new_symbol(name, mod, export); - /* Don't complain when we find it later. */ - s->preloaded = 1; - } + /* + * Ignore stand-alone __crc_*, which might be auto-generated symbols + * such as __*_veneer in ARM ELF. + */ + if (!s) + return; + s->crc = crc; s->crc_valid = 1; } @@ -683,12 +684,34 @@ static int ignore_undef_symbol(struct elf_info *info, const char *symname) return 0; } +static void handle_modversion(const struct module *mod, + const struct elf_info *info, + const Elf_Sym *sym, const char *symname) +{ + unsigned int crc; + + if (sym->st_shndx == SHN_UNDEF) { + warn("EXPORT symbol \"%s\" [%s%s] version generation failed, symbol will not be versioned.\n", + symname, mod->name, is_vmlinux(mod->name) ? "":".ko"); + return; + } + + if (sym->st_shndx == SHN_ABS) { + crc = sym->st_value; + } else { + unsigned int *crcp; + + /* symbol points to the CRC in the ELF object */ + crcp = sym_get_data(info, sym); + crc = TO_NATIVE(*crcp); + } + sym_set_crc(symname, crc); +} + static void handle_symbol(struct module *mod, struct elf_info *info, const Elf_Sym *sym, const char *symname) { - unsigned int crc; enum export export; - bool is_crc = false; const char *name; if ((!is_vmlinux(mod->name) || mod->is_dot_o) && @@ -697,21 +720,6 @@ static void handle_symbol(struct module *mod, struct elf_info *info, else export = export_from_sec(info, get_secindex(info, sym)); - /* CRC'd symbol */ - if (strstarts(symname, "__crc_")) { - is_crc = true; - crc = (unsigned int) sym->st_value; - if (sym->st_shndx != SHN_UNDEF && sym->st_shndx != SHN_ABS) { - unsigned int *crcp; - - /* symbol points to the CRC in the ELF object */ - crcp = sym_get_data(info, sym); - crc = TO_NATIVE(*crcp); - } - sym_update_crc(symname + strlen("__crc_"), mod, crc, - export); - } - switch (sym->st_shndx) { case SHN_COMMON: if (strstarts(symname, "__gnu_lto_")) { @@ -746,11 +754,6 @@ static void handle_symbol(struct module *mod, struct elf_info *info, } #endif - if (is_crc) { - const char *e = is_vmlinux(mod->name) ?"":".ko"; - warn("EXPORT symbol \"%s\" [%s%s] version generation failed, symbol will not be versioned.\n", - symname + strlen("__crc_"), mod->name, e); - } mod->unres = alloc_symbol(symname, ELF_ST_BIND(sym->st_info) == STB_WEAK, mod->unres); @@ -2055,14 +2058,18 @@ 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); + /* Apply symbol namespaces from __kstrtabns_ entries. */ if (strstarts(symname, "__kstrtabns_")) sym_update_namespace(symname + strlen("__kstrtabns_"), namespace_from_kstrtabns(&info, sym)); + + if (strstarts(symname, "__crc_")) + handle_modversion(mod, &info, sym, + symname + strlen("__crc_")); } // check for static EXPORT_SYMBOL_* functions && global vars @@ -2476,7 +2483,7 @@ static void read_dump(const char *fname, unsigned int kernel) s->kernel = kernel; s->preloaded = 1; s->is_static = 0; - sym_update_crc(symname, mod, crc, export_no(export)); + sym_set_crc(symname, crc); sym_update_namespace(symname, namespace); } release_file(file, size); -- cgit v1.2.3 From e4b26c9f75e48b5ef9e31ac6c8a445d4479b469c Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Fri, 15 Nov 2019 02:42:25 +0900 Subject: modpost: do not set ->preloaded for symbols from Module.symvers Now that there is no overwrap between symbols from ELF files and ones from Module.symvers. So, the 'exported twice' warning should be reported irrespective of where the symbol in question came from. The exceptional case is external module; in some cases, we build an external module to provide a different version/variant of the corresponding in-kernel module, overriding the same set of exported symbols. You can see this use-case in upstream; tools/testing/nvdimm/libnvdimm.ko replaces drivers/nvdimm/libnvdimm.ko in order to link it against mocked version of core kernel symbols. So, let's relax the 'exported twice' warning when building external modules. The multiple export from external modules is warned only when the previous one is from vmlinux or itself. With this refactoring, the ugly preloading goes away. Signed-off-by: Masahiro Yamada --- scripts/mod/modpost.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'scripts/mod/modpost.c') diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 1c22cf7aa732..2fa58fbbd10b 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -169,7 +169,6 @@ struct symbol { unsigned int vmlinux:1; /* 1 if symbol is defined in vmlinux */ unsigned int kernel:1; /* 1 if symbol is from kernel * (only for external modules) **/ - unsigned int preloaded:1; /* 1 if symbol from Module.symvers */ unsigned int is_static:1; /* 1 if symbol is not global */ enum export export; /* Type of export */ char name[0]; @@ -394,7 +393,8 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod, if (!s) { s = new_symbol(name, mod, export); } else { - if (!s->preloaded) { + if (!external_module || is_vmlinux(s->module->name) || + s->module == mod) { warn("%s: '%s' exported twice. Previous export was in %s%s\n", mod->name, name, s->module->name, is_vmlinux(s->module->name) ? "" : ".ko"); @@ -403,7 +403,6 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod, s->module = mod; } } - s->preloaded = 0; s->vmlinux = is_vmlinux(mod->name); s->kernel = 0; s->export = export; @@ -2481,7 +2480,6 @@ static void read_dump(const char *fname, unsigned int kernel) } s = sym_add_exported(symname, mod, export_no(export)); s->kernel = kernel; - s->preloaded = 1; s->is_static = 0; sym_set_crc(symname, crc); sym_update_namespace(symname, namespace); -- cgit v1.2.3 From 7ef9ab3b32b4bb72a7d70b832d2eb12ceb93d9fd Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Fri, 15 Nov 2019 02:42:26 +0900 Subject: modpost: respect the previous export when 'exported twice' is warned When 'exported twice' is warned, let sym_add_exported() return without updating the symbol info. This respects the previous export, which is ordered first in modules.order This simplifies the code too. Signed-off-by: Masahiro Yamada --- scripts/mod/modpost.c | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) (limited to 'scripts/mod/modpost.c') diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 2fa58fbbd10b..6e892c93d104 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -211,13 +211,11 @@ static struct symbol *new_symbol(const char *name, struct module *module, enum export export) { unsigned int hash; - struct symbol *new; hash = tdb_hash(name) % SYMBOL_HASH_SIZE; - new = symbolhash[hash] = alloc_symbol(name, 0, symbolhash[hash]); - new->module = module; - new->export = export; - return new; + symbolhash[hash] = alloc_symbol(name, 0, symbolhash[hash]); + + return symbolhash[hash]; } static struct symbol *find_symbol(const char *name) @@ -392,17 +390,15 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod, if (!s) { s = new_symbol(name, mod, export); - } else { - if (!external_module || is_vmlinux(s->module->name) || - s->module == mod) { - warn("%s: '%s' exported twice. Previous export was in %s%s\n", - mod->name, name, s->module->name, - is_vmlinux(s->module->name) ? "" : ".ko"); - } else { - /* In case Module.symvers was out of date */ - s->module = mod; - } + } else if (!external_module || is_vmlinux(s->module->name) || + s->module == mod) { + warn("%s: '%s' exported twice. Previous export was in %s%s\n", + mod->name, name, s->module->name, + is_vmlinux(s->module->name) ? "" : ".ko"); + return s; } + + s->module = mod; s->vmlinux = is_vmlinux(mod->name); s->kernel = 0; s->export = export; -- cgit v1.2.3