Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1631671 > unrolled thread
| Started by | Kees Cook <keescook@chromium.org> |
|---|---|
| First post | 2017-04-26 21:10 +0200 |
| Last post | 2017-04-26 22:00 +0200 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH] x86/mm/kaslr: Add operand size suffix to 'mul' instruction Kees Cook <keescook@chromium.org> - 2017-04-26 21:10 +0200
Re: [PATCH] x86/mm/kaslr: Add operand size suffix to 'mul' instruction Matthias Kaehlcke <mka@chromium.org> - 2017-04-26 21:40 +0200
Re: [PATCH] x86/mm/kaslr: Add operand size suffix to 'mul' instruction Kees Cook <keescook@chromium.org> - 2017-04-26 22:00 +0200
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-04-26 21:10 +0200 |
| Subject | Re: [PATCH] x86/mm/kaslr: Add operand size suffix to 'mul' instruction |
| Message-ID | <tAAu5-6qw-7@gated-at.bofh.it> |
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
[toc] | [next] | [standalone]
| From | Matthias Kaehlcke <mka@chromium.org> |
|---|---|
| Date | 2017-04-26 21:40 +0200 |
| Subject | Re: [PATCH] x86/mm/kaslr: Add operand size suffix to 'mul' instruction |
| Message-ID | <tAAX7-6zY-5@gated-at.bofh.it> |
| In reply to | #1631671 |
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
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-04-26 22:00 +0200 |
| Message-ID | <tABgt-6Kl-7@gated-at.bofh.it> |
| In reply to | #1631686 |
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
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web