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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAK7LNAQ5Fw97MKSMGeMWk+Fj5=tGpJVQxAO4LTG7BLY3iaPHkg@mail.gmail.com>
Date: Sat, 21 Dec 2024 12:48:21 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: Jon Hunter <jonathanh@...dia.com>
Cc: linux-kbuild@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Nathan Chancellor <nathan@...nel.org>, Nicolas Schier <nicolas@...sle.eu>, 
	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>
Subject: Re: [PATCH] modpost: distinguish same module paths from different
 dump files

On Thu, Dec 19, 2024 at 7:48 PM Jon Hunter <jonathanh@...dia.com> wrote:
>
>
> 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@...dia.com>
> >> Closes: https://lore.kernel.org/all/eb21a546-a19c-40df-b821-
> >> bbba80f19a3d@...dia.com/
> >> Signed-off-by: Masahiro Yamada <masahiroy@...nel.org>
> >> ---
> >>
> >>   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@...dia.com>
>
>
> I have not seen this land in -next yet. Would be great to get this applied.
>

Now applied to linux-kbuild/fixes.


-- 
Best Regards
Masahiro Yamada