From c7c4e29fb5a422fd144b3fb0d3e32375d8f9b0ba Mon Sep 17 00:00:00 2001 From: YueHaibing Date: Sun, 6 Oct 2019 12:44:56 +0800 Subject: [PATCH 1/8] scripts: add_namespace: Fix coccicheck failed Now all scripts in scripts/coccinelle to be automatically called by coccicheck. However new adding add_namespace.cocci does not support report mode, which make coccicheck failed. This add "virtual report" to make the coccicheck go ahead smoothly. Fixes: eb8305aecb95 ("scripts: Coccinelle script for namespace dependencies.") Acked-by: Julia Lawall Acked-by: Matthias Maennich Signed-off-by: YueHaibing Signed-off-by: Jessica Yu --- scripts/coccinelle/misc/add_namespace.cocci | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/coccinelle/misc/add_namespace.cocci b/scripts/coccinelle/misc/add_namespace.cocci index c832bb6445a8..99e93a6c2e24 100644 --- a/scripts/coccinelle/misc/add_namespace.cocci +++ b/scripts/coccinelle/misc/add_namespace.cocci @@ -6,6 +6,8 @@ /// add a missing namespace tag to a module source file. /// +virtual report + @has_ns_import@ declarer name MODULE_IMPORT_NS; identifier virtual.ns; From bf70b0503abd19194dba25fe383d143d0229dc6a Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Thu, 3 Oct 2019 16:58:21 +0900 Subject: [PATCH 2/8] module: swap the order of symbol.namespace Currently, EXPORT_SYMBOL_NS(_GPL) constructs the kernel symbol as follows: __ksymtab_SYMBOL.NAMESPACE The sym_extract_namespace() in modpost allocates memory for the part SYMBOL.NAMESPACE when '.' is contained. One problem is that the pointer returned by strdup() is lost because the symbol name will be copied to malloc'ed memory by alloc_symbol(). No one will keep track of the pointer of strdup'ed memory. sym->namespace still points to the NAMESPACE part. So, you can free it with complicated code like this: free(sym->namespace - strlen(sym->name) - 1); It complicates memory free. To fix it elegantly, I swapped the order of the symbol and the namespace as follows: __ksymtab_NAMESPACE.SYMBOL then, simplified sym_extract_namespace() so that it allocates memory only for the NAMESPACE part. I prefer this order because it is intuitive and also matches to major languages. For example, NAMESPACE::NAME in C++, MODULE.NAME in Python. Reviewed-by: Matthias Maennich Signed-off-by: Masahiro Yamada Signed-off-by: Jessica Yu --- include/linux/export.h | 4 ++-- scripts/mod/modpost.c | 16 +++++++--------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/include/linux/export.h b/include/linux/export.h index 95f55b7f83a0..0695d4e847d9 100644 --- a/include/linux/export.h +++ b/include/linux/export.h @@ -52,7 +52,7 @@ extern struct module __this_module; __ADDRESSABLE(sym) \ asm(" .section \"___ksymtab" sec "+" #sym "\", \"a\" \n" \ " .balign 4 \n" \ - "__ksymtab_" #sym NS_SEPARATOR #ns ": \n" \ + "__ksymtab_" #ns NS_SEPARATOR #sym ": \n" \ " .long " #sym "- . \n" \ " .long __kstrtab_" #sym "- . \n" \ " .long __kstrtab_ns_" #sym "- . \n" \ @@ -76,7 +76,7 @@ struct kernel_symbol { #else #define __KSYMTAB_ENTRY_NS(sym, sec, ns) \ static const struct kernel_symbol __ksymtab_##sym##__##ns \ - asm("__ksymtab_" #sym NS_SEPARATOR #ns) \ + asm("__ksymtab_" #ns NS_SEPARATOR #sym) \ __attribute__((section("___ksymtab" sec "+" #sym), used)) \ __aligned(sizeof(void *)) \ = { (unsigned long)&sym, __kstrtab_##sym, __kstrtab_ns_##sym } diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 3961941e8e7a..c4b536ac1327 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -350,18 +350,16 @@ static enum export export_from_sec(struct elf_info *elf, unsigned int sec) static const char *sym_extract_namespace(const char **symname) { - size_t n; - char *dupsymname; + char *namespace = NULL; + char *ns_separator; - n = strcspn(*symname, "."); - if (n < strlen(*symname) - 1) { - dupsymname = NOFAIL(strdup(*symname)); - dupsymname[n] = '\0'; - *symname = dupsymname; - return dupsymname + n + 1; + ns_separator = strchr(*symname, '.'); + if (ns_separator) { + namespace = NOFAIL(strndup(*symname, ns_separator - *symname)); + *symname = ns_separator + 1; } - return NULL; + return namespace; } /** From 389eb3f5f4abbdb9810458ac9b87427336ba5b91 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Thu, 3 Oct 2019 16:58:22 +0900 Subject: [PATCH 3/8] modpost: fix broken sym->namespace for external module builds Currently, external module builds produce tons of false-positives: WARNING: module uses symbol from namespace , but does not import it. Here, the part shows a random string. When you build external modules, the symbol info of vmlinux and in-kernel modules are read from $(objtree)/Module.symvers, but read_dump() is buggy in multiple ways: [1] When the modpost is run for vmlinux and in-kernel modules, sym_extract_namespace() allocates memory for the namespace. On the other hand, read_dump() does not, then sym->namespace will point to somewhere in the line buffer of get_next_line(). The data in the buffer will be replaced soon, and sym->namespace will end up with pointing to unrelated data. As a result, check_exports() will show random strings in the warning messages. [2] When there is no namespace, sym_extract_namespace() returns NULL. On the other hand, read_dump() sets namespace to an empty string "". (but, it will be later replaced with unrelated data due to bug [1].) The check_exports() shows a warning unless exp->namespace is NULL, so every symbol read from read_dump() emits the warning, which is mostly false positive. To address [1], sym_add_exported() calls strdup() for s->namespace. The namespace from sym_extract_namespace() must be freed to avoid memory leak. For [2], I changed the if-conditional in check_exports(). This commit also fixes sym_add_exported() to set s->namespace correctly when the symbol is preloaded. Reviewed-by: Matthias Maennich Signed-off-by: Masahiro Yamada Signed-off-by: Jessica Yu --- scripts/mod/modpost.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index c4b536ac1327..4d2cdb4d71e3 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -166,7 +166,7 @@ struct symbol { struct module *module; unsigned int crc; int crc_valid; - const char *namespace; + char *namespace; unsigned int weak:1; unsigned int vmlinux:1; /* 1 if symbol is defined in vmlinux */ unsigned int kernel:1; /* 1 if symbol is from kernel @@ -348,7 +348,7 @@ static enum export export_from_sec(struct elf_info *elf, unsigned int sec) return export_unknown; } -static const char *sym_extract_namespace(const char **symname) +static char *sym_extract_namespace(const char **symname) { char *namespace = NULL; char *ns_separator; @@ -373,7 +373,6 @@ static struct symbol *sym_add_exported(const char *name, const char *namespace, if (!s) { s = new_symbol(name, mod, export); - s->namespace = namespace; } else { if (!s->preloaded) { warn("%s: '%s' exported twice. Previous export was in %s%s\n", @@ -384,6 +383,8 @@ static struct symbol *sym_add_exported(const char *name, const char *namespace, s->module = mod; } } + free(s->namespace); + s->namespace = namespace ? strdup(namespace) : NULL; s->preloaded = 0; s->vmlinux = is_vmlinux(mod->name); s->kernel = 0; @@ -670,7 +671,8 @@ static void handle_modversions(struct module *mod, struct elf_info *info, unsigned int crc; enum export export; bool is_crc = false; - const char *name, *namespace; + const char *name; + char *namespace; if ((!is_vmlinux(mod->name) || mod->is_dot_o) && strstarts(symname, "__ksymtab")) @@ -745,6 +747,7 @@ static void handle_modversions(struct module *mod, struct elf_info *info, name = symname + strlen("__ksymtab_"); namespace = sym_extract_namespace(&name); sym_add_exported(name, namespace, mod, export); + free(namespace); } if (strcmp(symname, "init_module") == 0) mod->has_init = 1; @@ -2193,7 +2196,7 @@ static int check_exports(struct module *mod) else basename = mod->name; - if (exp->namespace) { + if (exp->namespace && exp->namespace[0]) { add_namespace(&mod->required_namespaces, exp->namespace); From fa6643cdc5cd726b10d30eec45ff8dca267de735 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Thu, 3 Oct 2019 16:58:23 +0900 Subject: [PATCH 4/8] module: rename __kstrtab_ns_* to __kstrtabns_* to avoid symbol conflict The module namespace produces __strtab_ns_ symbols to store namespace strings, but it does not guarantee the name uniqueness. This is a potential problem because we have exported symbols starting with "ns_". For example, kernel/capability.c exports the following symbols: EXPORT_SYMBOL(ns_capable); EXPORT_SYMBOL(capable); Assume a situation where those are converted as follows: EXPORT_SYMBOL_NS(ns_capable, some_namespace); EXPORT_SYMBOL_NS(capable, some_namespace); The former expands to "__kstrtab_ns_capable" and "__kstrtab_ns_ns_capable", and the latter to "__kstrtab_capable" and "__kstrtab_ns_capable". Then, we have the duplicated "__kstrtab_ns_capable". To ensure the uniqueness, rename "__kstrtab_ns_*" to "__kstrtabns_*". Reviewed-by: Matthias Maennich Signed-off-by: Masahiro Yamada Signed-off-by: Jessica Yu --- include/linux/export.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/linux/export.h b/include/linux/export.h index 0695d4e847d9..621158ecd2e2 100644 --- a/include/linux/export.h +++ b/include/linux/export.h @@ -55,7 +55,7 @@ extern struct module __this_module; "__ksymtab_" #ns NS_SEPARATOR #sym ": \n" \ " .long " #sym "- . \n" \ " .long __kstrtab_" #sym "- . \n" \ - " .long __kstrtab_ns_" #sym "- . \n" \ + " .long __kstrtabns_" #sym "- . \n" \ " .previous \n") #define __KSYMTAB_ENTRY(sym, sec) \ @@ -79,7 +79,7 @@ struct kernel_symbol { asm("__ksymtab_" #ns NS_SEPARATOR #sym) \ __attribute__((section("___ksymtab" sec "+" #sym), used)) \ __aligned(sizeof(void *)) \ - = { (unsigned long)&sym, __kstrtab_##sym, __kstrtab_ns_##sym } + = { (unsigned long)&sym, __kstrtab_##sym, __kstrtabns_##sym } #define __KSYMTAB_ENTRY(sym, sec) \ static const struct kernel_symbol __ksymtab_##sym \ @@ -112,7 +112,7 @@ struct kernel_symbol { /* For every exported symbol, place a struct in the __ksymtab section */ #define ___EXPORT_SYMBOL_NS(sym, sec, ns) \ ___export_symbol_common(sym, sec); \ - static const char __kstrtab_ns_##sym[] \ + static const char __kstrtabns_##sym[] \ __attribute__((section("__ksymtab_strings"), used, aligned(1))) \ = #ns; \ __KSYMTAB_ENTRY_NS(sym, sec, ns) From d85103ac78a6d8573b21348b36f4cca2e1839a31 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Thu, 3 Oct 2019 16:58:24 +0900 Subject: [PATCH 5/8] kbuild: fix build error of 'make nsdeps' in clean tree Running 'make nsdeps' in a clean source tree fails as follows: $ make -s clean; make -s defconfig; make nsdeps [ snip ] awk: fatal: cannot open file `init/modules.order' for reading (No such file or directory) make: *** [Makefile;1307: modules.order] Error 2 make: *** Deleting file 'modules.order' make: *** Waiting for unfinished jobs.... The cause of the error is 'make nsdeps' does not build modules at all. Set KBUILD_MODULES to fix it. Reviewed-by: Matthias Maennich Signed-off-by: Masahiro Yamada Signed-off-by: Jessica Yu --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 6f54f2f95743..d97f38ad5d9f 100644 --- a/Makefile +++ b/Makefile @@ -616,7 +616,7 @@ endif # in addition to whatever we do anyway. # Just "make" or "make all" shall build modules as well -ifneq ($(filter all _all modules,$(MAKECMDGOALS)),) +ifneq ($(filter all _all modules nsdeps,$(MAKECMDGOALS)),) KBUILD_MODULES := 1 endif From 40997fb8799dab048bd4fa538f62a11ed4224e2a Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Thu, 3 Oct 2019 16:58:25 +0900 Subject: [PATCH 6/8] nsdeps: fix hashbang of scripts/nsdeps This script does not use bash-extension. I am guessing this hashbang was copied from scripts/coccicheck, which really uses bash-extension. /bin/sh is enough for this script. Reviewed-by: Matthias Maennich Signed-off-by: Masahiro Yamada Signed-off-by: Jessica Yu --- scripts/nsdeps | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/nsdeps b/scripts/nsdeps index ac2b6031dd13..964b7fb8c546 100644 --- a/scripts/nsdeps +++ b/scripts/nsdeps @@ -1,4 +1,4 @@ -#!/bin/bash +#!/bin/sh # SPDX-License-Identifier: GPL-2.0 # Linux kernel symbol namespace import generator # From df6f0987e55fd08ad2e93370eea12b5290705d4b Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Thu, 3 Oct 2019 16:58:26 +0900 Subject: [PATCH 7/8] nsdeps: make generated patches independent of locale scripts/nsdeps automatically generates a patch to add MODULE_IMPORT_NS tags, and what is nicer, it sorts the lines alphabetically with the 'sort' command. However, the output from the 'sort' command depends on locale. For example, I got this: $ { echo usbstorage; echo usb_storage; } | LANG=en_US.UTF-8 sort usbstorage usb_storage $ { echo usbstorage; echo usb_storage; } | LANG=C sort usb_storage usbstorage So, this means people might potentially send different patches. This kind of issue was reported in the past, for example, commit f55f2328bb28 ("kbuild: make sorting initramfs contents independent of locale"). Adding 'LANG=C' is a conventional way of fixing when a deterministic result is desirable. I added 'LANG=C' very close to the 'sort' command since changing locale affects the language of error messages etc. We should respect users' choice as much as possible. Reviewed-by: Matthias Maennich Signed-off-by: Masahiro Yamada Signed-off-by: Jessica Yu --- scripts/nsdeps | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/nsdeps b/scripts/nsdeps index 964b7fb8c546..3754dac13b31 100644 --- a/scripts/nsdeps +++ b/scripts/nsdeps @@ -41,7 +41,7 @@ generate_deps() { for source_file in $mod_source_files; do sed '/MODULE_IMPORT_NS/Q' $source_file > ${source_file}.tmp offset=$(wc -l ${source_file}.tmp | awk '{print $1;}') - cat $source_file | grep MODULE_IMPORT_NS | sort -u >> ${source_file}.tmp + cat $source_file | grep MODULE_IMPORT_NS | LANG=C sort -u >> ${source_file}.tmp tail -n +$((offset +1)) ${source_file} | grep -v MODULE_IMPORT_NS >> ${source_file}.tmp if ! diff -q ${source_file} ${source_file}.tmp; then mv ${source_file}.tmp ${source_file} From fcfacb9f83745d9fa97937b8bc94a73bb0607912 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Tue, 8 Oct 2019 12:10:09 +0900 Subject: [PATCH 8/8] doc: move namespaces.rst from kbuild/ to core-api/ We discussed a better location for this file, and agreed that core-api/ is a good fit. Rename it to symbol-namespaces.rst for disambiguation, and also add it to index.rst and MAINTAINERS. Signed-off-by: Masahiro Yamada Acked-by: Matthias Maennich Signed-off-by: Jessica Yu --- Documentation/core-api/index.rst | 1 + .../{kbuild/namespaces.rst => core-api/symbol-namespaces.rst} | 0 MAINTAINERS | 1 + 3 files changed, 2 insertions(+) rename Documentation/{kbuild/namespaces.rst => core-api/symbol-namespaces.rst} (100%) diff --git a/Documentation/core-api/index.rst b/Documentation/core-api/index.rst index fa16a0538dcb..ab0eae1c153a 100644 --- a/Documentation/core-api/index.rst +++ b/Documentation/core-api/index.rst @@ -38,6 +38,7 @@ Core utilities protection-keys ../RCU/index gcc-plugins + symbol-namespaces Interfaces for kernel debugging diff --git a/Documentation/kbuild/namespaces.rst b/Documentation/core-api/symbol-namespaces.rst similarity index 100% rename from Documentation/kbuild/namespaces.rst rename to Documentation/core-api/symbol-namespaces.rst diff --git a/MAINTAINERS b/MAINTAINERS index 296de2b51c83..3dec8d53c34b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -11543,6 +11543,7 @@ NSDEPS M: Matthias Maennich S: Maintained F: scripts/nsdeps +F: Documentation/core-api/symbol-namespaces.rst NTB AMD DRIVER M: Shyam Sundar S K