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] [thread-next>] [day] [month] [year] [list]
Message-ID: <5963c8b3-7e03-4f2d-9198-f02cc3140313@nvidia.com>
Date: Thu, 19 Dec 2024 10:48:29 +0000
From: Jon Hunter <jonathanh@...dia.com>
To: Masahiro Yamada <masahiroy@...nel.org>, linux-kbuild@...r.kernel.org
Cc: 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 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.

Thanks!
Jon

-- 
nvpublic