[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sun, 21 Jan 2018 11:25:03 -0800
From: Andy Lutomirski <luto@...capital.net>
To: Dan Williams <dan.j.williams@...el.com>
Cc: tglx@...utronix.de, linux-arch@...r.kernel.org,
kernel-hardening@...ts.openwall.com, gregkh@...uxfoundation.org,
x86@...nel.org, Ingo Molnar <mingo@...hat.com>,
Andy Lutomirski <luto@...nel.org>, "H. Peter Anvin" <hpa@...or.com>,
torvalds@...ux-foundation.org, alan@...ux.intel.com
Subject: Re: [PATCH v4.1 07/10] x86: narrow out of bounds syscalls to sys_read under speculation
> On Jan 21, 2018, at 11:16 AM, Andy Lutomirski <luto@...capital.net> wrote:
>
>
>
>> On Jan 20, 2018, at 1:06 PM, Dan Williams <dan.j.williams@...el.com> wrote:
>>
>> The syscall table base is a user controlled function pointer in kernel
>> space. Like, 'get_user, use 'MASK_NOSPEC' to prevent any out of bounds
>> speculation. While retpoline prevents speculating into the user
>> controlled target it does not stop the pointer de-reference, the concern
>> is leaking memory relative to the syscall table base.
>>
>> Reported-by: Linus Torvalds <torvalds@...ux-foundation.org>
>> Cc: Thomas Gleixner <tglx@...utronix.de>
>> Cc: Ingo Molnar <mingo@...hat.com>
>> Cc: "H. Peter Anvin" <hpa@...or.com>
>> Cc: x86@...nel.org
>> Cc: Andy Lutomirski <luto@...nel.org>
>> Signed-off-by: Dan Williams <dan.j.williams@...el.com>
>> ---
>> arch/x86/entry/entry_64.S | 2 ++
>> arch/x86/include/asm/smap.h | 9 ++++++++-
>> 2 files changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
>> index 63f4320602a3..584f6d2236b3 100644
>> --- a/arch/x86/entry/entry_64.S
>> +++ b/arch/x86/entry/entry_64.S
>> @@ -35,6 +35,7 @@
>> #include <asm/asm.h>
>> #include <asm/smap.h>
>> #include <asm/pgtable_types.h>
>> +#include <asm/smap.h>
>> #include <asm/export.h>
>> #include <asm/frame.h>
>> #include <asm/nospec-branch.h>
>> @@ -260,6 +261,7 @@ entry_SYSCALL_64_fastpath:
>> cmpl $__NR_syscall_max, %eax
>> #endif
>> ja 1f /* return -ENOSYS (already in pt_regs->ax) */
>> + MASK_NOSPEC %r11 %rax /* sanitize syscall_nr wrt speculation */
>
> What's the threat you're protecting against? No one does any computation or data dependent loads based on the value being read. Or are you worried about ASLR leaks due to merely speculatively loading from a bogus address.
Also, the actual guard you're using is unfortunate. At the very least, it's a maintenance disaster. Get rid of the macro and open code it. It's two instructions and it has crazy clobber requirements *and* a flag dependency.
But I think it's also just a bad way to do it. You're making the dependency chain longer in a place where it matters, and the retpoline prevents the CPU from hiding that latency. Just use an immediate mask with andq. Realistically, you should be fine rounding the table size up to a power of two without even extending the table, since I think this, at best, protects against ASLR bypass.
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
