Messages in this thread | | | From | Kees Cook <> | Date | Wed, 26 Apr 2017 12:57:10 -0700 | Subject | Re: [PATCH] x86/mm/kaslr: Add operand size suffix to 'mul' instruction |
| |
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
| |