Message ID | 20200122012932.129013-1-asteinhauser@google.com |
---|---|
State | Rejected |
Headers |
Received: (qmail 107662 invoked by alias); 22 Jan 2020 01:29:43 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 107647 invoked by uid 89); 22 Jan 2020 01:29:43 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=channels, manages, privilege, 1939 X-HELO: mail-pj1-f74.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:message-id:mime-version:subject:from:to:cc; bh=EolQ8G/OSOEy5EBpukvUWalCg8m1sKzOC8F1Xhgh7f8=; b=itQh3+lyTW7dh/V2ogMF/ZGu+nqG4sNDyyvWtsnoRQIdcPmVn77bMKhlAfUd5mNxfN E4hQhacbi3qY+PC4b/umNRi1WkAwqrwr3uGaSW95wvq22HV6iVJ533+gcIpnEAt3dPNf HfBEVEqSToi+UN9SyGoxbaIAkTWBCOx4WNdSx4aMNRTTmsk5sETUVii7otc2nfvShdJd ZTh+NfhK70ACnEKMicA+2hFOpDsQHFAFja1klwHWCPoaokWX1CkduKxOSnlX+jZWADgI yEF9Dcxk858iwjrnFRaRNZiSMeoUJ71KlB0NJXEQOUx7TLzTtaNDPAowbf2nDm0utkAB rUog== Date: Tue, 21 Jan 2020 17:29:32 -0800 Message-Id: <20200122012932.129013-1-asteinhauser@google.com> Mime-Version: 1.0 Subject: [PATCH] aarch64: fix speculative execution past SVC vulnerability From: Anthony Steinhauser <asteinhauser@google.com> To: libc-alpha@sourceware.org Cc: Anthony Steinhauser <asteinhauser@google.com> Content-Type: text/plain; charset="UTF-8" |
Commit Message
Anthony Steinhauser
Jan. 22, 2020, 1:29 a.m. UTC
Even though SVC always causes a jump to another address, aarch64 CPUs
speculatively execute following instructions as if the SVC
instruction was not a jump instruction.
The speculative execution does not cross privilege-levels (to the jump
target as one would expect), but it continues on the kernel privilege
level as if the SVC instruction did not change the control flow -
thus execution anything that is accidentally linked after the SVC
instruction. Later, the results of this speculative execution are
always architecturally discarded, however they can leak data using
microarchitectural side channels. This speculative execution is very
reliable (seems to be unconditional) and it manages to complete even
relatively performance-heavy operations (e.g. multiple dependent
fetches from uncached memory).
Something like:
read(2)
...
[parse read result]
Gets potentially dangerous, since this would allow us potentially
reasonably reliably hit uninitialized memory (with potentially attacker
controlled offsets from prior reads) if we speculate over the SVC in the
syscall. Particularly in a netspectre-like scope.
Bugzilla record:
https://sourceware.org/bugzilla/show_bug.cgi?id=25436
Signed-off-by: Anthony Steinhauser <asteinhauser@google.com>
---
sysdeps/unix/sysv/linux/aarch64/sysdep.h | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
Comments
* Anthony Steinhauser: > Even though SVC always causes a jump to another address, aarch64 CPUs > speculatively execute following instructions as if the SVC > instruction was not a jump instruction. > The speculative execution does not cross privilege-levels (to the jump > target as one would expect), but it continues on the kernel privilege > level as if the SVC instruction did not change the control flow - > thus execution anything that is accidentally linked after the SVC > instruction. The PDF attached to the bug report says this: | The speculative execution does not cross privilege-levels (to the jump | target as one would expect), but it continues on the current privilege | level as if those instructions did not change the control flow. I assume that “current privilege level” means “user mode”. Which is it? If it is possible to speculatively execute userspace code in kernel mode, then a simple userspace code change will not fix this because an attacker can simply use their own system call sequence. Is there a performance benefit from inhibiting speculative execution? It seems wasteful in many cases because it does take the side effects of the system call into account. Do we have to make this conditi onal on the CPU? It wouldn't be fair to penalize vendors who implement this correctly. (As a general rule, I really dislike software mitigation attempts because they are usually incomplete and slow down execution for everyone, without actually fixing any security issue.) Thanks, Florian
On 22/01/2020 01:29, Anthony Steinhauser wrote: > Even though SVC always causes a jump to another address, aarch64 CPUs > speculatively execute following instructions as if the SVC > instruction was not a jump instruction. thanks for the patch, it looks good, but i add somebody on cc who spent more time on spectre mitigation in case there is further insight. > The speculative execution does not cross privilege-levels (to the jump > target as one would expect), but it continues on the kernel privilege > level as if the SVC instruction did not change the control flow - i don't understand this bit: why is anything executing on the kernel privilege level here? > thus execution anything that is accidentally linked after the SVC > instruction. Later, the results of this speculative execution are > always architecturally discarded, however they can leak data using > microarchitectural side channels. This speculative execution is very > reliable (seems to be unconditional) and it manages to complete even > relatively performance-heavy operations (e.g. multiple dependent > fetches from uncached memory). > > Something like: > read(2) > ... > [parse read result] > Gets potentially dangerous, since this would allow us potentially > reasonably reliably hit uninitialized memory (with potentially attacker > controlled offsets from prior reads) if we speculate over the SVC in the > syscall. Particularly in a netspectre-like scope. > > Bugzilla record: > https://sourceware.org/bugzilla/show_bug.cgi?id=25436 > > Signed-off-by: Anthony Steinhauser <asteinhauser@google.com> > --- > sysdeps/unix/sysv/linux/aarch64/sysdep.h | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/sysdeps/unix/sysv/linux/aarch64/sysdep.h b/sysdeps/unix/sysv/linux/aarch64/sysdep.h > index 00b8e241c8..cf1e6ed45f 100644 > --- a/sysdeps/unix/sysv/linux/aarch64/sysdep.h > +++ b/sysdeps/unix/sysv/linux/aarch64/sysdep.h > @@ -147,7 +147,9 @@ > # undef DO_CALL > # define DO_CALL(syscall_name, args) \ > mov x8, SYS_ify (syscall_name); \ > - svc 0 > + svc 0; \ > + dsb nsh; \ > + isb > > #else /* not __ASSEMBLER__ */ > > @@ -191,7 +193,9 @@ > { \ > LOAD_ARGS_##nr (args) \ > register long _x8 asm ("x8") = (name); \ > - asm volatile ("svc 0 // syscall " # name \ > + asm volatile ("svc 0 // syscall\n\t" # name \ > + "dsb nsh\n\t" \ > + "isb" \ > : "=r" (_x0) : "r"(_x8) ASM_ARGS_##nr : "memory"); \ > _sys_result = _x0; \ > } \ >
On 22/01/2020 10:08, Szabolcs Nagy wrote: > On 22/01/2020 01:29, Anthony Steinhauser wrote: >> { \ >> LOAD_ARGS_##nr (args) \ >> register long _x8 asm ("x8") = (name); \ >> - asm volatile ("svc 0 // syscall " # name \ >> + asm volatile ("svc 0 // syscall\n\t" # name \ \n before the name looks wrong >> + "dsb nsh\n\t" \ >> + "isb" \ >> : "=r" (_x0) : "r"(_x8) ASM_ARGS_##nr : "memory"); \ >> _sys_result = _x0; \ >> } \
*Florian Weimer It's the user mode in the case of SVC instruction. I incorrectly copied that passage from the ERET kernel fixes. Sorry for that. If there are vendors or CPUs who implement this correctly, it would be good to make it CPU-specific. On the ARM and Cavium CPUs that I tested the control flow past SVC is always mispredicted, so it seems like the barrier there would have little if any overhead. However, it does not enhance the performance either. It just prevents very similar Spectre-like effects as in the ERET case. *Szabolcs Nagy I'm not sure whether the endline should be wrong. Without it, the resulting assembly would look like: svc 0 // syscall dsb nsh isb So the dsb nsh instruction will be commented-out. Or am I missing something? On Wed, Jan 22, 2020 at 2:12 AM Szabolcs Nagy <Szabolcs.Nagy@arm.com> wrote: > > On 22/01/2020 10:08, Szabolcs Nagy wrote: > > On 22/01/2020 01:29, Anthony Steinhauser wrote: > >> { \ > >> LOAD_ARGS_##nr (args) \ > >> register long _x8 asm ("x8") = (name); \ > >> - asm volatile ("svc 0 // syscall " # name \ > >> + asm volatile ("svc 0 // syscall\n\t" # name \ > > \n before the name looks wrong > > >> + "dsb nsh\n\t" \ > >> + "isb" \ > >> : "=r" (_x0) : "r"(_x8) ASM_ARGS_##nr : "memory"); \ > >> _sys_result = _x0; \ > >> } \
On Wed, Jan 22, 2020 at 12:39 PM Anthony Steinhauser <asteinhauser@google.com> wrote: > > *Florian Weimer > It's the user mode in the case of SVC instruction. I incorrectly > copied that passage from the ERET kernel fixes. Sorry for that. > If there are vendors or CPUs who implement this correctly, it would be > good to make it CPU-specific. > On the ARM and Cavium CPUs that I tested the control flow past SVC is > always mispredicted, so it seems like the barrier there would have > little if any overhead. > However, it does not enhance the performance either. It just prevents > very similar Spectre-like effects as in the ERET case. > > *Szabolcs Nagy > I'm not sure whether the endline should be wrong. Without it, the > resulting assembly would look like: > svc 0 // syscall dsb nsh > isb > So the dsb nsh instruction will be commented-out. Or am I missing something? He means the return (\n) is misplaced. It should be after the #name. e.g. asm volatile ("svc 0 // syscall" # name "\n\t" Thanks, Andrew Pinski > > > On Wed, Jan 22, 2020 at 2:12 AM Szabolcs Nagy <Szabolcs.Nagy@arm.com> wrote: > > > > On 22/01/2020 10:08, Szabolcs Nagy wrote: > > > On 22/01/2020 01:29, Anthony Steinhauser wrote: > > >> { \ > > >> LOAD_ARGS_##nr (args) \ > > >> register long _x8 asm ("x8") = (name); \ > > >> - asm volatile ("svc 0 // syscall " # name \ > > >> + asm volatile ("svc 0 // syscall\n\t" # name \ > > > > \n before the name looks wrong > > > > >> + "dsb nsh\n\t" \ > > >> + "isb" \ > > >> : "=r" (_x0) : "r"(_x8) ASM_ARGS_##nr : "memory"); \ > > >> _sys_result = _x0; \ > > >> } \
I get it. Sorry for the confusion. On Wed, Jan 22, 2020 at 12:43 PM Andrew Pinski <pinskia@gmail.com> wrote: > > On Wed, Jan 22, 2020 at 12:39 PM Anthony Steinhauser > <asteinhauser@google.com> wrote: > > > > *Florian Weimer > > It's the user mode in the case of SVC instruction. I incorrectly > > copied that passage from the ERET kernel fixes. Sorry for that. > > If there are vendors or CPUs who implement this correctly, it would be > > good to make it CPU-specific. > > On the ARM and Cavium CPUs that I tested the control flow past SVC is > > always mispredicted, so it seems like the barrier there would have > > little if any overhead. > > However, it does not enhance the performance either. It just prevents > > very similar Spectre-like effects as in the ERET case. > > > > *Szabolcs Nagy > > I'm not sure whether the endline should be wrong. Without it, the > > resulting assembly would look like: > > svc 0 // syscall dsb nsh > > isb > > So the dsb nsh instruction will be commented-out. Or am I missing something? > > He means the return (\n) is misplaced. It should be after the #name. > e.g. > asm volatile ("svc 0 // syscall" # name "\n\t" > > Thanks, > Andrew Pinski > > > > > > > On Wed, Jan 22, 2020 at 2:12 AM Szabolcs Nagy <Szabolcs.Nagy@arm.com> wrote: > > > > > > On 22/01/2020 10:08, Szabolcs Nagy wrote: > > > > On 22/01/2020 01:29, Anthony Steinhauser wrote: > > > >> { \ > > > >> LOAD_ARGS_##nr (args) \ > > > >> register long _x8 asm ("x8") = (name); \ > > > >> - asm volatile ("svc 0 // syscall " # name \ > > > >> + asm volatile ("svc 0 // syscall\n\t" # name \ > > > > > > \n before the name looks wrong > > > > > > >> + "dsb nsh\n\t" \ > > > >> + "isb" \ > > > >> : "=r" (_x0) : "r"(_x8) ASM_ARGS_##nr : "memory"); \ > > > >> _sys_result = _x0; \ > > > >> } \
* Anthony Steinhauser <asteinhauser@google.com> [2020-01-22 12:39:37 -0800]: > *Florian Weimer > It's the user mode in the case of SVC instruction. I incorrectly > copied that passage from the ERET kernel fixes. Sorry for that. please write a commit message that has enough detail so we understand why you want to fix this particular user mode speculation issue if there is no privilege escalation. > If there are vendors or CPUs who implement this correctly, it would be > good to make it CPU-specific. > On the ARM and Cavium CPUs that I tested the control flow past SVC is > always mispredicted, so it seems like the barrier there would have > little if any overhead. > However, it does not enhance the performance either. It just prevents > very similar Spectre-like effects as in the ERET case. > > *Szabolcs Nagy > I'm not sure whether the endline should be wrong. Without it, the > resulting assembly would look like: > svc 0 // syscall dsb nsh > isb > So the dsb nsh instruction will be commented-out. Or am I missing something? > > > On Wed, Jan 22, 2020 at 2:12 AM Szabolcs Nagy <Szabolcs.Nagy@arm.com> wrote: > > > > On 22/01/2020 10:08, Szabolcs Nagy wrote: > > > On 22/01/2020 01:29, Anthony Steinhauser wrote: > > >> { \ > > >> LOAD_ARGS_##nr (args) \ > > >> register long _x8 asm ("x8") = (name); \ > > >> - asm volatile ("svc 0 // syscall " # name \ > > >> + asm volatile ("svc 0 // syscall\n\t" # name \ > > > > \n before the name looks wrong > > > > >> + "dsb nsh\n\t" \ > > >> + "isb" \ > > >> : "=r" (_x0) : "r"(_x8) ASM_ARGS_##nr : "memory"); \ > > >> _sys_result = _x0; \ > > >> } \
diff --git a/sysdeps/unix/sysv/linux/aarch64/sysdep.h b/sysdeps/unix/sysv/linux/aarch64/sysdep.h index 00b8e241c8..cf1e6ed45f 100644 --- a/sysdeps/unix/sysv/linux/aarch64/sysdep.h +++ b/sysdeps/unix/sysv/linux/aarch64/sysdep.h @@ -147,7 +147,9 @@ # undef DO_CALL # define DO_CALL(syscall_name, args) \ mov x8, SYS_ify (syscall_name); \ - svc 0 + svc 0; \ + dsb nsh; \ + isb #else /* not __ASSEMBLER__ */ @@ -191,7 +193,9 @@ { \ LOAD_ARGS_##nr (args) \ register long _x8 asm ("x8") = (name); \ - asm volatile ("svc 0 // syscall " # name \ + asm volatile ("svc 0 // syscall\n\t" # name \ + "dsb nsh\n\t" \ + "isb" \ : "=r" (_x0) : "r"(_x8) ASM_ARGS_##nr : "memory"); \ _sys_result = _x0; \ } \