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