[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190826162138.GA31739@linux-8ccs>
Date: Mon, 26 Aug 2019 18:21:38 +0200
From: Jessica Yu <jeyu@...nel.org>
To: Matthias Maennich <maennich@...gle.com>
Cc: linux-kernel@...r.kernel.org, kernel-team@...roid.com,
arnd@...db.de, geert@...ux-m68k.org, gregkh@...uxfoundation.org,
hpa@...or.com, joel@...lfernandes.org,
kstewart@...uxfoundation.org, linux-arch@...r.kernel.org,
linux-kbuild@...r.kernel.org, linux-m68k@...ts.linux-m68k.org,
linux-modules@...r.kernel.org, linux-scsi@...r.kernel.org,
linux-usb@...r.kernel.org, lucas.de.marchi@...il.com,
maco@...roid.com, maco@...gle.com, michal.lkml@...kovi.net,
mingo@...hat.com, oneukum@...e.com, pombredanne@...b.com,
sam@...nborg.org, sspatil@...gle.com, stern@...land.harvard.edu,
tglx@...utronix.de, usb-storage@...ts.one-eyed-alien.net,
x86@...nel.org, yamada.masahiro@...ionext.com
Subject: Re: [PATCH v3 04/11] modpost: add support for symbol namespaces
+++ Matthias Maennich [21/08/19 12:49 +0100]:
>Add support for symbols that are exported into namespaces. For that,
>extract any namespace suffix from the symbol name. In addition, emit a
>warning whenever a module refers to an exported symbol without
>explicitly importing the namespace that it is defined in. This patch
>consistently adds the namespace suffix to symbol names exported into
>Module.symvers.
>
>Example warning emitted by modpost in case of the above violation:
>
> WARNING: module ums-usbat uses symbol usb_stor_resume from namespace
> USB_STORAGE, but does not import it.
>
>Co-developed-by: Martijn Coenen <maco@...roid.com>
>Signed-off-by: Martijn Coenen <maco@...roid.com>
>Reviewed-by: Joel Fernandes (Google) <joel@...lfernandes.org>
>Reviewed-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
>Signed-off-by: Matthias Maennich <maennich@...gle.com>
>---
> scripts/mod/modpost.c | 91 +++++++++++++++++++++++++++++++++++++------
> scripts/mod/modpost.h | 7 ++++
> 2 files changed, 87 insertions(+), 11 deletions(-)
>
>diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
>index f277e116e0eb..538bb24ffee3 100644
>--- a/scripts/mod/modpost.c
>+++ b/scripts/mod/modpost.c
>@@ -164,6 +164,7 @@ struct symbol {
> struct module *module;
> unsigned int crc;
> int crc_valid;
>+ const 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
>@@ -233,6 +234,37 @@ static struct symbol *find_symbol(const char *name)
> return NULL;
> }
>
>+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)
>+ return true;
>+
>+ return false;
>+}
>+
>+static void add_namespace(struct namespace_list **list, const char *namespace)
>+{
>+ struct namespace_list *ns_entry;
>+
>+ if (!contains_namespace(*list, namespace)) {
>+ ns_entry = NOFAIL(malloc(sizeof(struct namespace_list) +
>+ strlen(namespace) + 1));
>+ strcpy(ns_entry->namespace, namespace);
>+ ns_entry->next = *list;
>+ *list = ns_entry;
>+ }
>+}
>+
>+static bool module_imports_namespace(struct module *module,
>+ const char *namespace)
>+{
>+ return contains_namespace(module->imported_namespaces, namespace);
>+}
>+
> static const struct {
> const char *str;
> enum export export;
>@@ -312,6 +344,22 @@ 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)
>+{
>+ size_t n;
>+ char *dupsymname;
>+
>+ n = strcspn(*symname, ".");
>+ if (n < strlen(*symname) - 1) {
>+ dupsymname = NOFAIL(strdup(*symname));
>+ dupsymname[n] = '\0';
>+ *symname = dupsymname;
>+ return dupsymname + n + 1;
>+ }
>+
>+ return NULL;
>+}
>+
> /**
> * Add an exported symbol - it may have already been added without a
> * CRC, in this case just update the CRC
>@@ -319,16 +367,18 @@ static enum export export_from_sec(struct elf_info *elf, unsigned int sec)
> static struct symbol *sym_add_exported(const char *name, struct module *mod,
> enum export export)
> {
>- struct symbol *s = find_symbol(name);
>+ const char *symbol_name = name;
>+ const char *namespace = sym_extract_namespace(&symbol_name);
>+ struct symbol *s = find_symbol(symbol_name);
>
> if (!s) {
>- s = new_symbol(name, mod, export);
>+ s = new_symbol(symbol_name, mod, export);
>+ s->namespace = namespace;
> } else {
> if (!s->preloaded) {
>- warn("%s: '%s' exported twice. Previous export "
>- "was in %s%s\n", mod->name, name,
>- s->module->name,
>- is_vmlinux(s->module->name) ?"":".ko");
>+ warn("%s: '%s' exported twice. Previous export was in %s%s\n",
>+ mod->name, symbol_name, s->module->name,
>+ is_vmlinux(s->module->name) ? "" : ".ko");
> } else {
> /* In case Module.symvers was out of date */
> s->module = mod;
>@@ -1943,6 +1993,7 @@ static void read_symbols(const char *modname)
> const char *symname;
> char *version;
> char *license;
>+ char *namespace;
> struct module *mod;
> struct elf_info info = { };
> Elf_Sym *sym;
>@@ -1974,6 +2025,12 @@ static void read_symbols(const char *modname)
> license = get_next_modinfo(&info, "license", license);
> }
>
>+ namespace = get_modinfo(&info, "import_ns");
>+ while (namespace) {
>+ add_namespace(&mod->imported_namespaces, namespace);
>+ namespace = get_next_modinfo(&info, "import_ns", namespace);
>+ }
>+
> for (sym = info.symtab_start; sym < info.symtab_stop; sym++) {
> symname = remove_dot(info.strtab + sym->st_name);
>
>@@ -2118,6 +2175,13 @@ static int check_exports(struct module *mod)
> basename++;
> else
> basename = mod->name;
>+
>+ 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);
>+ }
>+
> if (!mod->gpl_compatible)
> check_for_gpl_usage(exp->export, basename, exp->name);
> check_for_unused(exp->export, basename, exp->name);
>@@ -2395,16 +2459,21 @@ static void write_dump(const char *fname)
> {
> struct buffer buf = { };
> struct symbol *symbol;
>+ const char *namespace;
> int n;
>
> for (n = 0; n < SYMBOL_HASH_SIZE ; n++) {
> symbol = symbolhash[n];
> while (symbol) {
>- if (dump_sym(symbol))
>- buf_printf(&buf, "0x%08x\t%s\t%s\t%s\n",
>- symbol->crc, symbol->name,
>- symbol->module->name,
>- export_str(symbol->export));
>+ if (dump_sym(symbol)) {
>+ namespace = symbol->namespace;
>+ buf_printf(&buf, "0x%08x\t%s%s%s\t%s\t%s\n",
>+ symbol->crc, symbol->name,
>+ namespace ? "." : "",
>+ namespace ? namespace : "",
I think it might be cleaner to just have namespace be a separate
field in Module.symvers, rather than appending a dot and the
namespace at the end of a symbol name. Maybe something like
<crc> <symbol_name> <namespace> <module>
For symbols without a namespace, we could just have "", with all
fields delimited by tabs. This is just a stylistic suggestion, what do
you think?
Regardless of the chosen format, I think we need to document how
namespaces are represented in Documentation/kbuild/modules.rst, where
it describes the Module.symvers format.
Thanks!
Jessica
>+ symbol->module->name,
>+ export_str(symbol->export));
>+ }
> symbol = symbol->next;
> }
> }
>diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
>index 8453d6ac2f77..9626bf3e7424 100644
>--- a/scripts/mod/modpost.h
>+++ b/scripts/mod/modpost.h
>@@ -109,6 +109,11 @@ buf_printf(struct buffer *buf, const char *fmt, ...);
> void
> buf_write(struct buffer *buf, const char *s, int len);
>
>+struct namespace_list {
>+ struct namespace_list *next;
>+ char namespace[0];
>+};
>+
> struct module {
> struct module *next;
> const char *name;
>@@ -121,6 +126,8 @@ struct module {
> struct buffer dev_table_buf;
> char srcversion[25];
> int is_dot_o;
>+ // Actual imported namespaces
>+ struct namespace_list *imported_namespaces;
> };
>
> struct elf_info {
>--
>2.23.0.rc1.153.gdeed80330f-goog
>
Powered by blists - more mailing lists