* [PATCH] modpost: distinguish same module paths from different dump files
@ 2024-12-12 15:46 Masahiro Yamada
2024-12-12 15:49 ` Masahiro Yamada
2024-12-12 20:17 ` Jon Hunter
0 siblings, 2 replies; 5+ messages in thread
From: Masahiro Yamada @ 2024-12-12 15:46 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-kernel, Masahiro Yamada, Jon Hunter, Nathan Chancellor,
Nicolas Schier
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@nvidia.com>
Closes: https://lore.kernel.org/all/eb21a546-a19c-40df-b821-bbba80f19a3d@nvidia.com/
Signed-off-by: Masahiro Yamada <masahiroy@kernel.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;
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] modpost: distinguish same module paths from different dump files
2024-12-12 15:46 [PATCH] modpost: distinguish same module paths from different dump files Masahiro Yamada
@ 2024-12-12 15:49 ` Masahiro Yamada
2024-12-12 20:17 ` Jon Hunter
1 sibling, 0 replies; 5+ messages in thread
From: Masahiro Yamada @ 2024-12-12 15:49 UTC (permalink / raw)
To: linux-kbuild; +Cc: linux-kernel, Jon Hunter, Nathan Chancellor, Nicolas Schier
On Fri, Dec 13, 2024 at 12:46 AM Masahiro Yamada <masahiroy@kernel.org> 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
is -> are
> 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@nvidia.com>
> Closes: https://lore.kernel.org/all/eb21a546-a19c-40df-b821-bbba80f19a3d@nvidia.com/
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] modpost: distinguish same module paths from different dump files
2024-12-12 15:46 [PATCH] modpost: distinguish same module paths from different dump files Masahiro Yamada
2024-12-12 15:49 ` Masahiro Yamada
@ 2024-12-12 20:17 ` Jon Hunter
2024-12-19 10:48 ` Jon Hunter
1 sibling, 1 reply; 5+ messages in thread
From: Jon Hunter @ 2024-12-12 20:17 UTC (permalink / raw)
To: Masahiro Yamada, linux-kbuild
Cc: linux-kernel, Nathan Chancellor, Nicolas Schier,
linux-tegra@vger.kernel.org
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@nvidia.com>
> Closes: https://lore.kernel.org/all/eb21a546-a19c-40df-b821-bbba80f19a3d@nvidia.com/
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.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@nvidia.com>
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] modpost: distinguish same module paths from different dump files
2024-12-12 20:17 ` Jon Hunter
@ 2024-12-19 10:48 ` Jon Hunter
2024-12-21 3:48 ` Masahiro Yamada
0 siblings, 1 reply; 5+ messages in thread
From: Jon Hunter @ 2024-12-19 10:48 UTC (permalink / raw)
To: Masahiro Yamada, linux-kbuild
Cc: linux-kernel, Nathan Chancellor, Nicolas Schier,
linux-tegra@vger.kernel.org
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@nvidia.com>
>> Closes: https://lore.kernel.org/all/eb21a546-a19c-40df-b821-
>> bbba80f19a3d@nvidia.com/
>> Signed-off-by: Masahiro Yamada <masahiroy@kernel.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@nvidia.com>
I have not seen this land in -next yet. Would be great to get this applied.
Thanks!
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] modpost: distinguish same module paths from different dump files
2024-12-19 10:48 ` Jon Hunter
@ 2024-12-21 3:48 ` Masahiro Yamada
0 siblings, 0 replies; 5+ messages in thread
From: Masahiro Yamada @ 2024-12-21 3:48 UTC (permalink / raw)
To: Jon Hunter
Cc: linux-kbuild, linux-kernel, Nathan Chancellor, Nicolas Schier,
linux-tegra@vger.kernel.org
On Thu, Dec 19, 2024 at 7:48 PM Jon Hunter <jonathanh@nvidia.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@nvidia.com>
> >> Closes: https://lore.kernel.org/all/eb21a546-a19c-40df-b821-
> >> bbba80f19a3d@nvidia.com/
> >> Signed-off-by: Masahiro Yamada <masahiroy@kernel.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@nvidia.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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-12-21 3:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-12 15:46 [PATCH] modpost: distinguish same module paths from different dump files Masahiro Yamada
2024-12-12 15:49 ` Masahiro Yamada
2024-12-12 20:17 ` Jon Hunter
2024-12-19 10:48 ` Jon Hunter
2024-12-21 3:48 ` Masahiro Yamada
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox