All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthias Kaehlcke <mka@chromium.org>
To: Kees Cook <keescook@chromium.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H . Peter Anvin" <hpa@zytor.com>,
	"x86@kernel.org" <x86@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Grant Grundler <grundler@chromium.org>,
	Greg Hackmann <ghackmann@google.com>,
	Michael Davidson <md@google.com>
Subject: Re: [PATCH] x86/mm/kaslr: Add operand size suffix to 'mul' instruction
Date: Wed, 26 Apr 2017 12:31:47 -0700	[thread overview]
Message-ID: <20170426193147.GQ128305@google.com> (raw)
In-Reply-To: <CAGXu5j+TH1-Jq=2Zh83KkJxdArJLtRctB=t_zVAk=f2pC-FSnA@mail.gmail.com>

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

  reply	other threads:[~2017-04-26 19:31 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2017-04-26 19:57     ` Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170426193147.GQ128305@google.com \
    --to=mka@chromium.org \
    --cc=ghackmann@google.com \
    --cc=grundler@chromium.org \
    --cc=hpa@zytor.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=md@google.com \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.