* 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox