* Re: memcmp in modules
[not found] <CAHp75VdDwh0wgCWrVDcPps1C2qfdDZra15y9BQvzfWHzSMqbWA@mail.gmail.com>
@ 2013-04-12 15:08 ` Andy Shevchenko
2013-04-22 14:26 ` Tetsuo Handa
0 siblings, 1 reply; 18+ messages in thread
From: Andy Shevchenko @ 2013-04-12 15:08 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org
HI.
I have an issue with memcmp, but it seems I didn't get what is
happening there. Let's consider the details.
I have a module which has one call of memcmp and licensed as Dual BSD/GPL.
#include <linux/string.h>
...
if (memcmp(...))
...
MODULE_LICENSE("Dual BSD/GPL");
I compile it for ARCH=i386 together with kernel and try to load it.
modprobe test-string_helpers
[ 25.134433] test_string_helpers: no symbol version for memcmp
[ 25.140156] test_string_helpers: Unknown symbol memcmp (err -22)
Then I've changed string_32.h a bit (I think it's not a solution of the issue)
-#define memcmp __builtin_memcmp
+#define memcmp(a, b, n) __builtin_memcmp(a, b, n)
And in this case:
modprobe test_string_helpers
[ 15.391589] test_string_helpers: Running tests...
What did I miss?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: memcmp in modules
2013-04-12 15:08 ` memcmp in modules Andy Shevchenko
@ 2013-04-22 14:26 ` Tetsuo Handa
2013-04-23 12:40 ` [PATCH] x86_32: Fix module version table mismatch Tetsuo Handa
2013-04-23 23:15 ` memcmp in modules H. Peter Anvin
0 siblings, 2 replies; 18+ messages in thread
From: Tetsuo Handa @ 2013-04-22 14:26 UTC (permalink / raw)
To: andy.shevchenko, arjan, hpa; +Cc: linux-kernel
Andy Shevchenko wrote:
> What did I miss?
Well, as of linux-next-20130422, memcmp() is not correctly exported to modules.
Since linux-3.9-rc8 correctly exports memcmp(), this problem seems to be introduced
in linux-next tree. Also, this problem seems to involve CONFIG_MODVERSIONS=y.
[root@localhost linux-next]# modprobe ipv6
FATAL: Error inserting ipv6 (/lib/modules/3.9.0-rc8-next-20130422/kernel/net/ipv6/ipv6.ko): Invalid argument
[root@localhost linux-next]# dmesg
ipv6: no symbol version for memcmp
ipv6: Unknown symbol memcmp (err -22)
Since arch/x86/include/asm/string_64.h uses
int memcmp(const void *cs, const void *ct, size_t count);
while arch/x86/include/asm/string_32.h uses
#define memcmp __builtin_memcmp
changing to what you have tried
#define memcmp(a, b, n) __builtin_memcmp(a, b, n)
or changing to what x86_64 does
int memcmp(const void *cs, const void *ct, size_t count);
might solve this problem. But I don't know which one is correct solution...
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] x86_32: Fix module version table mismatch.
2013-04-22 14:26 ` Tetsuo Handa
@ 2013-04-23 12:40 ` Tetsuo Handa
2013-04-23 14:14 ` Andy Shevchenko
` (3 more replies)
2013-04-23 23:15 ` memcmp in modules H. Peter Anvin
1 sibling, 4 replies; 18+ messages in thread
From: Tetsuo Handa @ 2013-04-23 12:40 UTC (permalink / raw)
To: arjan, hpa, james.hogan, rusty; +Cc: linux-kernel, andy.shevchenko
Commit a4b6a77b "module: fix symbol versioning with symbol prefixes" broke
loading of net/ipv6/ipv6.ko built with CONFIG_MODVERSIONS=y for x86_32.
# modprobe ipv6
FATAL: Error inserting ipv6 (/lib/modules/3.9.0-rc8-next-20130422/kernel/net/ipv6/ipv6.ko): Invalid argument
# dmesg
ipv6: no symbol version for memcmp
ipv6: Unknown symbol memcmp (err -22)
The reason for breakage is that check_version() in kernel/module.c tries to
find symname == "memcmp" but versions[i].name == "__builtin_memcmp".
The reason for versions[i].name == "__builtin_memcmp" is that
memcmp() for x86_32 is defined as
#define memcmp __builtin_memcmp
in arch/x86/include/asm/string_32.h while memcmp() for x86_64 is defined as
int memcmp(const void *cs, const void *ct, size_t count);
in arch/x86/include/asm/string_64.h.
Since __builtin_memcmp is a gcc's built-in function which might emit a call to
memcmp, __builtin_memcmp should not be used for versions[i].name field.
In order to make sure that versions[i].name == "memcmp", make the definition of
memcmp() for x86_32 identical with that of x86_64.
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: James Hogan <james.hogan@imgtec.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>
---
arch/x86/include/asm/string_32.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/x86/include/asm/string_32.h b/arch/x86/include/asm/string_32.h
index 3d3e835..bb85b4e 100644
--- a/arch/x86/include/asm/string_32.h
+++ b/arch/x86/include/asm/string_32.h
@@ -199,7 +199,7 @@ static inline void *__memcpy3d(void *to, const void *from, size_t len)
#define __HAVE_ARCH_MEMMOVE
void *memmove(void *dest, const void *src, size_t n);
-#define memcmp __builtin_memcmp
+int memcmp(const void *cs, const void *ct, size_t count);
#define __HAVE_ARCH_MEMCHR
extern void *memchr(const void *cs, int c, size_t count);
--
1.7.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] x86_32: Fix module version table mismatch.
2013-04-23 12:40 ` [PATCH] x86_32: Fix module version table mismatch Tetsuo Handa
@ 2013-04-23 14:14 ` Andy Shevchenko
2013-04-23 15:17 ` H. Peter Anvin
` (2 subsequent siblings)
3 siblings, 0 replies; 18+ messages in thread
From: Andy Shevchenko @ 2013-04-23 14:14 UTC (permalink / raw)
To: Tetsuo Handa
Cc: arjan, hpa, james.hogan, Rusty Russell,
linux-kernel@vger.kernel.org
On Tue, Apr 23, 2013 at 3:40 PM, Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
> Commit a4b6a77b "module: fix symbol versioning with symbol prefixes" broke
> loading of net/ipv6/ipv6.ko
I would rephrase this to "any module uses memcmp, e.g. ipv6"
Otherwise: Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
...and
Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> built with CONFIG_MODVERSIONS=y for x86_32.
>
> # modprobe ipv6
> FATAL: Error inserting ipv6 (/lib/modules/3.9.0-rc8-next-20130422/kernel/net/ipv6/ipv6.ko): Invalid argument
> # dmesg
> ipv6: no symbol version for memcmp
> ipv6: Unknown symbol memcmp (err -22)
>
> The reason for breakage is that check_version() in kernel/module.c tries to
> find symname == "memcmp" but versions[i].name == "__builtin_memcmp".
>
> The reason for versions[i].name == "__builtin_memcmp" is that
> memcmp() for x86_32 is defined as
>
> #define memcmp __builtin_memcmp
>
> in arch/x86/include/asm/string_32.h while memcmp() for x86_64 is defined as
>
> int memcmp(const void *cs, const void *ct, size_t count);
>
> in arch/x86/include/asm/string_64.h.
>
> Since __builtin_memcmp is a gcc's built-in function which might emit a call to
> memcmp, __builtin_memcmp should not be used for versions[i].name field.
>
> In order to make sure that versions[i].name == "memcmp", make the definition of
> memcmp() for x86_32 identical with that of x86_64.
>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Cc: James Hogan <james.hogan@imgtec.com>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> ---
> arch/x86/include/asm/string_32.h | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/include/asm/string_32.h b/arch/x86/include/asm/string_32.h
> index 3d3e835..bb85b4e 100644
> --- a/arch/x86/include/asm/string_32.h
> +++ b/arch/x86/include/asm/string_32.h
> @@ -199,7 +199,7 @@ static inline void *__memcpy3d(void *to, const void *from, size_t len)
> #define __HAVE_ARCH_MEMMOVE
> void *memmove(void *dest, const void *src, size_t n);
>
> -#define memcmp __builtin_memcmp
> +int memcmp(const void *cs, const void *ct, size_t count);
>
> #define __HAVE_ARCH_MEMCHR
> extern void *memchr(const void *cs, int c, size_t count);
> --
> 1.7.1
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86_32: Fix module version table mismatch.
2013-04-23 12:40 ` [PATCH] x86_32: Fix module version table mismatch Tetsuo Handa
2013-04-23 14:14 ` Andy Shevchenko
@ 2013-04-23 15:17 ` H. Peter Anvin
2013-04-23 15:41 ` Andy Shevchenko
2013-04-23 22:56 ` H. Peter Anvin
2013-04-24 0:52 ` H. Peter Anvin
3 siblings, 1 reply; 18+ messages in thread
From: H. Peter Anvin @ 2013-04-23 15:17 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: arjan, james.hogan, rusty, linux-kernel, andy.shevchenko
On 04/23/2013 05:40 AM, Tetsuo Handa wrote:
>
> -#define memcmp __builtin_memcmp
> +int memcmp(const void *cs, const void *ct, size_t count);
>
Yuck. I really don't like this option unless it truly can't be
avoided... it might be a fix for 3.9/stable but a better way to do this
would be much more appreciated.
-hpa
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86_32: Fix module version table mismatch.
2013-04-23 15:17 ` H. Peter Anvin
@ 2013-04-23 15:41 ` Andy Shevchenko
2013-04-23 22:07 ` H. Peter Anvin
0 siblings, 1 reply; 18+ messages in thread
From: Andy Shevchenko @ 2013-04-23 15:41 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Tetsuo Handa, arjan, james.hogan, Rusty Russell,
linux-kernel@vger.kernel.org
On Tue, Apr 23, 2013 at 6:17 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 04/23/2013 05:40 AM, Tetsuo Handa wrote:
>>
>> -#define memcmp __builtin_memcmp
>> +int memcmp(const void *cs, const void *ct, size_t count);
>>
>
> Yuck. I really don't like this option unless it truly can't be
> avoided... it might be a fix for 3.9/stable but a better way to do this
> would be much more appreciated.
What about my proposal to supply parameters for __builtin_memcmp in
memcmp macro.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86_32: Fix module version table mismatch.
2013-04-23 15:41 ` Andy Shevchenko
@ 2013-04-23 22:07 ` H. Peter Anvin
0 siblings, 0 replies; 18+ messages in thread
From: H. Peter Anvin @ 2013-04-23 22:07 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Tetsuo Handa, arjan, james.hogan, Rusty Russell,
linux-kernel@vger.kernel.org
On 04/23/2013 08:41 AM, Andy Shevchenko wrote:
> On Tue, Apr 23, 2013 at 6:17 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>> On 04/23/2013 05:40 AM, Tetsuo Handa wrote:
>>>
>>> -#define memcmp __builtin_memcmp
>>> +int memcmp(const void *cs, const void *ct, size_t count);
>>>
>>
>> Yuck. I really don't like this option unless it truly can't be
>> avoided... it might be a fix for 3.9/stable but a better way to do this
>> would be much more appreciated.
>
> What about my proposal to supply parameters for __builtin_memcmp in
> memcmp macro.
>
I don't see it anywhere.
-hpa
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86_32: Fix module version table mismatch.
2013-04-23 12:40 ` [PATCH] x86_32: Fix module version table mismatch Tetsuo Handa
2013-04-23 14:14 ` Andy Shevchenko
2013-04-23 15:17 ` H. Peter Anvin
@ 2013-04-23 22:56 ` H. Peter Anvin
2013-04-24 0:52 ` H. Peter Anvin
3 siblings, 0 replies; 18+ messages in thread
From: H. Peter Anvin @ 2013-04-23 22:56 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: arjan, james.hogan, rusty, linux-kernel, andy.shevchenko
On 04/23/2013 05:40 AM, Tetsuo Handa wrote:
> Commit a4b6a77b "module: fix symbol versioning with symbol prefixes" broke
> loading of net/ipv6/ipv6.ko built with CONFIG_MODVERSIONS=y for x86_32.
>
> # modprobe ipv6
> FATAL: Error inserting ipv6 (/lib/modules/3.9.0-rc8-next-20130422/kernel/net/ipv6/ipv6.ko): Invalid argument
> # dmesg
> ipv6: no symbol version for memcmp
> ipv6: Unknown symbol memcmp (err -22)
>
> The reason for breakage is that check_version() in kernel/module.c tries to
> find symname == "memcmp" but versions[i].name == "__builtin_memcmp".
>
> The reason for versions[i].name == "__builtin_memcmp" is that
> memcmp() for x86_32 is defined as
>
> #define memcmp __builtin_memcmp
>
> in arch/x86/include/asm/string_32.h while memcmp() for x86_64 is defined as
>
> int memcmp(const void *cs, const void *ct, size_t count);
>
> in arch/x86/include/asm/string_64.h.
>
> Since __builtin_memcmp is a gcc's built-in function which might emit a call to
> memcmp, __builtin_memcmp should not be used for versions[i].name field.
>
> In order to make sure that versions[i].name == "memcmp", make the definition of
> memcmp() for x86_32 identical with that of x86_64.
>
I'm still confused by all of this. VMLINUX_SYMBOL_STR() ought to be a
noop on x86, so how on Earth could a4b6a77b break anything?
Secondly, although memcmp is a macro, it is #undef'd before the
definition in lib/string.c, which is the one that is exported.
I'm wondering if the real culprit isn't b92021b0, but I'm looking into
it now.
-hpa
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: memcmp in modules
2013-04-22 14:26 ` Tetsuo Handa
2013-04-23 12:40 ` [PATCH] x86_32: Fix module version table mismatch Tetsuo Handa
@ 2013-04-23 23:15 ` H. Peter Anvin
1 sibling, 0 replies; 18+ messages in thread
From: H. Peter Anvin @ 2013-04-23 23:15 UTC (permalink / raw)
To: Tetsuo Handa, Rusty Russell; +Cc: andy.shevchenko, arjan, linux-kernel
On 04/22/2013 07:26 AM, Tetsuo Handa wrote:
> Andy Shevchenko wrote:
>> What did I miss?
>
> Well, as of linux-next-20130422, memcmp() is not correctly exported to modules.
> Since linux-3.9-rc8 correctly exports memcmp(), this problem seems to be introduced
> in linux-next tree. Also, this problem seems to involve CONFIG_MODVERSIONS=y.
>
> [root@localhost linux-next]# modprobe ipv6
> FATAL: Error inserting ipv6 (/lib/modules/3.9.0-rc8-next-20130422/kernel/net/ipv6/ipv6.ko): Invalid argument
> [root@localhost linux-next]# dmesg
> ipv6: no symbol version for memcmp
> ipv6: Unknown symbol memcmp (err -22)
>
> Since arch/x86/include/asm/string_64.h uses
>
> int memcmp(const void *cs, const void *ct, size_t count);
>
> while arch/x86/include/asm/string_32.h uses
>
> #define memcmp __builtin_memcmp
>
> changing to what you have tried
>
> #define memcmp(a, b, n) __builtin_memcmp(a, b, n)
>
> or changing to what x86_64 does
>
> int memcmp(const void *cs, const void *ct, size_t count);
>
> might solve this problem. But I don't know which one is correct solution...
>
Rusty, this seems like a problem with your changes to the prefix
handling. Somehow memcmp being a macro gets picked up by some, but not
all, of the module-metadata generation tools.
Changing memcmp to a macro with arguments (see above) seems to paper
over the problem for this one, but there seems to be something much more
sinister going on and I would really like a good explanation as I fear
this can crop up in other places.
-hpa
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86_32: Fix module version table mismatch.
2013-04-23 12:40 ` [PATCH] x86_32: Fix module version table mismatch Tetsuo Handa
` (2 preceding siblings ...)
2013-04-23 22:56 ` H. Peter Anvin
@ 2013-04-24 0:52 ` H. Peter Anvin
2013-04-24 1:00 ` H. Peter Anvin
3 siblings, 1 reply; 18+ messages in thread
From: H. Peter Anvin @ 2013-04-24 0:52 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: arjan, james.hogan, rusty, linux-kernel, andy.shevchenko
On 04/23/2013 05:40 AM, Tetsuo Handa wrote:
> Commit a4b6a77b "module: fix symbol versioning with symbol prefixes" broke
> loading of net/ipv6/ipv6.ko built with CONFIG_MODVERSIONS=y for x86_32.
This really does seem to be the offending commit, although I'm still
confused how the heck that is possible.
-hpa
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86_32: Fix module version table mismatch.
2013-04-24 0:52 ` H. Peter Anvin
@ 2013-04-24 1:00 ` H. Peter Anvin
2013-04-24 10:13 ` James Hogan
0 siblings, 1 reply; 18+ messages in thread
From: H. Peter Anvin @ 2013-04-24 1:00 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: arjan, james.hogan, rusty, linux-kernel, andy.shevchenko
On 04/23/2013 05:52 PM, H. Peter Anvin wrote:
> On 04/23/2013 05:40 AM, Tetsuo Handa wrote:
>> Commit a4b6a77b "module: fix symbol versioning with symbol prefixes" broke
>> loading of net/ipv6/ipv6.ko built with CONFIG_MODVERSIONS=y for x86_32.
>
> This really does seem to be the offending commit, although I'm still
> confused how the heck that is possible.
>
OK, now I grok.
The bug is the use of VMLINUX_SYMBOL_STR(%s) which expands at the time
the output of modpost is compiled. However, VMLINUX_SYMBOL_STR() unlike
__VMLINUX_SYMBOL_STR() does macro expansion on its argument, which is
actively wrong here.
I think the choice is either to change this to __VMLINUX_SYMBOL_STR() or
re-introduce CONFIG_SYMBOL_PREFIX (or its equivalent) so that modprobe
can emit it at compile time (assuming there even should *be* a prefix on
the symbol here, i.e. that the compiler won't add it.)
Either way -- James, Rusty, this is in your court.
-hpa
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86_32: Fix module version table mismatch.
2013-04-24 1:00 ` H. Peter Anvin
@ 2013-04-24 10:13 ` James Hogan
2013-04-24 11:24 ` Andy Shevchenko
` (3 more replies)
0 siblings, 4 replies; 18+ messages in thread
From: James Hogan @ 2013-04-24 10:13 UTC (permalink / raw)
To: H. Peter Anvin, rusty; +Cc: Tetsuo Handa, arjan, linux-kernel, andy.shevchenko
On 24/04/13 02:00, H. Peter Anvin wrote:
> On 04/23/2013 05:52 PM, H. Peter Anvin wrote:
>> On 04/23/2013 05:40 AM, Tetsuo Handa wrote:
>>> Commit a4b6a77b "module: fix symbol versioning with symbol prefixes" broke
>>> loading of net/ipv6/ipv6.ko built with CONFIG_MODVERSIONS=y for x86_32.
>>
>> This really does seem to be the offending commit, although I'm still
>> confused how the heck that is possible.
>>
>
> OK, now I grok.
>
> The bug is the use of VMLINUX_SYMBOL_STR(%s) which expands at the time
> the output of modpost is compiled. However, VMLINUX_SYMBOL_STR() unlike
> __VMLINUX_SYMBOL_STR() does macro expansion on its argument, which is
> actively wrong here.
Yes, nasty bug there (sorry!)
> I think the choice is either to change this to __VMLINUX_SYMBOL_STR() or
> re-introduce CONFIG_SYMBOL_PREFIX (or its equivalent) so that modprobe
> can emit it at compile time
Using __VMLINUX_SYMBOL_STR looks like the correct solution to me.
> (assuming there even should *be* a prefix on
> the symbol here, i.e. that the compiler won't add it.)
[__]VMLINUX_SYMBOL_STR expands to a string (e.g. "_" "memcmp" or just
"memcmp") so the compiler won't touch it.
>
> Either way -- James, Rusty, this is in your court.
How does the patch below look? I presume this is preferred over
making VMLINUX_SYMBOL_STR non-argument-expanding?
Thanks
James
Subject: [PATCH 1/1] modpost: fix unwanted VMLINUX_SYMBOL_STR expansion
Commit a4b6a77b77ba4f526392612c2365797fab956014 ("module: fix symbol
versioning with symbol prefixes") broke the MODVERSIONS loading of any
module using memcmp (e.g. ipv6) on x86_32, as it's defined to
__builtin_memcmp which is expanded by VMLINUX_SYMBOL_STR. Use
__VMLINUX_SYMBOL_STR instead which doesn't expand the argument.
Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: H. Peter Anvin <hpa@zytor.com>
---
scripts/mod/modpost.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 1f90961..a4be8e1 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1957,7 +1957,7 @@ static int add_versions(struct buffer *b, struct module *mod)
s->name, mod->name);
continue;
}
- buf_printf(b, "\t{ %#8x, VMLINUX_SYMBOL_STR(%s) },\n",
+ buf_printf(b, "\t{ %#8x, __VMLINUX_SYMBOL_STR(%s) },\n",
s->crc, s->name);
}
--
1.8.1.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] x86_32: Fix module version table mismatch.
2013-04-24 10:13 ` James Hogan
@ 2013-04-24 11:24 ` Andy Shevchenko
2013-04-24 12:28 ` Tetsuo Handa
` (2 subsequent siblings)
3 siblings, 0 replies; 18+ messages in thread
From: Andy Shevchenko @ 2013-04-24 11:24 UTC (permalink / raw)
To: James Hogan
Cc: H. Peter Anvin, Rusty Russell, Tetsuo Handa, arjan,
linux-kernel@vger.kernel.org
On Wed, Apr 24, 2013 at 1:13 PM, James Hogan <james.hogan@imgtec.com> wrote:
> On 24/04/13 02:00, H. Peter Anvin wrote:
>> On 04/23/2013 05:52 PM, H. Peter Anvin wrote:
>> Either way -- James, Rusty, this is in your court.
>
> How does the patch below look? I presume this is preferred over
> making VMLINUX_SYMBOL_STR non-argument-expanding?
I will test it soon.
> Subject: [PATCH 1/1] modpost: fix unwanted VMLINUX_SYMBOL_STR expansion
>
> Commit a4b6a77b77ba4f526392612c2365797fab956014 ("module: fix symbol
> versioning with symbol prefixes") broke the MODVERSIONS loading of any
> module using memcmp (e.g. ipv6) on x86_32, as it's defined to
> __builtin_memcmp which is expanded by VMLINUX_SYMBOL_STR. Use
> __VMLINUX_SYMBOL_STR instead which doesn't expand the argument.
>
> Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
To be strongly correct Tetsuo and me reported about it and Tetsuo did
first analysis.
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86_32: Fix module version table mismatch.
2013-04-24 10:13 ` James Hogan
2013-04-24 11:24 ` Andy Shevchenko
@ 2013-04-24 12:28 ` Tetsuo Handa
2013-04-24 13:49 ` Andy Shevchenko
2013-04-24 17:48 ` H. Peter Anvin
3 siblings, 0 replies; 18+ messages in thread
From: Tetsuo Handa @ 2013-04-24 12:28 UTC (permalink / raw)
To: james.hogan, hpa, rusty; +Cc: arjan, linux-kernel, andy.shevchenko
James Hogan wrote:
> How does the patch below look? I presume this is preferred over
> making VMLINUX_SYMBOL_STR non-argument-expanding?
>
> Thanks
> James
>
> Subject: [PATCH 1/1] modpost: fix unwanted VMLINUX_SYMBOL_STR expansion
>
> Commit a4b6a77b77ba4f526392612c2365797fab956014 ("module: fix symbol
> versioning with symbol prefixes") broke the MODVERSIONS loading of any
> module using memcmp (e.g. ipv6) on x86_32, as it's defined to
> __builtin_memcmp which is expanded by VMLINUX_SYMBOL_STR. Use
> __VMLINUX_SYMBOL_STR instead which doesn't expand the argument.
>
The patch solves this problem. Thank you.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86_32: Fix module version table mismatch.
2013-04-24 10:13 ` James Hogan
2013-04-24 11:24 ` Andy Shevchenko
2013-04-24 12:28 ` Tetsuo Handa
@ 2013-04-24 13:49 ` Andy Shevchenko
2013-04-24 17:48 ` H. Peter Anvin
3 siblings, 0 replies; 18+ messages in thread
From: Andy Shevchenko @ 2013-04-24 13:49 UTC (permalink / raw)
To: James Hogan
Cc: H. Peter Anvin, Rusty Russell, Tetsuo Handa, Arjan van de Ven,
linux-kernel@vger.kernel.org
It works
Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
On Wed, Apr 24, 2013 at 1:13 PM, James Hogan <james.hogan@imgtec.com> wrote:
> On 24/04/13 02:00, H. Peter Anvin wrote:
>> On 04/23/2013 05:52 PM, H. Peter Anvin wrote:
>>> On 04/23/2013 05:40 AM, Tetsuo Handa wrote:
>>>> Commit a4b6a77b "module: fix symbol versioning with symbol prefixes" broke
>>>> loading of net/ipv6/ipv6.ko built with CONFIG_MODVERSIONS=y for x86_32.
>>>
>>> This really does seem to be the offending commit, although I'm still
>>> confused how the heck that is possible.
>>>
>>
>> OK, now I grok.
>>
>> The bug is the use of VMLINUX_SYMBOL_STR(%s) which expands at the time
>> the output of modpost is compiled. However, VMLINUX_SYMBOL_STR() unlike
>> __VMLINUX_SYMBOL_STR() does macro expansion on its argument, which is
>> actively wrong here.
>
> Yes, nasty bug there (sorry!)
>
>> I think the choice is either to change this to __VMLINUX_SYMBOL_STR() or
>> re-introduce CONFIG_SYMBOL_PREFIX (or its equivalent) so that modprobe
>> can emit it at compile time
>
> Using __VMLINUX_SYMBOL_STR looks like the correct solution to me.
>
>> (assuming there even should *be* a prefix on
>> the symbol here, i.e. that the compiler won't add it.)
>
> [__]VMLINUX_SYMBOL_STR expands to a string (e.g. "_" "memcmp" or just
> "memcmp") so the compiler won't touch it.
>
>>
>> Either way -- James, Rusty, this is in your court.
>
> How does the patch below look? I presume this is preferred over
> making VMLINUX_SYMBOL_STR non-argument-expanding?
>
> Thanks
> James
>
> Subject: [PATCH 1/1] modpost: fix unwanted VMLINUX_SYMBOL_STR expansion
>
> Commit a4b6a77b77ba4f526392612c2365797fab956014 ("module: fix symbol
> versioning with symbol prefixes") broke the MODVERSIONS loading of any
> module using memcmp (e.g. ipv6) on x86_32, as it's defined to
> __builtin_memcmp which is expanded by VMLINUX_SYMBOL_STR. Use
> __VMLINUX_SYMBOL_STR instead which doesn't expand the argument.
>
> Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: H. Peter Anvin <hpa@zytor.com>
> ---
> scripts/mod/modpost.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 1f90961..a4be8e1 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1957,7 +1957,7 @@ static int add_versions(struct buffer *b, struct module *mod)
> s->name, mod->name);
> continue;
> }
> - buf_printf(b, "\t{ %#8x, VMLINUX_SYMBOL_STR(%s) },\n",
> + buf_printf(b, "\t{ %#8x, __VMLINUX_SYMBOL_STR(%s) },\n",
> s->crc, s->name);
> }
>
> --
> 1.8.1.2
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] x86_32: Fix module version table mismatch.
2013-04-24 10:13 ` James Hogan
` (2 preceding siblings ...)
2013-04-24 13:49 ` Andy Shevchenko
@ 2013-04-24 17:48 ` H. Peter Anvin
2013-04-25 10:42 ` [PATCH v2] modpost: fix unwanted VMLINUX_SYMBOL_STR expansion James Hogan
3 siblings, 1 reply; 18+ messages in thread
From: H. Peter Anvin @ 2013-04-24 17:48 UTC (permalink / raw)
To: James Hogan; +Cc: rusty, Tetsuo Handa, arjan, linux-kernel, andy.shevchenko
OK, I assume Rusty will pick this up in his tree before pushing it to Linus.
-hpa
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2] modpost: fix unwanted VMLINUX_SYMBOL_STR expansion
2013-04-24 17:48 ` H. Peter Anvin
@ 2013-04-25 10:42 ` James Hogan
2013-04-29 2:10 ` Rusty Russell
0 siblings, 1 reply; 18+ messages in thread
From: James Hogan @ 2013-04-25 10:42 UTC (permalink / raw)
To: rusty; +Cc: H. Peter Anvin, Tetsuo Handa, arjan, linux-kernel,
andy.shevchenko
Commit a4b6a77b77ba4f526392612c2365797fab956014 ("module: fix symbol
versioning with symbol prefixes") broke the MODVERSIONS loading of any
module using memcmp (e.g. ipv6) on x86_32, as it's defined to
__builtin_memcmp which is expanded by VMLINUX_SYMBOL_STR. Use
__VMLINUX_SYMBOL_STR instead which doesn't expand the argument.
Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: H. Peter Anvin <hpa@zytor.com>
Tested-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
I've corrected commit message tags for Rusty's convenience.
Updated Reported-by to include Andy
Taken the liberty of adding Tetsuo's Tested-by
Cheers
James
scripts/mod/modpost.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 1f90961..a4be8e1 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1957,7 +1957,7 @@ static int add_versions(struct buffer *b, struct module *mod)
s->name, mod->name);
continue;
}
- buf_printf(b, "\t{ %#8x, VMLINUX_SYMBOL_STR(%s) },\n",
+ buf_printf(b, "\t{ %#8x, __VMLINUX_SYMBOL_STR(%s) },\n",
s->crc, s->name);
}
--
1.8.1.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2] modpost: fix unwanted VMLINUX_SYMBOL_STR expansion
2013-04-25 10:42 ` [PATCH v2] modpost: fix unwanted VMLINUX_SYMBOL_STR expansion James Hogan
@ 2013-04-29 2:10 ` Rusty Russell
0 siblings, 0 replies; 18+ messages in thread
From: Rusty Russell @ 2013-04-29 2:10 UTC (permalink / raw)
To: James Hogan
Cc: H. Peter Anvin, Tetsuo Handa, arjan, linux-kernel,
andy.shevchenko
James Hogan <james.hogan@imgtec.com> writes:
> Commit a4b6a77b77ba4f526392612c2365797fab956014 ("module: fix symbol
> versioning with symbol prefixes") broke the MODVERSIONS loading of any
> module using memcmp (e.g. ipv6) on x86_32, as it's defined to
> __builtin_memcmp which is expanded by VMLINUX_SYMBOL_STR. Use
> __VMLINUX_SYMBOL_STR instead which doesn't expand the argument.
>
> Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Tested-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>
> I've corrected commit message tags for Rusty's convenience.
> Updated Reported-by to include Andy
> Taken the liberty of adding Tetsuo's Tested-by
Applied.
Thanks especially for chasing this: I was away for a few days last week.
Cheers,
Rusty.
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2013-04-29 3:18 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CAHp75VdDwh0wgCWrVDcPps1C2qfdDZra15y9BQvzfWHzSMqbWA@mail.gmail.com>
2013-04-12 15:08 ` memcmp in modules Andy Shevchenko
2013-04-22 14:26 ` Tetsuo Handa
2013-04-23 12:40 ` [PATCH] x86_32: Fix module version table mismatch Tetsuo Handa
2013-04-23 14:14 ` Andy Shevchenko
2013-04-23 15:17 ` H. Peter Anvin
2013-04-23 15:41 ` Andy Shevchenko
2013-04-23 22:07 ` H. Peter Anvin
2013-04-23 22:56 ` H. Peter Anvin
2013-04-24 0:52 ` H. Peter Anvin
2013-04-24 1:00 ` H. Peter Anvin
2013-04-24 10:13 ` James Hogan
2013-04-24 11:24 ` Andy Shevchenko
2013-04-24 12:28 ` Tetsuo Handa
2013-04-24 13:49 ` Andy Shevchenko
2013-04-24 17:48 ` H. Peter Anvin
2013-04-25 10:42 ` [PATCH v2] modpost: fix unwanted VMLINUX_SYMBOL_STR expansion James Hogan
2013-04-29 2:10 ` Rusty Russell
2013-04-23 23:15 ` memcmp in modules H. Peter Anvin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.