linux-kernel - Question about alternatives (Re: [PATCH 0/5] x86-64: Remove syscall instructions at fixed addresses)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <BANLkTinC=bL6NvRzU2hrFLgmbyFu6L-8zA@mail.gmail.com>
Date:	Tue, 31 May 2011 11:31:55 -0400
From:	Andrew Lutomirski <luto@....edu>
To:	Ingo Molnar <mingo@...e.hu>
Cc:	Thomas Gleixner <tglx@...utronix.de>, x86@...nel.org,
	linux-kernel@...r.kernel.org,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Arjan van de Ven <arjan@...radead.org>,
	Jan Beulich <JBeulich@...ell.com>
Subject: Question about alternatives (Re: [PATCH 0/5] x86-64: Remove syscall
 instructions at fixed addresses)

On Tue, May 31, 2011 at 9:17 AM, Andrew Lutomirski <luto@....edu> wrote:
> On Tue, May 31, 2011 at 9:11 AM, Ingo Molnar <mingo@...e.hu> wrote:
>>
>> * Andrew Lutomirski <luto@....edu> wrote:
>>
>>> > You could start with picking the more compatible alternative
>>> > instruction initially. I don't at all mind losing half a cycle of
>>> > performance in that case ... this code should be secure first.
>>>
>>> The more compatible one is mfence, which in some cases could (I
>>> think) be a lot more than half a cycle.
>>
>> I'd still suggest to do the mfence change now and remove the
>> alternatives patching for now - if it's more than half a cycle then
>> it sure will be implemented properly, right?
>
> I don't know.  I just cut 5 ns off the thing a couple weeks ago and no
> one beat me to it :)
>
> I'll take a look at how hard the patching will be.

I don't think it'll be that hard to get the alternatives code to work
on the vdso, but there's an annoyance: the alt_instr descriptor
contains the absolute addresses of the old and new code.  That means
that if we generate a shared object with alternatives, that shared
object will contain relocations, and I'd rather not teach the kernel
how to relocate the vdso image.

Would you be okay with changing the addresses to be relative, like
this: (sorry, whitespace damaged):

diff --git a/arch/x86/include/asm/alternative.h
b/arch/x86/include/asm/alternative.h
index bf535f9..de25be6 100644
--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -84,8 +84,8 @@ static inline int alternatives_text_reserved(void
*start, void *end)
       "661:\n\t" oldinstr "\n662:\n"                                   \
       ".section .altinstructions,\"a\"\n"                              \
       _ASM_ALIGN "\n"                                                  \
-      _ASM_PTR "661b\n"                                /* label
    */   \
-      _ASM_PTR "663f\n"                                /* new
instruction */   \
+      _ASM_PTR "661b - .\n"                    /* label           */   \
+      _ASM_PTR "663f - .\n"                    /* new instruction */   \
       "         .word " __stringify(feature) "\n"      /* feature bit
    */   \
       "         .byte 662b-661b\n"                     /* sourcelen
    */   \
       "         .byte 664f-663f\n"                     /*
replacementlen  */   \

This won't result in relocations.

(Obviously I'd have to change the asm version and the patching code to
match it.)

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/