sorting of exports breaks modpost's GPL checking

sorting of exports breaks modpost's GPL checking

[Jan Beulich]

Is it known (and being worked on) that exported symbols residing
themselves in modules are no longer properly handled by modpost,
due to the .o files to be linked into .ko (other than vmlinux and the
.ko files) still carrying the ___ksymtab*+<symbol> sections?

If not - what are the thoughts to resolve this? It's not really obvious,
since
- doing the section sorting already when generating the .o-s is
  incompatible (at least) with .ko-s consisting of just a single .o,
- passing the .ko-s instead of the .o-s to modpost would create a
  chicken-and-egg situation, and
- adjusting modpost itself would require the tool to track all the
  ___ksymtab*+<symbol> section (i.e. causing possibly measurable
  extra overhead).

Thanks, Jan

Re: sorting of exports breaks modpost's GPL checking

[Alessio Igor Bogani]

Dear Mr. Beulich,

2011/7/4 Jan Beulich <JBeulich@novell.com>:
> Is it known (and being worked on) that exported symbols residing
> themselves in modules are no longer properly handled by modpost,
> due to the .o files to be linked into .ko (other than vmlinux and the
> .ko files) still carrying the ___ksymtab*+<symbol> sections?
>
> If not - what are the thoughts to resolve this?
[...]

Could you tell me how reproduce the problem?

Thanks!

Ciao,
Alessio

Re: sorting of exports breaks modpost's GPL checking

[Jan Beulich]

>>> On 05.07.11 at 11:36, Alessio Igor Bogani <abogani@kernel.org> wrote:
> Dear Mr. Beulich,
> 
> 2011/7/4 Jan Beulich <JBeulich@novell.com>:
>> Is it known (and being worked on) that exported symbols residing
>> themselves in modules are no longer properly handled by modpost,
>> due to the .o files to be linked into .ko (other than vmlinux and the
>> .ko files) still carrying the ___ksymtab*+<symbol> sections?
>>
>> If not - what are the thoughts to resolve this?
> [...]
> 
> Could you tell me how reproduce the problem?

Just look at Modules.symvers - all the symbols coming from modules
will be tagged "(unknown)" rather than EXPORT_SYMBOL,
EXPORT_SYMBOL_GPL, etc.

As to reproducing the actual bad behavior - creating a non-GPL
module that consumes a GPL-only export should be all you need.

Jan

Re: sorting of exports breaks modpost's GPL checking

[Rusty Russell]

On Tue, 05 Jul 2011 10:47:55 +0100, "Jan Beulich" <JBeulich@novell.com> wrote:
> >>> On 05.07.11 at 11:36, Alessio Igor Bogani <abogani@kernel.org> wrote:
> > Dear Mr. Beulich,
> > 
> > 2011/7/4 Jan Beulich <JBeulich@novell.com>:
> >> Is it known (and being worked on) that exported symbols residing
> >> themselves in modules are no longer properly handled by modpost,
> >> due to the .o files to be linked into .ko (other than vmlinux and the
> >> .ko files) still carrying the ___ksymtab*+<symbol> sections?
> >>
> >> If not - what are the thoughts to resolve this?
> > [...]
> > 
> > Could you tell me how reproduce the problem?
> 
> Just look at Modules.symvers - all the symbols coming from modules
> will be tagged "(unknown)" rather than EXPORT_SYMBOL,
> EXPORT_SYMBOL_GPL, etc.
> 
> As to reproducing the actual bad behavior - creating a non-GPL
> module that consumes a GPL-only export should be all you need.
> 
> Jan

We could just drop this detection.  It's a courtesy, after all: the gpl
stuff is enforced at runtime.

Thoughts?
Rusty.

Re: sorting of exports breaks modpost's GPL checking

[Jan Beulich]

>>> On 07.07.11 at 03:03, Rusty Russell <rusty@rustcorp.com.au> wrote:
> On Tue, 05 Jul 2011 10:47:55 +0100, "Jan Beulich" <JBeulich@novell.com> wrote:
>> >>> On 05.07.11 at 11:36, Alessio Igor Bogani <abogani@kernel.org> wrote:
>> > Dear Mr. Beulich,
>> > 
>> > 2011/7/4 Jan Beulich <JBeulich@novell.com>:
>> >> Is it known (and being worked on) that exported symbols residing
>> >> themselves in modules are no longer properly handled by modpost,
>> >> due to the .o files to be linked into .ko (other than vmlinux and the
>> >> .ko files) still carrying the ___ksymtab*+<symbol> sections?
>> >>
>> >> If not - what are the thoughts to resolve this?
>> > [...]
>> > 
>> > Could you tell me how reproduce the problem?
>> 
>> Just look at Modules.symvers - all the symbols coming from modules
>> will be tagged "(unknown)" rather than EXPORT_SYMBOL,
>> EXPORT_SYMBOL_GPL, etc.
>> 
>> As to reproducing the actual bad behavior - creating a non-GPL
>> module that consumes a GPL-only export should be all you need.
>> 
>> Jan
> 
> We could just drop this detection.  It's a courtesy, after all: the gpl
> stuff is enforced at runtime.
> 
> Thoughts?

Especially for the purpose of distros I think it is quite valuable to
know at build time whether there's any inconsistency that would
result in runtime problems (and you certainly don't expect that
testing would manage to cover each and every obscure module).

Jan

> Rusty.

Re: sorting of exports breaks modpost's GPL checking

[Alessio Igor Bogani]

Dear Mr. Beulich,

2011/7/7 Jan Beulich <JBeulich@novell.com>:
[...]
> Especially for the purpose of distros I think it is quite valuable to
> know at build time whether there's any inconsistency that would
> result in runtime problems (and you certainly don't expect that
> testing would manage to cover each and every obscure module).

I have just started working on this issue. I hope to fix as soon as
possible but the modpost.c's code isn't trivial so I'll could take
some days.

Any help by more experienced developers is appreciated.

Thanks!

Ciao,
Alessio

[PATCH] modpost: Fix modpost's license checking

[Alessio Igor Bogani]

The commit f02e8a6 sorts symbols placing each of them in its own elf section.
The sorting and merging into the canonical sections are done by the linker.
Unfortunately modpost to generate Module.symvers file parses vmlinux
(already linked) and all modules object files (which aren't linked yet).
These aren't sanitized by the linker yet. That breaks modpost that can't
detect license properly for modules.

This patch makes modpost aware of  the new exported symbols structure.

This work was supported by a hardware donation from the CE Linux Forum.

Signed-off-by: Alessio Igor Bogani <abogani@kernel.org>
---
 scripts/mod/modpost.c |   27 ++++++++++++++++++++++++++-
 1 files changed, 26 insertions(+), 1 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 413c536..f41283c 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -254,6 +254,26 @@ static enum export export_no(const char *s)
 	return export_unknown;
 }
 
+static const char *sec_name(struct elf_info *elf, int secindex);
+
+static enum export export_from_secname(struct elf_info *elf, unsigned int sec)
+{
+	const char *secname = sec_name(elf, sec);
+
+	if (strncmp(secname, "___ksymtab+", 11) == 0)
+		return export_plain;
+	else if (strncmp(secname, "___ksymtab_unused+", 18) == 0)
+		return export_unused;
+	else if (strncmp(secname, "___ksymtab_gpl+", 15) == 0)
+		return export_gpl;
+	else if (strncmp(secname, "___ksymtab_unused_gpl+", 22) == 0)
+		return export_unused_gpl;
+	else if (strncmp(secname, "___ksymtab_gpl_future+", 22) == 0)
+		return export_gpl_future;
+	else
+		return export_unknown;
+}
+
 static enum export export_from_sec(struct elf_info *elf, unsigned int sec)
 {
 	if (sec == elf->export_sec)
@@ -563,7 +583,12 @@ static void handle_modversions(struct module *mod, struct elf_info *info,
 			       Elf_Sym *sym, const char *symname)
 {
 	unsigned int crc;
-	enum export export = export_from_sec(info, get_secindex(info, sym));
+	enum export export;
+
+	if (!is_vmlinux(mod->name) && strncmp(symname, "__ksymtab", 9) == 0)
+		export = export_from_secname(info, get_secindex(info, sym));
+	else
+		export = export_from_sec(info, get_secindex(info, sym));
 
 	switch (sym->st_shndx) {
 	case SHN_COMMON:
-- 
1.7.4.1

Re: [PATCH] modpost: Fix modpost's license checking

[Arnaud Lacombe]

Hi,

On Sat, Jul 9, 2011 at 7:13 PM, Alessio Igor Bogani <abogani@kernel.org> wrote:
> The commit f02e8a6 sorts symbols placing each of them in its own elf section.
> The sorting and merging into the canonical sections are done by the linker.
> Unfortunately modpost to generate Module.symvers file parses vmlinux
> (already linked) and all modules object files (which aren't linked yet).
> These aren't sanitized by the linker yet. That breaks modpost that can't
> detect license properly for modules.
>
> This patch makes modpost aware of  the new exported symbols structure.
>
> This work was supported by a hardware donation from the CE Linux Forum.
>
> Signed-off-by: Alessio Igor Bogani <abogani@kernel.org>
> ---
>  scripts/mod/modpost.c |   27 ++++++++++++++++++++++++++-
>  1 files changed, 26 insertions(+), 1 deletions(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 413c536..f41283c 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -254,6 +254,26 @@ static enum export export_no(const char *s)
>        return export_unknown;
>  }
>
> +static const char *sec_name(struct elf_info *elf, int secindex);
> +
> +static enum export export_from_secname(struct elf_info *elf, unsigned int sec)
> +{
> +       const char *secname = sec_name(elf, sec);
> +
> +       if (strncmp(secname, "___ksymtab+", 11) == 0)
I'd suppose 11 here is `sizeof "___ksymtab+" - 1' ? Why not making
that explicit though a macro, ala:

#define SEC_NAME_MATCH(name, str) (strncmp(name, str, strlen(str) - 1) == 0)

This would avoid bug in the future if the string is updated but not
the size, and/or copy paste error where only the string is changed.

 - Arnaud

> +               return export_plain;
> +       else if (strncmp(secname, "___ksymtab_unused+", 18) == 0)
> +               return export_unused;
> +       else if (strncmp(secname, "___ksymtab_gpl+", 15) == 0)
> +               return export_gpl;
> +       else if (strncmp(secname, "___ksymtab_unused_gpl+", 22) == 0)
> +               return export_unused_gpl;
> +       else if (strncmp(secname, "___ksymtab_gpl_future+", 22) == 0)
> +               return export_gpl_future;
> +       else
> +               return export_unknown;
> +}
> +
>  static enum export export_from_sec(struct elf_info *elf, unsigned int sec)
>  {
>        if (sec == elf->export_sec)
> @@ -563,7 +583,12 @@ static void handle_modversions(struct module *mod, struct elf_info *info,
>                               Elf_Sym *sym, const char *symname)
>  {
>        unsigned int crc;
> -       enum export export = export_from_sec(info, get_secindex(info, sym));
> +       enum export export;
> +
> +       if (!is_vmlinux(mod->name) && strncmp(symname, "__ksymtab", 9) == 0)
> +               export = export_from_secname(info, get_secindex(info, sym));
> +       else
> +               export = export_from_sec(info, get_secindex(info, sym));
>
>        switch (sym->st_shndx) {
>        case SHN_COMMON:
> --
> 1.7.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

[PATCH] modpost: Fix modpost's license checking V2

[Alessio Igor Bogani]

The commit f02e8a6 sorts symbols placing each of them in its own elf section.
The sorting and merging into the canonical sections are done by the linker.
Unfortunately modpost to generate Module.symvers file parses vmlinux
(already linked) and all modules object files (which aren't linked yet).
These aren't sanitized by the linker yet. That breaks modpost that can't
detect license properly for modules. This patch makes modpost aware of
the new exported symbols structure. Thanks to Arnaud Lacombe
<lacombar@gmail.com> for providing useful suggestion about code.

This work was supported by a hardware donation from the CE Linux Forum.

Reported-by: Jan Beulich <jbeulich@novell.com>
Signed-off-by: Alessio Igor Bogani <abogani@kernel.org>
---
 scripts/mod/modpost.c |   29 ++++++++++++++++++++++++++++-
 1 files changed, 28 insertions(+), 1 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 413c536..348db13 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -254,6 +254,28 @@ static enum export export_no(const char *s)
 	return export_unknown;
 }
 
+static const char *sec_name(struct elf_info *elf, int secindex);
+
+#define SEC_NAME_MATCH(name, str) (strncmp(name, str, strlen(str) - 1) == 0)
+
+static enum export export_from_secname(struct elf_info *elf, unsigned int sec)
+{
+	const char *secname = sec_name(elf, sec);
+
+	if (SEC_NAME_MATCH(secname, "___ksymtab+"))
+		return export_plain;
+	else if (SEC_NAME_MATCH(secname, "___ksymtab_unused+"))
+		return export_unused;
+	else if (SEC_NAME_MATCH(secname, "___ksymtab_gpl+"))
+		return export_gpl;
+	else if (SEC_NAME_MATCH(secname, "___ksymtab_unused_gpl+"))
+		return export_unused_gpl;
+	else if (SEC_NAME_MATCH(secname, "___ksymtab_gpl_future+"))
+		return export_gpl_future;
+	else
+		return export_unknown;
+}
+
 static enum export export_from_sec(struct elf_info *elf, unsigned int sec)
 {
 	if (sec == elf->export_sec)
@@ -563,7 +585,12 @@ static void handle_modversions(struct module *mod, struct elf_info *info,
 			       Elf_Sym *sym, const char *symname)
 {
 	unsigned int crc;
-	enum export export = export_from_sec(info, get_secindex(info, sym));
+	enum export export;
+
+	if (!is_vmlinux(mod->name) && strncmp(symname, "__ksymtab", 9) == 0)
+		export = export_from_secname(info, get_secindex(info, sym));
+	else
+		export = export_from_sec(info, get_secindex(info, sym));
 
 	switch (sym->st_shndx) {
 	case SHN_COMMON:
-- 
1.7.4.1

Re: [PATCH] modpost: Fix modpost's license checking V2

[Anders Kaseorg]

On Tue, Jul 12, 2011 at 03:00, Alessio Igor Bogani <abogani@kernel.org> wrote:
> +#define SEC_NAME_MATCH(name, str) (strncmp(name, str, strlen(str) - 1) == 0)

Surely you meant strlen() or sizeof() - 1, not strlen() - 1?

> +       if (!is_vmlinux(mod->name) && strncmp(symname, "__ksymtab", 9) == 0)

Same strncmp pattern here.  BTW, your macro is the strstarts()
function in include/linux/string.h.

Anders

Re: [PATCH] modpost: Fix modpost's license checking V2

[Arnaud Lacombe]

Hi,

On Tue, Jul 12, 2011 at 2:02 PM, Anders Kaseorg <andersk@ksplice.com> wrote:
> On Tue, Jul 12, 2011 at 03:00, Alessio Igor Bogani <abogani@kernel.org> wrote:
>> +#define SEC_NAME_MATCH(name, str) (strncmp(name, str, strlen(str) - 1) == 0)
>
> Surely you meant strlen() or sizeof() - 1, not strlen() - 1?
>
>> +       if (!is_vmlinux(mod->name) && strncmp(symname, "__ksymtab", 9) == 0)
>
> Same strncmp pattern here.  BTW, your macro is the strstarts()
> function in include/linux/string.h.
>
modpost is an host tool, it should not include kernel header. Content
of this file is not exported to userland anyway.

 - Arnaud

 - Arnaud

Re: [PATCH] modpost: Fix modpost's license checking V2

[Anders Kaseorg]

On Tue, Jul 12, 2011 at 14:15, Arnaud Lacombe <lacombar@gmail.com> wrote:
> modpost is an host tool, it should not include kernel header. Content
> of this file is not exported to userland anyway.

My point was more that you might as well reuse the same name and
probably the same code so that people don’t have to look carefully at
a new macro to decide whether it’s off-by-one from what they expect.

Anders

Re: [PATCH] modpost: Fix modpost's license checking V2

[Arnaud Lacombe]

Hi,

On Tue, Jul 12, 2011 at 2:35 PM, Anders Kaseorg <andersk@ksplice.com> wrote:
> On Tue, Jul 12, 2011 at 14:15, Arnaud Lacombe <lacombar@gmail.com> wrote:
>> modpost is an host tool, it should not include kernel header. Content
>> of this file is not exported to userland anyway.
>
> My point was more that you might as well reuse the same name and
> probably the same code so that people don’t have to look carefully at
> a new macro to decide whether it’s off-by-one from what they expect.
>
Agree. I was not entirely sure of what you meant, ie. whether
including "include/linux/string.h" or adjusting the name and
definition to match the existing. So I thought it would just be better
to clear any potential misunderstanding :)

 - Arnaud

[PATCH] modpost: Fix modpost's license checking V3

[Alessio Igor Bogani]

The commit f02e8a6 sorts symbols placing each of them in its own elf section.
The sorting and merging into the canonical sections are done by the linker.
Unfortunately modpost to generate Module.symvers file parses vmlinux
(already linked) and all modules object files (which aren't linked yet).
These aren't sanitized by the linker yet. That breaks modpost that can't
detect license properly for modules. This patch makes modpost aware of
the new exported symbols structure.

Thanks to Arnaud Lacombe <lacombar@gmail.com> and Anders Kaseorg
<andersk@ksplice.com> for providing useful suggestions about code.

This work was supported by a hardware donation from the CE Linux Forum.

Reported-by: Jan Beulich <jbeulich@novell.com>
Signed-off-by: Alessio Igor Bogani <abogani@kernel.org>
---
 scripts/mod/modpost.c |   29 ++++++++++++++++++++++++++++-
 1 files changed, 28 insertions(+), 1 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 413c536..a509ff8 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -254,6 +254,28 @@ static enum export export_no(const char *s)
 	return export_unknown;
 }
 
+static const char *sec_name(struct elf_info *elf, int secindex);
+
+#define strstarts(str, prefix) (strncmp(str, prefix, strlen(prefix)) == 0)
+
+static enum export export_from_secname(struct elf_info *elf, unsigned int sec)
+{
+	const char *secname = sec_name(elf, sec);
+
+	if (strstarts(secname, "___ksymtab+"))
+		return export_plain;
+	else if (strstarts(secname, "___ksymtab_unused+"))
+		return export_unused;
+	else if (strstarts(secname, "___ksymtab_gpl+"))
+		return export_gpl;
+	else if (strstarts(secname, "___ksymtab_unused_gpl+"))
+		return export_unused_gpl;
+	else if (strstarts(secname, "___ksymtab_gpl_future+"))
+		return export_gpl_future;
+	else
+		return export_unknown;
+}
+
 static enum export export_from_sec(struct elf_info *elf, unsigned int sec)
 {
 	if (sec == elf->export_sec)
@@ -563,7 +585,12 @@ static void handle_modversions(struct module *mod, struct elf_info *info,
 			       Elf_Sym *sym, const char *symname)
 {
 	unsigned int crc;
-	enum export export = export_from_sec(info, get_secindex(info, sym));
+	enum export export;
+
+	if (!is_vmlinux(mod->name) && strncmp(symname, "__ksymtab", 9) == 0)
+		export = export_from_secname(info, get_secindex(info, sym));
+	else
+		export = export_from_sec(info, get_secindex(info, sym));
 
 	switch (sym->st_shndx) {
 	case SHN_COMMON:
-- 
1.7.4.1

Re: [PATCH] modpost: Fix modpost's license checking V3

[Rusty Russell]

On Thu, 14 Jul 2011 08:51:16 +0200, Alessio Igor Bogani <abogani@kernel.org> wrote:
> The commit f02e8a6 sorts symbols placing each of them in its own elf section.
> The sorting and merging into the canonical sections are done by the linker.
> Unfortunately modpost to generate Module.symvers file parses vmlinux
> (already linked) and all modules object files (which aren't linked yet).
> These aren't sanitized by the linker yet. That breaks modpost that can't
> detect license properly for modules. This patch makes modpost aware of
> the new exported symbols structure.
> 
> Thanks to Arnaud Lacombe <lacombar@gmail.com> and Anders Kaseorg
> <andersk@ksplice.com> for providing useful suggestions about code.
> 
> This work was supported by a hardware donation from the CE Linux Forum.

Applied.

Thanks!
Rusty.

Re: [PATCH] modpost: Fix modpost's license checking V3

[Rusty Russell]

On Thu, 14 Jul 2011 08:51:16 +0200, Alessio Igor Bogani <abogani@kernel.org> wrote:
> The commit f02e8a6 sorts symbols placing each of them in its own elf section.
> The sorting and merging into the canonical sections are done by the linker.
> Unfortunately modpost to generate Module.symvers file parses vmlinux
> (already linked) and all modules object files (which aren't linked yet).
> These aren't sanitized by the linker yet. That breaks modpost that can't
> detect license properly for modules. This patch makes modpost aware of
> the new exported symbols structure.
> 
> Thanks to Arnaud Lacombe <lacombar@gmail.com> and Anders Kaseorg
> <andersk@ksplice.com> for providing useful suggestions about code.
> 
> This work was supported by a hardware donation from the CE Linux Forum.

Applied.

Thanks!
Rusty.

Re: [PATCH] modpost: Fix modpost's license checking V3

[Michal Marek]

On 14.7.2011 08:51, Alessio Igor Bogani wrote:
> The commit f02e8a6 sorts symbols placing each of them in its own elf section.
> The sorting and merging into the canonical sections are done by the linker.
> Unfortunately modpost to generate Module.symvers file parses vmlinux
> (already linked) and all modules object files (which aren't linked yet).
> These aren't sanitized by the linker yet. That breaks modpost that can't
> detect license properly for modules. This patch makes modpost aware of
> the new exported symbols structure.
>
> Thanks to Arnaud Lacombe<lacombar@gmail.com>  and Anders Kaseorg
> <andersk@ksplice.com>  for providing useful suggestions about code.
>
> This work was supported by a hardware donation from the CE Linux Forum.
>
> Reported-by: Jan Beulich<jbeulich@novell.com>
> Signed-off-by: Alessio Igor Bogani<abogani@kernel.org>
> ---
>   scripts/mod/modpost.c |   29 ++++++++++++++++++++++++++++-
>   1 files changed, 28 insertions(+), 1 deletions(-)

Rusty, will you take this patch or should I apply it to kbuild-2.6.git?

Michal


> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 413c536..a509ff8 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -254,6 +254,28 @@ static enum export export_no(const char *s)
>   	return export_unknown;
>   }
>
> +static const char *sec_name(struct elf_info *elf, int secindex);
> +
> +#define strstarts(str, prefix) (strncmp(str, prefix, strlen(prefix)) == 0)
> +
> +static enum export export_from_secname(struct elf_info *elf, unsigned int sec)
> +{
> +	const char *secname = sec_name(elf, sec);
> +
> +	if (strstarts(secname, "___ksymtab+"))
> +		return export_plain;
> +	else if (strstarts(secname, "___ksymtab_unused+"))
> +		return export_unused;
> +	else if (strstarts(secname, "___ksymtab_gpl+"))
> +		return export_gpl;
> +	else if (strstarts(secname, "___ksymtab_unused_gpl+"))
> +		return export_unused_gpl;
> +	else if (strstarts(secname, "___ksymtab_gpl_future+"))
> +		return export_gpl_future;
> +	else
> +		return export_unknown;
> +}
> +
>   static enum export export_from_sec(struct elf_info *elf, unsigned int sec)
>   {
>   	if (sec == elf->export_sec)
> @@ -563,7 +585,12 @@ static void handle_modversions(struct module *mod, struct elf_info *info,
>   			       Elf_Sym *sym, const char *symname)
>   {
>   	unsigned int crc;
> -	enum export export = export_from_sec(info, get_secindex(info, sym));
> +	enum export export;
> +
> +	if (!is_vmlinux(mod->name)&&  strncmp(symname, "__ksymtab", 9) == 0)
> +		export = export_from_secname(info, get_secindex(info, sym));
> +	else
> +		export = export_from_sec(info, get_secindex(info, sym));
>
>   	switch (sym->st_shndx) {
>   	case SHN_COMMON:

Re: [PATCH] modpost: Fix modpost's license checking V3

[Rusty Russell]

On Wed, 20 Jul 2011 17:25:18 +0200, Michal Marek <mmarek@suse.cz> wrote:
> On 14.7.2011 08:51, Alessio Igor Bogani wrote:
> > The commit f02e8a6 sorts symbols placing each of them in its own elf section.
> > The sorting and merging into the canonical sections are done by the linker.
> > Unfortunately modpost to generate Module.symvers file parses vmlinux
> > (already linked) and all modules object files (which aren't linked yet).
> > These aren't sanitized by the linker yet. That breaks modpost that can't
> > detect license properly for modules. This patch makes modpost aware of
> > the new exported symbols structure.
> >
> > Thanks to Arnaud Lacombe<lacombar@gmail.com>  and Anders Kaseorg
> > <andersk@ksplice.com>  for providing useful suggestions about code.
> >
> > This work was supported by a hardware donation from the CE Linux Forum.
> >
> > Reported-by: Jan Beulich<jbeulich@novell.com>
> > Signed-off-by: Alessio Igor Bogani<abogani@kernel.org>
> > ---
> >   scripts/mod/modpost.c |   29 ++++++++++++++++++++++++++++-
> >   1 files changed, 28 insertions(+), 1 deletions(-)
> 
> Rusty, will you take this patch or should I apply it to kbuild-2.6.git?

I took it.

Thanks,
Rusty.

Re: [PATCH] modpost: Fix modpost's license checking V3

[Frank Rowand]

On 07/13/11 23:51, Alessio Igor Bogani wrote:
> The commit f02e8a6 sorts symbols placing each of them in its own elf section.
> The sorting and merging into the canonical sections are done by the linker.
> Unfortunately modpost to generate Module.symvers file parses vmlinux

Yet another unfortunately: modpost parses vmlinux.o instead of vmlinux (vmlinux
does not yet exist at this point of the build).  vmlinux.o also does not have
the many sections sorted and merged into the canonical sections.  As a result,
the Module.symvers created my modpost incorrectly reports the license of all
exports as "(unknown)".

Can you fix this also please?


> (already linked) and all modules object files (which aren't linked yet).
> These aren't sanitized by the linker yet. That breaks modpost that can't
> detect license properly for modules. This patch makes modpost aware of
> the new exported symbols structure.
> 
> Thanks to Arnaud Lacombe <lacombar@gmail.com> and Anders Kaseorg
> <andersk@ksplice.com> for providing useful suggestions about code.
> 
> This work was supported by a hardware donation from the CE Linux Forum.
> 
> Reported-by: Jan Beulich <jbeulich@novell.com>
> Signed-off-by: Alessio Igor Bogani <abogani@kernel.org>
> ---
>  scripts/mod/modpost.c |   29 ++++++++++++++++++++++++++++-
>  1 files changed, 28 insertions(+), 1 deletions(-)

< snip >

-Frank Rowand

Re: [PATCH] modpost: Fix modpost's license checking V3

[Frank Rowand]

On 03/23/12 19:04, Frank Rowand wrote:
> On 07/13/11 23:51, Alessio Igor Bogani wrote:
>> The commit f02e8a6 sorts symbols placing each of them in its own elf section.
>> The sorting and merging into the canonical sections are done by the linker.
>> Unfortunately modpost to generate Module.symvers file parses vmlinux
> 
> Yet another unfortunately: modpost parses vmlinux.o instead of vmlinux (vmlinux
> does not yet exist at this point of the build).  vmlinux.o also does not have
> the many sections sorted and merged into the canonical sections.  As a result,
> the Module.symvers created my modpost incorrectly reports the license of all

                             ^^^ s/my/by/

> exports as "(unknown)".
> 
> Can you fix this also please?
> 
> 
>> (already linked) and all modules object files (which aren't linked yet).
>> These aren't sanitized by the linker yet. That breaks modpost that can't
>> detect license properly for modules. This patch makes modpost aware of
>> the new exported symbols structure.
>>
>> Thanks to Arnaud Lacombe <lacombar@gmail.com> and Anders Kaseorg
>> <andersk@ksplice.com> for providing useful suggestions about code.
>>
>> This work was supported by a hardware donation from the CE Linux Forum.
>>
>> Reported-by: Jan Beulich <jbeulich@novell.com>
>> Signed-off-by: Alessio Igor Bogani <abogani@kernel.org>
>> ---
>>  scripts/mod/modpost.c |   29 ++++++++++++++++++++++++++++-
>>  1 files changed, 28 insertions(+), 1 deletions(-)
> 
> < snip >
> 
> -Frank Rowand
> .
> 

Re: [PATCH] modpost: Fix modpost's license checking V3

[Frank Rowand]

On 03/23/12 19:04, Frank Rowand wrote:
> On 07/13/11 23:51, Alessio Igor Bogani wrote:
>> The commit f02e8a6 sorts symbols placing each of them in its own elf section.
>> The sorting and merging into the canonical sections are done by the linker.
>> Unfortunately modpost to generate Module.symvers file parses vmlinux
> 
> Yet another unfortunately: modpost parses vmlinux.o instead of vmlinux (vmlinux
> does not yet exist at this point of the build).  vmlinux.o also does not have
> the many sections sorted and merged into the canonical sections.  As a result,
> the Module.symvers created my modpost incorrectly reports the license of all

                             ^^^ s/my/by/

> exports as "(unknown)".
> 
> Can you fix this also please?
> 
> 
>> (already linked) and all modules object files (which aren't linked yet).
>> These aren't sanitized by the linker yet. That breaks modpost that can't
>> detect license properly for modules. This patch makes modpost aware of
>> the new exported symbols structure.
>>
>> Thanks to Arnaud Lacombe <lacombar@gmail.com> and Anders Kaseorg
>> <andersk@ksplice.com> for providing useful suggestions about code.
>>
>> This work was supported by a hardware donation from the CE Linux Forum.
>>
>> Reported-by: Jan Beulich <jbeulich@novell.com>
>> Signed-off-by: Alessio Igor Bogani <abogani@kernel.org>
>> ---
>>  scripts/mod/modpost.c |   29 ++++++++++++++++++++++++++++-
>>  1 files changed, 28 insertions(+), 1 deletions(-)
> 
> < snip >
> 
> -Frank Rowand
> .
> 

Re: [PATCH] modpost: Fix modpost's license checking V3

[Frank Rowand]

On 03/23/12 19:04, Frank Rowand wrote:
> On 07/13/11 23:51, Alessio Igor Bogani wrote:
>> The commit f02e8a6 sorts symbols placing each of them in its own elf section.
>> The sorting and merging into the canonical sections are done by the linker.
>> Unfortunately modpost to generate Module.symvers file parses vmlinux
> 
> Yet another unfortunately: modpost parses vmlinux.o instead of vmlinux (vmlinux
> does not yet exist at this point of the build).  vmlinux.o also does not have
> the many sections sorted and merged into the canonical sections.  As a result,
> the Module.symvers created my modpost incorrectly reports the license of all
> exports as "(unknown)".
> 
> Can you fix this also please?

The attached patch modifies your patch to also use export_from_secname()
for vmlinux and vmlinux.o.

This is a rather blind shot at fixing the problem, so please review
carefully.  After applying the patch, Module.symvers reports the license
correctly for exports from vmlinux.o.

> 
> 
>> (already linked) and all modules object files (which aren't linked yet).
>> These aren't sanitized by the linker yet. That breaks modpost that can't
>> detect license properly for modules. This patch makes modpost aware of
>> the new exported symbols structure.
>>
>> Thanks to Arnaud Lacombe <lacombar@gmail.com> and Anders Kaseorg
>> <andersk@ksplice.com> for providing useful suggestions about code.
>>
>> This work was supported by a hardware donation from the CE Linux Forum.
>>
>> Reported-by: Jan Beulich <jbeulich@novell.com>
>> Signed-off-by: Alessio Igor Bogani <abogani@kernel.org>
>> ---
>>  scripts/mod/modpost.c |   29 ++++++++++++++++++++++++++++-
>>  1 files changed, 28 insertions(+), 1 deletions(-)
> 
> < snip >
> 
> -Frank Rowand

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
 scripts/mod/modpost.c |    2 	1 +	1 -	0 !
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: b/scripts/mod/modpost.c
===================================================================
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -589,7 +589,7 @@ static void handle_modversions(struct mo
 	unsigned int crc;
 	enum export export;
 
-	if (!is_vmlinux(mod->name) && strncmp(symname, "__ksymtab", 9) == 0)
+	if (strncmp(symname, "__ksymtab", 9) == 0)
 		export = export_from_secname(info, get_secindex(info, sym));
 	else
 		export = export_from_sec(info, get_secindex(info, sym));

Re: [PATCH] modpost: Fix modpost's license checking V3

[Frank Rowand]

On 03/23/12 19:04, Frank Rowand wrote:
> On 07/13/11 23:51, Alessio Igor Bogani wrote:
>> The commit f02e8a6 sorts symbols placing each of them in its own elf section.
>> The sorting and merging into the canonical sections are done by the linker.
>> Unfortunately modpost to generate Module.symvers file parses vmlinux
> 
> Yet another unfortunately: modpost parses vmlinux.o instead of vmlinux (vmlinux
> does not yet exist at this point of the build).  vmlinux.o also does not have
> the many sections sorted and merged into the canonical sections.  As a result,
> the Module.symvers created my modpost incorrectly reports the license of all
> exports as "(unknown)".
> 
> Can you fix this also please?

The attached patch modifies your patch to also use export_from_secname()
for vmlinux and vmlinux.o.

This is a rather blind shot at fixing the problem, so please review
carefully.  After applying the patch, Module.symvers reports the license
correctly for exports from vmlinux.o.

> 
> 
>> (already linked) and all modules object files (which aren't linked yet).
>> These aren't sanitized by the linker yet. That breaks modpost that can't
>> detect license properly for modules. This patch makes modpost aware of
>> the new exported symbols structure.
>>
>> Thanks to Arnaud Lacombe <lacombar@gmail.com> and Anders Kaseorg
>> <andersk@ksplice.com> for providing useful suggestions about code.
>>
>> This work was supported by a hardware donation from the CE Linux Forum.
>>
>> Reported-by: Jan Beulich <jbeulich@novell.com>
>> Signed-off-by: Alessio Igor Bogani <abogani@kernel.org>
>> ---
>>  scripts/mod/modpost.c |   29 ++++++++++++++++++++++++++++-
>>  1 files changed, 28 insertions(+), 1 deletions(-)
> 
> < snip >
> 
> -Frank Rowand

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
 scripts/mod/modpost.c |    2 	1 +	1 -	0 !
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: b/scripts/mod/modpost.c
===================================================================
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -589,7 +589,7 @@ static void handle_modversions(struct mo
 	unsigned int crc;
 	enum export export;
 
-	if (!is_vmlinux(mod->name) && strncmp(symname, "__ksymtab", 9) == 0)
+	if (strncmp(symname, "__ksymtab", 9) == 0)
 		export = export_from_secname(info, get_secindex(info, sym));
 	else
 		export = export_from_sec(info, get_secindex(info, sym));

Re: [PATCH] modpost: Fix modpost's license checking V3

[Alessio Igor Bogani]

Dear Mr. Rowand,

Il 27 marzo 2012 03:58, Frank Rowand <frank.rowand@am.sony.com> ha scritto:
> On 03/23/12 19:04, Frank Rowand wrote:
>> On 07/13/11 23:51, Alessio Igor Bogani wrote:
>>> The commit f02e8a6 sorts symbols placing each of them in its own elf section.
>>> The sorting and merging into the canonical sections are done by the linker.
>>> Unfortunately modpost to generate Module.symvers file parses vmlinux
>>
>> Yet another unfortunately: modpost parses vmlinux.o instead of vmlinux (vmlinux
>> does not yet exist at this point of the build).  vmlinux.o also does not have
>> the many sections sorted and merged into the canonical sections.  As a result,
>> the Module.symvers created my modpost incorrectly reports the license of all
>> exports as "(unknown)".
>>
>> Can you fix this also please?
>
> The attached patch modifies your patch to also use export_from_secname()
> for vmlinux and vmlinux.o.
>
> This is a rather blind shot at fixing the problem, so please review
> carefully.  After applying the patch, Module.symvers reports the license
> correctly for exports from vmlinux.o.

Could you show me how reproduce that problem? Indeed on my system
"unknown" is reported (erroneously) in the v3.0 series before the
commit 62a2635610dbc83c5e8d724e00941eee4d18c186 (and obviously after
my patchset which adds that bug).

Thanks!

Ciao,
Alessio

Re: [PATCH] modpost: Fix modpost's license checking V3

[Frank Rowand]

On 03/27/12 00:19, Alessio Igor Bogani wrote:
> Dear Mr. Rowand,
> 
> Il 27 marzo 2012 03:58, Frank Rowand <frank.rowand@am.sony.com> ha scritto:
>> On 03/23/12 19:04, Frank Rowand wrote:
>>> On 07/13/11 23:51, Alessio Igor Bogani wrote:
>>>> The commit f02e8a6 sorts symbols placing each of them in its own elf section.
>>>> The sorting and merging into the canonical sections are done by the linker.
>>>> Unfortunately modpost to generate Module.symvers file parses vmlinux
>>>
>>> Yet another unfortunately: modpost parses vmlinux.o instead of vmlinux (vmlinux
>>> does not yet exist at this point of the build).  vmlinux.o also does not have
>>> the many sections sorted and merged into the canonical sections.  As a result,
>>> the Module.symvers created my modpost incorrectly reports the license of all
>>> exports as "(unknown)".
>>>
>>> Can you fix this also please?
>>
>> The attached patch modifies your patch to also use export_from_secname()
>> for vmlinux and vmlinux.o.
>>
>> This is a rather blind shot at fixing the problem, so please review
>> carefully.  After applying the patch, Module.symvers reports the license
>> correctly for exports from vmlinux.o.

That version of my patch broke modpost for vmlinux.  I don't know if
anyone uses modpost against vmlinux instead of vmlinux.o, but the
attached patch works for both vmlinux and vmlinux.o

> 
> Could you show me how reproduce that problem? Indeed on my system
> "unknown" is reported (erroneously) in the v3.0 series before the
> commit 62a2635610dbc83c5e8d724e00941eee4d18c186 (and obviously after
> my patchset which adds that bug).

I'm not sure what you are asking here.

You mention "my patchset which adds that bug" (which from looking at
the source, appears to me to probably be commit
f02e8a6596b7dc9b2171f7ff5654039ef0950cdc).

The email that I originally replied to is your fix to modpost for modules,
and shows up as commit 62a2635610dbc83c5e8d724e00941eee4d18c186.

My attached fix is meant to extend 62a2635610dbc83c5e8d724e00941eee4d18c186
to also fix modpost for vmlinux.o.

> 
> Thanks!
> 
> Ciao,
> Alessio
> 

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
 scripts/mod/modpost.c |    7 	5 +	2 -	0 !
 scripts/mod/modpost.h |    1 	1 +	0 -	0 !
 2 files changed, 6 insertions(+), 2 deletions(-)

Index: b/scripts/mod/modpost.c
===================================================================
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -132,8 +132,10 @@ static struct module *new_module(char *m
 	/* strip trailing .o */
 	s = strrchr(p, '.');
 	if (s != NULL)
-		if (strcmp(s, ".o") == 0)
+		if (strcmp(s, ".o") == 0) {
 			*s = '\0';
+			mod->is_dot_o = 1;
+		}
 
 	/* add to list */
 	mod->name = p;
@@ -589,7 +591,8 @@ static void handle_modversions(struct mo
 	unsigned int crc;
 	enum export export;
 
-	if (!is_vmlinux(mod->name) && strncmp(symname, "__ksymtab", 9) == 0)
+	if ((!is_vmlinux(mod->name) || mod->is_dot_o)
+	    && strncmp(symname, "__ksymtab", 9) == 0)
 		export = export_from_secname(info, get_secindex(info, sym));
 	else
 		export = export_from_sec(info, get_secindex(info, sym));
Index: b/scripts/mod/modpost.h
===================================================================
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -115,6 +115,7 @@ struct module {
 	char **markers;
 	size_t nmarkers;
 	char	     srcversion[25];
+	int is_dot_o;
 };
 
 struct elf_info {

Re: [PATCH] modpost: Fix modpost's license checking V3

[Alessio Igor Bogani]

Dear Mr. Rowand,

Frank Rowand <frank.rowand@am.sony.com> wrote:
[...]
>> Could you show me how reproduce that problem?
>
> I'm not sure what you are asking here.

I meant the kernel version, the kernel configuration and so the
Module.symvers file you obtain.
Thanks!

Ciao,
Alessio

Re: [PATCH] modpost: Fix modpost's license checking V3

[Rusty Russell]

On Mon, 26 Mar 2012 18:58:24 -0700, Frank Rowand <frank.rowand@am.sony.com> wrote:
> The attached patch modifies your patch to also use export_from_secname()
> for vmlinux and vmlinux.o.

OK, today is my last day online for six weeks, so Alessio, please come
up with a fix and push straight to Linus once consensus is reached.

If that's impossible, I'll review when I get back.

Thanks,
Rusty.
-- 
  How could I marry someone with more hair than me?  http://baldalex.org

Re: [PATCH] modpost: Fix modpost's license checking V3

[Rusty Russell]

On Mon, 26 Mar 2012 18:58:24 -0700, Frank Rowand <frank.rowand@am.sony.com> wrote:
> The attached patch modifies your patch to also use export_from_secname()
> for vmlinux and vmlinux.o.

OK, today is my last day online for six weeks, so Alessio, please come
up with a fix and push straight to Linus once consensus is reached.

If that's impossible, I'll review when I get back.

Thanks,
Rusty.
-- 
  How could I marry someone with more hair than me?  http://baldalex.org

Re: [PATCH] modpost: Fix modpost's license checking V3

[Frank Rowand]

On 03/28/12 21:37, Rusty Russell wrote:
> On Mon, 26 Mar 2012 18:58:24 -0700, Frank Rowand <frank.rowand@am.sony.com> wrote:
>> The attached patch modifies your patch to also use export_from_secname()
>> for vmlinux and vmlinux.o.
> 
> OK, today is my last day online for six weeks, so Alessio, please come
> up with a fix and push straight to Linus once consensus is reached.
> 
> If that's impossible, I'll review when I get back.
> 
> Thanks,
> Rusty.

Linus,

Alessio asked me to send this directly to you.

-Frank


The commit f02e8a6 sorts symbols placing each of them in its own elf section.
The sorting and merging into the canonical sections are done by the linker.
Unfortunately modpost to generate Module.symvers file parses vmlinux.o (which
is not linked yet) and all modules object files (which aren't linked yet).
These aren't sanitized by the linker yet. That breaks modpost that can't
detect license properly for modules.

This patch makes modpost aware of the new exported symbols structure.

The above is a slightly corrected version of the explanation of the problem,
copied from commit 62a26356.  That commit fixed the problem for module object
files, but not for vmlinux.o.  This patch fixes modpost for vmlinux.o.

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
Signed-off-by: Alessio Igor Bogani <abogani@kernel.org>
---
 scripts/mod/modpost.c |    7 	5 +	2 -	0 !
 scripts/mod/modpost.h |    1 	1 +	0 -	0 !
 2 files changed, 6 insertions(+), 2 deletions(-)

Index: b/scripts/mod/modpost.c
===================================================================
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -132,8 +132,10 @@ static struct module *new_module(char *m
 	/* strip trailing .o */
 	s = strrchr(p, '.');
 	if (s != NULL)
-		if (strcmp(s, ".o") == 0)
+		if (strcmp(s, ".o") == 0) {
 			*s = '\0';
+			mod->is_dot_o = 1;
+		}
 
 	/* add to list */
 	mod->name = p;
@@ -587,7 +589,8 @@ static void handle_modversions(struct mo
 	unsigned int crc;
 	enum export export;
 
-	if (!is_vmlinux(mod->name) && strncmp(symname, "__ksymtab", 9) == 0)
+	if ((!is_vmlinux(mod->name) || mod->is_dot_o) &&
+	    strncmp(symname, "__ksymtab", 9) == 0)
 		export = export_from_secname(info, get_secindex(info, sym));
 	else
 		export = export_from_sec(info, get_secindex(info, sym));
Index: b/scripts/mod/modpost.h
===================================================================
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -113,6 +113,7 @@ struct module {
 	int has_cleanup;
 	struct buffer dev_table_buf;
 	char	     srcversion[25];
+	int is_dot_o;
 };
 
 struct elf_info {

Re: [PATCH] modpost: Fix modpost's license checking V3

[Frank Rowand]

On 03/28/12 21:37, Rusty Russell wrote:
> On Mon, 26 Mar 2012 18:58:24 -0700, Frank Rowand <frank.rowand@am.sony.com> wrote:
>> The attached patch modifies your patch to also use export_from_secname()
>> for vmlinux and vmlinux.o.
> 
> OK, today is my last day online for six weeks, so Alessio, please come
> up with a fix and push straight to Linus once consensus is reached.
> 
> If that's impossible, I'll review when I get back.
> 
> Thanks,
> Rusty.

Linus,

Alessio asked me to send this directly to you.

-Frank


The commit f02e8a6 sorts symbols placing each of them in its own elf section.
The sorting and merging into the canonical sections are done by the linker.
Unfortunately modpost to generate Module.symvers file parses vmlinux.o (which
is not linked yet) and all modules object files (which aren't linked yet).
These aren't sanitized by the linker yet. That breaks modpost that can't
detect license properly for modules.

This patch makes modpost aware of the new exported symbols structure.

The above is a slightly corrected version of the explanation of the problem,
copied from commit 62a26356.  That commit fixed the problem for module object
files, but not for vmlinux.o.  This patch fixes modpost for vmlinux.o.

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
Signed-off-by: Alessio Igor Bogani <abogani@kernel.org>
---
 scripts/mod/modpost.c |    7 	5 +	2 -	0 !
 scripts/mod/modpost.h |    1 	1 +	0 -	0 !
 2 files changed, 6 insertions(+), 2 deletions(-)

Index: b/scripts/mod/modpost.c
===================================================================
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -132,8 +132,10 @@ static struct module *new_module(char *m
 	/* strip trailing .o */
 	s = strrchr(p, '.');
 	if (s != NULL)
-		if (strcmp(s, ".o") == 0)
+		if (strcmp(s, ".o") == 0) {
 			*s = '\0';
+			mod->is_dot_o = 1;
+		}
 
 	/* add to list */
 	mod->name = p;
@@ -587,7 +589,8 @@ static void handle_modversions(struct mo
 	unsigned int crc;
 	enum export export;
 
-	if (!is_vmlinux(mod->name) && strncmp(symname, "__ksymtab", 9) == 0)
+	if ((!is_vmlinux(mod->name) || mod->is_dot_o) &&
+	    strncmp(symname, "__ksymtab", 9) == 0)
 		export = export_from_secname(info, get_secindex(info, sym));
 	else
 		export = export_from_sec(info, get_secindex(info, sym));
Index: b/scripts/mod/modpost.h
===================================================================
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -113,6 +113,7 @@ struct module {
 	int has_cleanup;
 	struct buffer dev_table_buf;
 	char	     srcversion[25];
+	int is_dot_o;
 };
 
 struct elf_info {