* [PATCH] x86/mm/kaslr: Add operand size suffix to 'mul' instruction @ 2017-04-26 19:01 Matthias Kaehlcke 2017-04-26 19:09 ` Kees Cook 0 siblings, 1 reply; 4+ messages in thread From: Matthias Kaehlcke @ 2017-04-26 19:01 UTC (permalink / raw) To: Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Kees Cook Cc: x86, linux-kernel, Grant Grundler, Greg Hackmann, Michael Davidson, Matthias Kaehlcke In difference to gas clang doesn't seem to infer the size from the operands. Adding the suffix fixes the following error when building with clang: CC arch/x86/lib/kaslr.o /tmp/kaslr-dfe1ad.s: Assembler messages: /tmp/kaslr-dfe1ad.s:182: Error: no instruction mnemonic suffix given and no register operands; can't size instruction Signed-off-by: Matthias Kaehlcke <mka@chromium.org> --- arch/x86/lib/kaslr.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/arch/x86/lib/kaslr.c b/arch/x86/lib/kaslr.c index 121f59c6ee54..947d4aa92ff7 100644 --- a/arch/x86/lib/kaslr.c +++ b/arch/x86/lib/kaslr.c @@ -79,7 +79,11 @@ unsigned long kaslr_get_random_long(const char *purpose) } /* Circular multiply for better bit diffusion */ - asm("mul %3" +#ifdef CONFIG_X86_64 + asm("mulq %3" +#else + asm("mull %3" +#endif : "=a" (random), "=d" (raw) : "a" (random), "rm" (mix_const)); random += raw; -- 2.13.0.rc0.306.g87b477812d-goog ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] x86/mm/kaslr: Add operand size suffix to 'mul' instruction 2017-04-26 19:01 [PATCH] x86/mm/kaslr: Add operand size suffix to 'mul' instruction Matthias Kaehlcke @ 2017-04-26 19:09 ` Kees Cook 2017-04-26 19:31 ` Matthias Kaehlcke 0 siblings, 1 reply; 4+ messages in thread From: Kees Cook @ 2017-04-26 19:09 UTC (permalink / raw) To: Matthias Kaehlcke Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin, x86@kernel.org, LKML, Grant Grundler, Greg Hackmann, Michael Davidson On Wed, Apr 26, 2017 at 12:01 PM, Matthias Kaehlcke <mka@chromium.org> wrote: > In difference to gas clang doesn't seem to infer the size from the > operands. Adding the suffix fixes the following error when building > with clang: > > CC arch/x86/lib/kaslr.o > /tmp/kaslr-dfe1ad.s: Assembler messages: > /tmp/kaslr-dfe1ad.s:182: Error: no instruction mnemonic suffix given and > no register operands; can't size instruction > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org> > --- > arch/x86/lib/kaslr.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/lib/kaslr.c b/arch/x86/lib/kaslr.c > index 121f59c6ee54..947d4aa92ff7 100644 > --- a/arch/x86/lib/kaslr.c > +++ b/arch/x86/lib/kaslr.c > @@ -79,7 +79,11 @@ unsigned long kaslr_get_random_long(const char *purpose) > } > > /* Circular multiply for better bit diffusion */ > - asm("mul %3" > +#ifdef CONFIG_X86_64 > + asm("mulq %3" > +#else > + asm("mull %3" > +#endif > : "=a" (random), "=d" (raw) > : "a" (random), "rm" (mix_const)); > random += raw; > -- > 2.13.0.rc0.306.g87b477812d-goog > Since there may be more of these and we try to avoid #ifdef in .c files, I think we should create an isns helper macro and use it here instead. Like: #ifdef CONFIG_X86_64 # define ASM_MUL "mulq" #else # define ASM_MUL "mull" #endif ... asm(ASM_MUL " %3" ... -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] x86/mm/kaslr: Add operand size suffix to 'mul' instruction 2017-04-26 19:09 ` Kees Cook @ 2017-04-26 19:31 ` Matthias Kaehlcke 2017-04-26 19:57 ` Kees Cook 0 siblings, 1 reply; 4+ messages in thread From: Matthias Kaehlcke @ 2017-04-26 19:31 UTC (permalink / raw) To: Kees Cook Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin, x86@kernel.org, LKML, Grant Grundler, Greg Hackmann, Michael Davidson Hi Kees, El Wed, Apr 26, 2017 at 12:09:13PM -0700 Kees Cook ha dit: > On Wed, Apr 26, 2017 at 12:01 PM, Matthias Kaehlcke <mka@chromium.org> wrote: > > In difference to gas clang doesn't seem to infer the size from the > > operands. Adding the suffix fixes the following error when building > > with clang: > > > > CC arch/x86/lib/kaslr.o > > /tmp/kaslr-dfe1ad.s: Assembler messages: > > /tmp/kaslr-dfe1ad.s:182: Error: no instruction mnemonic suffix given and > > no register operands; can't size instruction > > > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org> > > --- > > arch/x86/lib/kaslr.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/arch/x86/lib/kaslr.c b/arch/x86/lib/kaslr.c > > index 121f59c6ee54..947d4aa92ff7 100644 > > --- a/arch/x86/lib/kaslr.c > > +++ b/arch/x86/lib/kaslr.c > > @@ -79,7 +79,11 @@ unsigned long kaslr_get_random_long(const char *purpose) > > } > > > > /* Circular multiply for better bit diffusion */ > > - asm("mul %3" > > +#ifdef CONFIG_X86_64 > > + asm("mulq %3" > > +#else > > + asm("mull %3" > > +#endif > > : "=a" (random), "=d" (raw) > > : "a" (random), "rm" (mix_const)); > > random += raw; > > > > Since there may be more of these and we try to avoid #ifdef in .c > files, I think we should create an isns helper macro and use it here > instead. Like: > > #ifdef CONFIG_X86_64 > # define ASM_MUL "mulq" > #else > # define ASM_MUL "mull" > #endif > > ... > > > asm(ASM_MUL " %3" ... Sounds very reasonable. Thanks for the suggestion, I'll rework the patch. Matthias ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] x86/mm/kaslr: Add operand size suffix to 'mul' instruction 2017-04-26 19:31 ` Matthias Kaehlcke @ 2017-04-26 19:57 ` Kees Cook 0 siblings, 0 replies; 4+ messages in thread From: Kees Cook @ 2017-04-26 19:57 UTC (permalink / raw) To: Matthias Kaehlcke Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin, x86@kernel.org, LKML, Grant Grundler, Greg Hackmann, Michael Davidson On Wed, Apr 26, 2017 at 12:31 PM, Matthias Kaehlcke <mka@chromium.org> wrote: > Hi Kees, > > El Wed, Apr 26, 2017 at 12:09:13PM -0700 Kees Cook ha dit: > >> On Wed, Apr 26, 2017 at 12:01 PM, Matthias Kaehlcke <mka@chromium.org> wrote: >> > In difference to gas clang doesn't seem to infer the size from the >> > operands. Adding the suffix fixes the following error when building >> > with clang: >> > >> > CC arch/x86/lib/kaslr.o >> > /tmp/kaslr-dfe1ad.s: Assembler messages: >> > /tmp/kaslr-dfe1ad.s:182: Error: no instruction mnemonic suffix given and >> > no register operands; can't size instruction >> > >> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org> >> > --- >> > arch/x86/lib/kaslr.c | 6 +++++- >> > 1 file changed, 5 insertions(+), 1 deletion(-) >> > >> > diff --git a/arch/x86/lib/kaslr.c b/arch/x86/lib/kaslr.c >> > index 121f59c6ee54..947d4aa92ff7 100644 >> > --- a/arch/x86/lib/kaslr.c >> > +++ b/arch/x86/lib/kaslr.c >> > @@ -79,7 +79,11 @@ unsigned long kaslr_get_random_long(const char *purpose) >> > } >> > >> > /* Circular multiply for better bit diffusion */ >> > - asm("mul %3" >> > +#ifdef CONFIG_X86_64 >> > + asm("mulq %3" >> > +#else >> > + asm("mull %3" >> > +#endif >> > : "=a" (random), "=d" (raw) >> > : "a" (random), "rm" (mix_const)); >> > random += raw; >> > >> >> Since there may be more of these and we try to avoid #ifdef in .c >> files, I think we should create an isns helper macro and use it here >> instead. Like: >> >> #ifdef CONFIG_X86_64 >> # define ASM_MUL "mulq" >> #else >> # define ASM_MUL "mull" >> #endif >> >> ... >> >> >> asm(ASM_MUL " %3" ... > > Sounds very reasonable. Thanks for the suggestion, I'll rework the patch. Oh, hah, this almost already exists. I went looking for where it might best live and I see this is already built up in arch/x86/include/asm/asm.h I think you just need to add: #define _ASM_MUL __ASM_SIZE(mul) to that header, and use _ASM_MUL in kaslr.c. -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-04-26 19:57 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-04-26 19:01 [PATCH] x86/mm/kaslr: Add operand size suffix to 'mul' instruction Matthias Kaehlcke 2017-04-26 19:09 ` Kees Cook 2017-04-26 19:31 ` Matthias Kaehlcke 2017-04-26 19:57 ` Kees Cook
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox