[PATCH 0/7] module: various bug-fixes and clean-ups for module namespace

[PATCH 0/7] module: various bug-fixes and clean-ups for module namespace

[Masahiro Yamada]

I was hit by some problems caused by the module namespace feature
that was merged recently. At least, the breakage of
external module builds is a fatal one. I just took a look at the code
closer, and I noticed some more issues and improvements.

I hope these patches are mostly OK.
The 4th patch might have room for argument since it is a trade-off
of "cleaner implermentation" vs "code size".



Masahiro Yamada (7):
  modpost: fix broken sym->namespace for external module builds
  module: swap the order of symbol.namespace
  module: rename __kstrtab_ns_* to __kstrtabns_* to avoid symbol
    conflict
  module: avoid code duplication in include/linux/export.h
  kbuild: fix build error of 'make nsdeps' in clean tree
  nsdeps: fix hashbang of scripts/nsdeps
  nsdeps: make generated patches independent of locale

 Makefile               |   2 +-
 include/linux/export.h | 104 ++++++++++++++---------------------------
 kernel/module.c        |   2 +-
 scripts/mod/modpost.c  |  20 ++++----
 scripts/nsdeps         |   4 +-
 5 files changed, 47 insertions(+), 85 deletions(-)

-- 
2.17.1

[PATCH 1/7] modpost: fix broken sym->namespace for external module builds

[Masahiro Yamada]

Currently, external module builds produce tons of false-positives:

  WARNING: module <mod> uses symbol <sym> from namespace <ns>, but does not import it.

Here, the <ns> part shows a random string.

When you build external modules, the symbol info of vmlinux and
in-kernel modules are read from $(objtree)/Module.symvers, but
read_dump() is buggy in multiple ways:

[1] When the modpost is run for vmlinux and in-kernel modules,
sym_extract_namespace() correctly allocates memory for the namespace.
On the other hand, read_dump() does not, then sym->namespace will
point to somewhere in the line buffer of get_next_line(). The data
in the buffer will be replaced soon, and sym->namespace will end up
with pointing to unrelated data. As a result, check_exports() will
show random strings in the warning messages.

[2] When there is no namespace, sym_extract_namespace() returns NULL.
On the other hand, read_dump() sets namespace to an empty string "".
(but, it will be later replaced with unrelated data due to bug [1].)
The check_exports() shows a warning unless exp->namespace is NULL,
so every symbol read from read_dump() emits the warning, which is
mostly false positive.

To address [1], I added NOFAIL(strdup(...)) to allocate memory.
For [2], I changed the if-conditional in check_exports().

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 scripts/mod/modpost.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 3961941e8e7a..5c628a7d80f7 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -2195,7 +2195,7 @@ static int check_exports(struct module *mod)
 		else
 			basename = mod->name;
 
-		if (exp->namespace) {
+		if (exp->namespace && exp->namespace[0]) {
 			add_namespace(&mod->required_namespaces,
 				      exp->namespace);
 
@@ -2453,7 +2453,7 @@ static void read_dump(const char *fname, unsigned int kernel)
 			mod = new_module(modname);
 			mod->skip = 1;
 		}
-		s = sym_add_exported(symname, namespace, mod,
+		s = sym_add_exported(symname, NOFAIL(strdup(namespace)), mod,
 				     export_no(export));
 		s->kernel    = kernel;
 		s->preloaded = 1;
-- 
2.17.1

[PATCH 2/7] module: swap the order of symbol.namespace

[Masahiro Yamada]

Currently, EXPORT_SYMBOL_NS(_GPL) constructs the kernel symbol as
follows:

  __ksymtab_SYMBOL.NAMESPACE

The sym_extract_namespace() in modpost allocates memory for the part
SYMBOL.NAMESPACE when '.' is contained. One problem is that the pointer
returned by strdup() is lost because the symbol name will be copied to
malloc'ed memory by alloc_symbol(). No one will keep track of the
pointer of strdup'ed memory.

sym->namespace still points to the NAMESPACE part. So, if you like,
you can free it with complicated code like this:

   free(sym->namespace - strlen(sym->name) - 1);

I would not say it is fatal since we seldom bother to manually free
memory in host programs. But, I can fix it in an elegant way.

I swapped the order of the symbol and the namespace as follows:

  __ksymtab_NAMESPACE.SYMBOL

then, simplified sym_extract_namespace() so that it allocates memory
only for the NAMESPACE part.

I prefer this order because it is intuitive and also matches to major
languages. For example, NAMESPACE::NAME in C++, MODULE.NAME in Python.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 include/linux/export.h |  4 ++--
 scripts/mod/modpost.c  | 16 +++++++---------
 2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/include/linux/export.h b/include/linux/export.h
index 95f55b7f83a0..0695d4e847d9 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -52,7 +52,7 @@ extern struct module __this_module;
 	__ADDRESSABLE(sym)						\
 	asm("	.section \"___ksymtab" sec "+" #sym "\", \"a\"	\n"	\
 	    "	.balign	4					\n"	\
-	    "__ksymtab_" #sym NS_SEPARATOR #ns ":		\n"	\
+	    "__ksymtab_" #ns NS_SEPARATOR #sym ":		\n"	\
 	    "	.long	" #sym "- .				\n"	\
 	    "	.long	__kstrtab_" #sym "- .			\n"	\
 	    "	.long	__kstrtab_ns_" #sym "- .		\n"	\
@@ -76,7 +76,7 @@ struct kernel_symbol {
 #else
 #define __KSYMTAB_ENTRY_NS(sym, sec, ns)				\
 	static const struct kernel_symbol __ksymtab_##sym##__##ns	\
-	asm("__ksymtab_" #sym NS_SEPARATOR #ns)				\
+	asm("__ksymtab_" #ns NS_SEPARATOR #sym)				\
 	__attribute__((section("___ksymtab" sec "+" #sym), used))	\
 	__aligned(sizeof(void *))					\
 	= { (unsigned long)&sym, __kstrtab_##sym, __kstrtab_ns_##sym }
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 5c628a7d80f7..d171b0cffb05 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -350,18 +350,16 @@ static enum export export_from_sec(struct elf_info *elf, unsigned int sec)
 
 static const char *sym_extract_namespace(const char **symname)
 {
-	size_t n;
-	char *dupsymname;
+	char *namespace = NULL;
+	char *ns_separator;
 
-	n = strcspn(*symname, ".");
-	if (n < strlen(*symname) - 1) {
-		dupsymname = NOFAIL(strdup(*symname));
-		dupsymname[n] = '\0';
-		*symname = dupsymname;
-		return dupsymname + n + 1;
+	ns_separator = strchr(*symname, '.');
+	if (ns_separator) {
+		namespace = NOFAIL(strndup(*symname, ns_separator - *symname));
+		*symname = ns_separator + 1;
 	}
 
-	return NULL;
+	return namespace;
 }
 
 /**
-- 
2.17.1

[PATCH 5/7] kbuild: fix build error of 'make nsdeps' in clean tree

[Masahiro Yamada]

Running 'make nsdeps' in a clean source tree fails as follows:

$ make -s clean; make -s defconfig; make nsdeps
   [ snip ]
awk: fatal: cannot open file `init/modules.order' for reading (No such file or directory)
make: *** [Makefile;1307: modules.order] Error 2
make: *** Deleting file 'modules.order'
make: *** Waiting for unfinished jobs....

The cause of the error is 'make nsdeps' does not build modules at all.
Set KBUILD_MODULES to fix it.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index d456746da347..80ba8efd56bb 100644
--- a/Makefile
+++ b/Makefile
@@ -616,7 +616,7 @@ endif
 # in addition to whatever we do anyway.
 # Just "make" or "make all" shall build modules as well
 
-ifneq ($(filter all _all modules,$(MAKECMDGOALS)),)
+ifneq ($(filter all _all modules nsdeps,$(MAKECMDGOALS)),)
   KBUILD_MODULES := 1
 endif
 
-- 
2.17.1

Re: [PATCH 1/7] modpost: fix broken sym->namespace for external module builds

[Masahiro Yamada]

On Fri, Sep 27, 2019 at 6:37 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> Currently, external module builds produce tons of false-positives:
>
>   WARNING: module <mod> uses symbol <sym> from namespace <ns>, but does not import it.
>
> Here, the <ns> part shows a random string.
>
> When you build external modules, the symbol info of vmlinux and
> in-kernel modules are read from $(objtree)/Module.symvers, but
> read_dump() is buggy in multiple ways:
>
> [1] When the modpost is run for vmlinux and in-kernel modules,
> sym_extract_namespace() correctly allocates memory for the namespace.
> On the other hand, read_dump() does not, then sym->namespace will
> point to somewhere in the line buffer of get_next_line(). The data
> in the buffer will be replaced soon, and sym->namespace will end up
> with pointing to unrelated data. As a result, check_exports() will
> show random strings in the warning messages.
>
> [2] When there is no namespace, sym_extract_namespace() returns NULL.
> On the other hand, read_dump() sets namespace to an empty string "".
> (but, it will be later replaced with unrelated data due to bug [1].)
> The check_exports() shows a warning unless exp->namespace is NULL,
> so every symbol read from read_dump() emits the warning, which is
> mostly false positive.
>
> To address [1], I added NOFAIL(strdup(...)) to allocate memory.
> For [2], I changed the if-conditional in check_exports().
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>

Fixes: cb9b55d21fe0 ("modpost: add support for symbol namespaces")


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 1/7] modpost: fix broken sym->namespace for external module builds

[Matthias Maennich]

On Fri, Sep 27, 2019 at 06:35:57PM +0900, Masahiro Yamada wrote:
>Currently, external module builds produce tons of false-positives:
>
>  WARNING: module <mod> uses symbol <sym> from namespace <ns>, but does not import it.
>
>Here, the <ns> part shows a random string.
>
>When you build external modules, the symbol info of vmlinux and
>in-kernel modules are read from $(objtree)/Module.symvers, but
>read_dump() is buggy in multiple ways:
>
>[1] When the modpost is run for vmlinux and in-kernel modules,
>sym_extract_namespace() correctly allocates memory for the namespace.
>On the other hand, read_dump() does not, then sym->namespace will
>point to somewhere in the line buffer of get_next_line(). The data
>in the buffer will be replaced soon, and sym->namespace will end up
>with pointing to unrelated data. As a result, check_exports() will
>show random strings in the warning messages.
>
>[2] When there is no namespace, sym_extract_namespace() returns NULL.
>On the other hand, read_dump() sets namespace to an empty string "".
>(but, it will be later replaced with unrelated data due to bug [1].)
>The check_exports() shows a warning unless exp->namespace is NULL,
>so every symbol read from read_dump() emits the warning, which is
>mostly false positive.
>
>To address [1], I added NOFAIL(strdup(...)) to allocate memory.
>For [2], I changed the if-conditional in check_exports().


Thanks for addressing this. Greg had reported this earlier this week and
Shaun was proposing a fix earlier today. Shaun's fix also ensures that
memory is released when updating the namespace. But judging from the
code around 'symbolhash' it seems that leaking this is accepted for
modpost. Not sure about that. Having said that, please feel free to add

Reviewed-by: Matthias Maennich <maennich@google.com>

Cheers,
Matthias

>Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>---
>
> scripts/mod/modpost.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
>index 3961941e8e7a..5c628a7d80f7 100644
>--- a/scripts/mod/modpost.c
>+++ b/scripts/mod/modpost.c
>@@ -2195,7 +2195,7 @@ static int check_exports(struct module *mod)
> 		else
> 			basename = mod->name;
>
>-		if (exp->namespace) {
>+		if (exp->namespace && exp->namespace[0]) {
> 			add_namespace(&mod->required_namespaces,
> 				      exp->namespace);
>
>@@ -2453,7 +2453,7 @@ static void read_dump(const char *fname, unsigned int kernel)
> 			mod = new_module(modname);
> 			mod->skip = 1;
> 		}
>-		s = sym_add_exported(symname, namespace, mod,
>+		s = sym_add_exported(symname, NOFAIL(strdup(namespace)), mod,
> 				     export_no(export));
> 		s->kernel    = kernel;
> 		s->preloaded = 1;
>-- 
>2.17.1
>

Re: [PATCH 2/7] module: swap the order of symbol.namespace

[Matthias Maennich]

On Fri, Sep 27, 2019 at 06:35:58PM +0900, Masahiro Yamada wrote:
>Currently, EXPORT_SYMBOL_NS(_GPL) constructs the kernel symbol as
>follows:
>
>  __ksymtab_SYMBOL.NAMESPACE
>
>The sym_extract_namespace() in modpost allocates memory for the part
>SYMBOL.NAMESPACE when '.' is contained. One problem is that the pointer
>returned by strdup() is lost because the symbol name will be copied to
>malloc'ed memory by alloc_symbol(). No one will keep track of the
>pointer of strdup'ed memory.
>
>sym->namespace still points to the NAMESPACE part. So, if you like,
>you can free it with complicated code like this:
>
>   free(sym->namespace - strlen(sym->name) - 1);
>
>I would not say it is fatal since we seldom bother to manually free
>memory in host programs. But, I can fix it in an elegant way.
>
>I swapped the order of the symbol and the namespace as follows:
>
>  __ksymtab_NAMESPACE.SYMBOL
>
>then, simplified sym_extract_namespace() so that it allocates memory
>only for the NAMESPACE part.
>
>I prefer this order because it is intuitive and also matches to major
>languages. For example, NAMESPACE::NAME in C++, MODULE.NAME in Python.

I agree with this rationale and like the implementation. I believe the
idea of appending the namespace came from being afraid of other tools
facing the problem of parsing the namespace out of the middle of the
entry. Thanks for this improvement.

Reviewed-by: Matthias Maennich <maennich@google.com>

Cheers,
Matthias

>Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>---
>
> include/linux/export.h |  4 ++--
> scripts/mod/modpost.c  | 16 +++++++---------
> 2 files changed, 9 insertions(+), 11 deletions(-)
>
>diff --git a/include/linux/export.h b/include/linux/export.h
>index 95f55b7f83a0..0695d4e847d9 100644
>--- a/include/linux/export.h
>+++ b/include/linux/export.h
>@@ -52,7 +52,7 @@ extern struct module __this_module;
> 	__ADDRESSABLE(sym)						\
> 	asm("	.section \"___ksymtab" sec "+" #sym "\", \"a\"	\n"	\
> 	    "	.balign	4					\n"	\
>-	    "__ksymtab_" #sym NS_SEPARATOR #ns ":		\n"	\
>+	    "__ksymtab_" #ns NS_SEPARATOR #sym ":		\n"	\
> 	    "	.long	" #sym "- .				\n"	\
> 	    "	.long	__kstrtab_" #sym "- .			\n"	\
> 	    "	.long	__kstrtab_ns_" #sym "- .		\n"	\
>@@ -76,7 +76,7 @@ struct kernel_symbol {
> #else
> #define __KSYMTAB_ENTRY_NS(sym, sec, ns)				\
> 	static const struct kernel_symbol __ksymtab_##sym##__##ns	\
>-	asm("__ksymtab_" #sym NS_SEPARATOR #ns)				\
>+	asm("__ksymtab_" #ns NS_SEPARATOR #sym)				\
> 	__attribute__((section("___ksymtab" sec "+" #sym), used))	\
> 	__aligned(sizeof(void *))					\
> 	= { (unsigned long)&sym, __kstrtab_##sym, __kstrtab_ns_##sym }
>diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
>index 5c628a7d80f7..d171b0cffb05 100644
>--- a/scripts/mod/modpost.c
>+++ b/scripts/mod/modpost.c
>@@ -350,18 +350,16 @@ static enum export export_from_sec(struct elf_info *elf, unsigned int sec)
>
> static const char *sym_extract_namespace(const char **symname)
> {
>-	size_t n;
>-	char *dupsymname;
>+	char *namespace = NULL;
>+	char *ns_separator;
>
>-	n = strcspn(*symname, ".");
>-	if (n < strlen(*symname) - 1) {
>-		dupsymname = NOFAIL(strdup(*symname));
>-		dupsymname[n] = '\0';
>-		*symname = dupsymname;
>-		return dupsymname + n + 1;
>+	ns_separator = strchr(*symname, '.');
>+	if (ns_separator) {
>+		namespace = NOFAIL(strndup(*symname, ns_separator - *symname));
>+		*symname = ns_separator + 1;
> 	}
>
>-	return NULL;
>+	return namespace;
> }
>
> /**
>-- 
>2.17.1
>

Re: [PATCH 5/7] kbuild: fix build error of 'make nsdeps' in clean tree

[Matthias Maennich]

On Fri, Sep 27, 2019 at 06:36:01PM +0900, Masahiro Yamada wrote:
>Running 'make nsdeps' in a clean source tree fails as follows:
>
>$ make -s clean; make -s defconfig; make nsdeps
>   [ snip ]
>awk: fatal: cannot open file `init/modules.order' for reading (No such file or directory)
>make: *** [Makefile;1307: modules.order] Error 2
>make: *** Deleting file 'modules.order'
>make: *** Waiting for unfinished jobs....
>
>The cause of the error is 'make nsdeps' does not build modules at all.
>Set KBUILD_MODULES to fix it.

You reported that issue earlier, but having nsdeps depend on modules
(see Makefile:1708) resolved that for me. I wonder what I missed. But I
won't disagree with you on kbuild advise. :-)

Reviewed-by: Matthias Maennich <maennich@google.com>

Cheers,
Matthias

>Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>---
>
> Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/Makefile b/Makefile
>index d456746da347..80ba8efd56bb 100644
>--- a/Makefile
>+++ b/Makefile
>@@ -616,7 +616,7 @@ endif
> # in addition to whatever we do anyway.
> # Just "make" or "make all" shall build modules as well
>
>-ifneq ($(filter all _all modules,$(MAKECMDGOALS)),)
>+ifneq ($(filter all _all modules nsdeps,$(MAKECMDGOALS)),)
>   KBUILD_MODULES := 1
> endif
>
>-- 
>2.17.1
>

Re: [PATCH 0/7] module: various bug-fixes and clean-ups for module namespace

[Matthias Maennich]

On Fri, Sep 27, 2019 at 06:35:56PM +0900, Masahiro Yamada wrote:
>
>I was hit by some problems caused by the module namespace feature
>that was merged recently. At least, the breakage of
>external module builds is a fatal one. I just took a look at the code
>closer, and I noticed some more issues and improvements.
>
>I hope these patches are mostly OK.
>The 4th patch might have room for argument since it is a trade-off
>of "cleaner implermentation" vs "code size".
>
Thanks Masahiro for taking the time to improve the implementation of the
symbol namespaces. These are all good points that you addressed!

For [04/07], I can work on this if you do not mind.

Cheers,
Matthias

>
>Masahiro Yamada (7):
>  modpost: fix broken sym->namespace for external module builds
>  module: swap the order of symbol.namespace
>  module: rename __kstrtab_ns_* to __kstrtabns_* to avoid symbol
>    conflict
>  module: avoid code duplication in include/linux/export.h
>  kbuild: fix build error of 'make nsdeps' in clean tree
>  nsdeps: fix hashbang of scripts/nsdeps
>  nsdeps: make generated patches independent of locale
>
> Makefile               |   2 +-
> include/linux/export.h | 104 ++++++++++++++---------------------------
> kernel/module.c        |   2 +-
> scripts/mod/modpost.c  |  20 ++++----
> scripts/nsdeps         |   4 +-
> 5 files changed, 47 insertions(+), 85 deletions(-)
>
>-- 
>2.17.1
>

Re: [PATCH 0/7] module: various bug-fixes and clean-ups for module namespace

[Masahiro Yamada]

Hi Matthias,

On Fri, Sep 27, 2019 at 10:41 PM Matthias Maennich <maennich@google.com> wrote:
>
> On Fri, Sep 27, 2019 at 06:35:56PM +0900, Masahiro Yamada wrote:
> >
> >I was hit by some problems caused by the module namespace feature
> >that was merged recently. At least, the breakage of
> >external module builds is a fatal one. I just took a look at the code
> >closer, and I noticed some more issues and improvements.
> >
> >I hope these patches are mostly OK.
> >The 4th patch might have room for argument since it is a trade-off
> >of "cleaner implermentation" vs "code size".
> >
> Thanks Masahiro for taking the time to improve the implementation of the
> symbol namespaces. These are all good points that you addressed!
>
> For [04/07], I can work on this if you do not mind.


Please feel free to.

Thanks for your review.


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 0/7] module: various bug-fixes and clean-ups for module namespace

[Jessica Yu]

+++ Matthias Maennich [27/09/19 14:41 +0100]:
>On Fri, Sep 27, 2019 at 06:35:56PM +0900, Masahiro Yamada wrote:
>>
>>I was hit by some problems caused by the module namespace feature
>>that was merged recently. At least, the breakage of
>>external module builds is a fatal one. I just took a look at the code
>>closer, and I noticed some more issues and improvements.
>>
>>I hope these patches are mostly OK.
>>The 4th patch might have room for argument since it is a trade-off
>>of "cleaner implermentation" vs "code size".
>>
>Thanks Masahiro for taking the time to improve the implementation of the
>symbol namespaces. These are all good points that you addressed!

Agreed, thanks Masahiro for fixing up all the rough edges! Your series
of fixes look good to me, I will queue this up on modules-next this
week with the exception of patch 4 - Matthias, you are planning to
submit a patch that would supercede patch 04/07, right?

Thanks!

Jessica

Re: [PATCH 0/7] module: various bug-fixes and clean-ups for module namespace

[Matthias Maennich]

On Wed, Oct 02, 2019 at 08:57:02PM +0200, Jessica Yu wrote:
>+++ Matthias Maennich [27/09/19 14:41 +0100]:
>>On Fri, Sep 27, 2019 at 06:35:56PM +0900, Masahiro Yamada wrote:
>>>
>>>I was hit by some problems caused by the module namespace feature
>>>that was merged recently. At least, the breakage of
>>>external module builds is a fatal one. I just took a look at the code
>>>closer, and I noticed some more issues and improvements.
>>>
>>>I hope these patches are mostly OK.
>>>The 4th patch might have room for argument since it is a trade-off
>>>of "cleaner implermentation" vs "code size".
>>>
>>Thanks Masahiro for taking the time to improve the implementation of the
>>symbol namespaces. These are all good points that you addressed!
>
>Agreed, thanks Masahiro for fixing up all the rough edges! Your series
>of fixes look good to me, I will queue this up on modules-next this
>week with the exception of patch 4 - Matthias, you are planning to
>submit a patch that would supercede patch 04/07, right?

I will most likely create a patch on top of 04/07 and will submit
everything as a separate series.

Cheers,
Matthias

Re: [PATCH 0/7] module: various bug-fixes and clean-ups for module namespace

[Masahiro Yamada]

H Jessica,

On Thu, Oct 3, 2019 at 3:57 AM Jessica Yu <jeyu@kernel.org> wrote:
>
> +++ Matthias Maennich [27/09/19 14:41 +0100]:
> >On Fri, Sep 27, 2019 at 06:35:56PM +0900, Masahiro Yamada wrote:
> >>
> >>I was hit by some problems caused by the module namespace feature
> >>that was merged recently. At least, the breakage of
> >>external module builds is a fatal one. I just took a look at the code
> >>closer, and I noticed some more issues and improvements.
> >>
> >>I hope these patches are mostly OK.
> >>The 4th patch might have room for argument since it is a trade-off
> >>of "cleaner implermentation" vs "code size".
> >>
> >Thanks Masahiro for taking the time to improve the implementation of the
> >symbol namespaces. These are all good points that you addressed!
>
> Agreed, thanks Masahiro for fixing up all the rough edges! Your series
> of fixes look good to me, I will queue this up on modules-next this
> week

Since these are bug fixes,
please send them before v5.4.

Thanks.



> with the exception of patch 4 - Matthias, you are planning to
> submit a patch that would supercede patch 04/07, right?
>
> Thanks!
>
> Jessica



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 0/7] module: various bug-fixes and clean-ups for module namespace

[Masahiro Yamada]

Hi Jessica,

On Thu, Oct 3, 2019 at 3:57 AM Jessica Yu <jeyu@kernel.org> wrote:
>
> +++ Matthias Maennich [27/09/19 14:41 +0100]:
> >On Fri, Sep 27, 2019 at 06:35:56PM +0900, Masahiro Yamada wrote:
> >>
> >>I was hit by some problems caused by the module namespace feature
> >>that was merged recently. At least, the breakage of
> >>external module builds is a fatal one. I just took a look at the code
> >>closer, and I noticed some more issues and improvements.
> >>
> >>I hope these patches are mostly OK.
> >>The 4th patch might have room for argument since it is a trade-off
> >>of "cleaner implermentation" vs "code size".
> >>
> >Thanks Masahiro for taking the time to improve the implementation of the
> >symbol namespaces. These are all good points that you addressed!
>
> Agreed, thanks Masahiro for fixing up all the rough edges! Your series
> of fixes look good to me, I will queue this up on modules-next this
> week with the exception of patch 4 - Matthias, you are planning to
> submit a patch that would supercede patch 04/07, right?
>
> Thanks!


I missed to fix one issue in v1.
sym_add_exported() misses to set s->namespace
if the struct symbol is already created by
read_dump() or sym_update_crc().


So, I just sent v2.

-- 
Best Regards
Masahiro Yamada