Re: [PATCH] modpost: distinguish same module paths from different dump files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 12/12/2024 20:17, Jon Hunter wrote:

On 12/12/2024 15:46, Masahiro Yamada wrote:
Since commit 13b25489b6f8 ("kbuild: change working directory to external
module directory with M="), module paths are always relative to the top
of the external module tree.

The module paths recorded in Module.symvers is no longer globally unique
when they are passed via KBUILD_EXTRA_SYMBOLS for building other external
modules, which may result in false positive "exported twice" errors.
Such errors should not occur because external modules should be able to
override in-tree modules.

To address this, record the dump file path in struct module and check it
when searching for a module.

Fixes: 13b25489b6f8 ("kbuild: change working directory to external module directory with M=")
Reported-by: Jon Hunter <jonathanh@xxxxxxxxxx>
Closes: https://lore.kernel.org/all/eb21a546-a19c-40df-b821- bbba80f19a3d@xxxxxxxxxx/
Signed-off-by: Masahiro Yamada <masahiroy@xxxxxxxxxx>
---

  scripts/mod/modpost.c | 17 +++++++++--------
  scripts/mod/modpost.h |  3 ++-
  2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index fb787a5715f5..94ee49207a45 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -155,12 +155,13 @@ char *get_line(char **stringp)
  /* A list of all modules we processed */
  LIST_HEAD(modules);
-static struct module *find_module(const char *modname)
+static struct module *find_module(const char *filename, const char *modname)
  {
      struct module *mod;
      list_for_each_entry(mod, &modules, list) {
-        if (strcmp(mod->name, modname) == 0)
+        if (!strcmp(mod->dump_file, filename) &&
+            !strcmp(mod->name, modname))
              return mod;
      }
      return NULL;
@@ -2030,10 +2031,10 @@ static void read_dump(const char *fname)
              continue;
          }
-        mod = find_module(modname);
+        mod = find_module(fname, modname);
          if (!mod) {
              mod = new_module(modname, strlen(modname));
-            mod->from_dump = true;
+            mod->dump_file = fname;
          }
          s = sym_add_exported(symname, mod, gpl_only, namespace);
          sym_set_crc(s, crc);
@@ -2052,7 +2053,7 @@ static void write_dump(const char *fname)
      struct symbol *sym;
      list_for_each_entry(mod, &modules, list) {
-        if (mod->from_dump)
+        if (mod->dump_file)
              continue;
          list_for_each_entry(sym, &mod->exported_symbols, list) {
              if (trim_unused_exports && !sym->used)
@@ -2076,7 +2077,7 @@ static void write_namespace_deps_files(const char *fname)
      list_for_each_entry(mod, &modules, list) {
-        if (mod->from_dump || list_empty(&mod->missing_namespaces))
+        if (mod->dump_file || list_empty(&mod->missing_namespaces))
              continue;
          buf_printf(&ns_deps_buf, "%s.ko:", mod->name);
@@ -2194,7 +2195,7 @@ int main(int argc, char **argv)
          read_symbols_from_files(files_source);
      list_for_each_entry(mod, &modules, list) {
-        if (mod->from_dump || mod->is_vmlinux)
+        if (mod->dump_file || mod->is_vmlinux)
              continue;
          check_modname_len(mod);
@@ -2205,7 +2206,7 @@ int main(int argc, char **argv)
          handle_white_list_exports(unused_exports_white_list);
      list_for_each_entry(mod, &modules, list) {
-        if (mod->from_dump)
+        if (mod->dump_file)
              continue;
          if (mod->is_vmlinux)
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 49848fcbe2a1..8b72c227ebf4 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -95,14 +95,15 @@ struct module_alias {
  /**
   * struct module - represent a module (vmlinux or *.ko)
   *
+ * @dump_file: path to the .symvers file if loaded from a file
   * @aliases: list head for module_aliases
   */
  struct module {
      struct list_head list;
      struct list_head exported_symbols;
      struct list_head unresolved_symbols;
+    const char *dump_file;
      bool is_gpl_compatible;
-    bool from_dump;        /* true if module was loaded from *.symvers */
      bool is_vmlinux;
      bool seen;
      bool has_init;


Thanks for fixing!

Tested-by: Jon Hunter <jonathanh@xxxxxxxxxx>


I have not seen this land in -next yet. Would be great to get this applied.

Thanks!
Jon

--
nvpublic





[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux