* 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: [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
* 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
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.