x86_64,entry: Clear NT on entry and speed up switch_to
cover x86_64,entry: Clear NT on entry and speed up switch_to
Anish Bhatt noticed that user programs can set RFLAGS.NT before syscall or sysenter, and the kernel entry code doesn't filter out NT. This causes kernel C code and, depending on thread flags, the exit slow path to run with NT set.
The former is a little bit scary (imagine calling into EFI with NT set), and the latter will fail with #GP and send a spurious SIGSEGV.
One answer would be "don't do that". But the kernel can do better here.
These patches, which I'm not completely thrilled by, filter NT on all kernel entries. For syscall (both bitnesses), this is free. For sysenter, it costs 15 cycles or so. As a consolation prize, we can speed up context switches by avoiding saving and restoring flags.
If we don't like the added sysenter overhead, there are few other options:
- Try to optimize it by folding it with other flag manipulations (my attempt to do that didn't end up being any faster).
- Only do the syscall part. That's free, but it serves little purpose other than being polite to buggy userspace code.
- Don't filter NT on sysenter. Instead, filter it on EFI entry and modify the IRET code to retry without NT set if NT was set.
- Don't filter NT on sysenter. Instead, only filter it when sysenter jumps to the slow path. (This is trivial, but it does nothing to reduce the chance that evil user code can cause trouble by, say, reading from sysfs with NT set using sysenter.)
See: https://bugs.winehq.org/show_bug.cgi?id=33275
Andy Lutomirski (2):
x86_64,entry: Filter RFLAGS.NT on entry from userspace
x86_64: Don't save flags on context switch
arch/x86/ia32/ia32entry.S | 10 +++++++++- arch/x86/include/asm/switch_to.h | 10 +++++++--- arch/x86/kernel/cpu/common.c | 2 +- 3 files changed, 17 insertions(+), 5 deletions(-)
Signature · 1 line
1.9.3
PATCH 1/2 x86_64,entry: Filter RFLAGS.NT on entry from userspace
The NT flag doesn't do anything in long mode other than causing IRET to #GP. Oddly, CPL3 code can still net NT using popf.
Entry via hardware or software interrupt clears NT automatically, so the only relevant entries are fast syscalls.
This patch programs the CPU to clear NT on entry via SYSCALL (both 32-bit and 64-bit, by my reading of the AMD APM). It also clears NT (and some other flags) in software on SYSENTER.
I haven't touched anything on 32-bit kernels.
If user code causes kernel code to run with NT set, then there's at least some (small) chance that it could cause trouble. For example, user code could cause a call to EFI code with NT set, and who knows what would happen. Apparently Wine sometimes does this (!), and, if an IRET return happens, Wine will segfault.
I think that Wine should be fixed to stop setting NT when a syscall happens, but handling NT more gracefully is still nice.
The syscall mask change comes from a variant of this patch by Anish Bhatt.
--- arch/x86/ia32/ia32entry.S | 10 +++++++++- arch/x86/kernel/cpu/common.c | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-)
arch/x86/ia32/ia32entry.S +9 -1
@@ -143,7 +143,15 @@ ENTRY(ia32_sysenter_target) pushq_cfi %r10 CFI_REL_OFFSET rip,0 pushq_cfi %rax - cld + + /* + * Sysenter doesn't filter flags, so we should filter them + * ourselves. + */ + pushfq_cfi + andl $~(X86_EFLAGS_TF|X86_EFLAGS_DF|X86_EFLAGS_NT|X86_EFLAGS_IOPL),(%rsp) + popfq_cfi + SAVE_ARGS 0,1,0 /* no need to do an access_ok check here because rbp has been 32bit zero extended */
arch/x86/kernel/cpu/common.c +1 -1
@@ -1184,7 +1184,7 @@ void syscall_init(void)/* Flags to clear on syscall */ wrmsrl (MSR_SYSCALL_MASK ,X86_EFLAGS_TF |X86_EFLAGS_DF |X86_EFLAGS_IF | -X86_EFLAGS_IOPL |X86_EFLAGS_AC ); +X86_EFLAGS_IOPL |X86_EFLAGS_AC |X86_EFLAGS_NT ); }/*
Signature · 1 line
1.9.3
Re: [PATCH 1/2] x86_64,entry: Filter RFLAGS.NT on entry from userspace
Quoted 6 lines · Andy Lutomirski
what would happen. Apparently Wine sometimes does this (!), and, if an IRET return happens, Wine will segfault.
I think that Wine should be fixed to stop setting NT when a syscall happens, but handling NT more gracefully is still nice.
Just to give some more background about this issue: Wine has no influence if the NT flag is set or not - as Wine doesn't trace each individual opcode, there is no chance to know, if a Windows program messes up the EFLAGS. This happens in closed source Windows applications, so its not really Wines fault.
I think the current approach should be fine, but if other people prefer one of the solutions without additional overhead on syscall entry:
At least for Wine it would also be absolutely fine, when the application would just get a proper signal (if adding the retry IRET is too complicated). I've attached the url to an additional example program, which shows that currently the signal handler is unable to process such a fault, and a proof-of-concept patch to clear the NT flags at least for the signal handler. Such a patch doesn't fix the potential issues with EFI though.
Example program:
http://ix.io/ezl
Proof-of-concept patch:
http://ix.io/ezm
Regards,
Sebastian
Re: [PATCH 1/2] x86_64,entry: Filter RFLAGS.NT on entry from userspace
Quoted 4 lines · Sebastian Lackner
Quoted 3 lines · Andy Lutomirski
what would happen. Apparently Wine sometimes does this (!), and, if an IRET return happens, Wine will segfault.
Quoted 8 lines
Quoted 3 lines
I think that Wine should be fixed to stop setting NT when a syscall happens, but handling NT more gracefully is still nice.
Just to give some more background about this issue: Wine has no influence if the NT flag is set or not - as Wine doesn't trace each individual opcode, there is no chance to know, if a Windows program messes up the EFLAGS. This happens in closed source Windows applications, so its not really Wines fault.
I don't buy this explanation at all. Surely Wine is responsible for all system calls that happen. There's only a problem when NT is set and a system call happens.
Quoted 11 lines
I think the current approach should be fine, but if other people prefer one of the solutions without additional overhead on syscall entry:
At least for Wine it would also be absolutely fine, when the application would just get a proper signal (if adding the retry IRET is too complicated). I've attached the url to an additional example program, which shows that currently the signal handler is unable to process such a fault, and a proof-of-concept patch to clear the NT flags at least for the signal handler. Such a patch doesn't fix the potential issues with EFI though.
Quoted 5 lines
Example program:
http://ix.io/ezlProof-of-concept patch:
http://ix.io/ezm
This patch would make more sense if you only cleared it from the actual regs, not the saved regs. User code can do the latter on its own.
It would certainly be possible to clear NT and retry IRET if IRET fails with NT set. This would have no overhead for anything relevant. That would be this alternative from my 0/2 email:
- Don't filter NT on sysenter. Instead, filter it on EFI entry and modify the IRET code to retry without NT set if NT was set.
Thomas hpa, etc: any thoughts?
Quoted 4 lines
Regards,
Sebastian
Signature · 2 lines
Andy Lutomirski AMA Capital Management, LLC
Re: [PATCH 1/2] x86_64,entry: Filter RFLAGS.NT on entry from userspace
Quoted 10 lines · Andy Lutomirski
Quoted 8 lines · Sebastian Lackner
Quoted 6 lines · Andy Lutomirski
what would happen. Apparently Wine sometimes does this (!), and, if an IRET return happens, Wine will segfault.
I think that Wine should be fixed to stop setting NT when a syscall happens, but handling NT more gracefully is still nice.
Quoted 8 lines
Quoted 4 lines
Just to give some more background about this issue: Wine has no influence if the NT flag is set or not - as Wine doesn't trace each individual opcode, there is no chance to know, if a Windows program messes up the EFLAGS. This happens in closed source Windows applications, so its not really Wines fault.
I don't buy this explanation at all. Surely Wine is responsible for all system calls that happen. There's only a problem when NT is set and a system call happens.
Wine maps all the Windows API calls to their corresponding Linux equivalent shared library, you will rarely find any direct syscalls. This means it would involve adding special code to clear NT at the beginning of _each_ function, which can be called directly from the Windows application. Also callbacks to/from native code would have to be protected...
I doubt that gcc will ever add such a feature, but even if there would be something like that, it still wouldn't be a good solution - lot more overhead than a kernel solution. ;)
Quoted 15 lines
Quoted 15 lines
I think the current approach should be fine, but if other people prefer one of the solutions without additional overhead on syscall entry:
At least for Wine it would also be absolutely fine, when the application would just get a proper signal (if adding the retry IRET is too complicated). I've attached the url to an additional example program, which shows that currently the signal handler is unable to process such a fault, and a proof-of-concept patch to clear the NT flags at least for the signal handler. Such a patch doesn't fix the potential issues with EFI though.
Example program:
http://ix.io/ezl
Quoted 6 lines
Quoted 2 lines
Proof-of-concept patch:
http://ix.io/ezmThis patch would make more sense if you only cleared it from the actual regs, not the saved regs. User code can do the latter on its own.
Maybe I misunderstood the comments in the source, but it sounded like regs->flags contains the flags which are used to enter the signal handler ?! (which would mean that they are saved on the stack and used for the following IRET)
Having the NT flag still set at this point would cause immediately another recursive fault, if the signal handler tries to run any syscall, which is most probably not what the developer wants.
Quoted 7 lines
It would certainly be possible to clear NT and retry IRET if IRET fails with NT set. This would have no overhead for anything relevant. That would be this alternative from my 0/2 email:
- Don't filter NT on sysenter. Instead, filter it on EFI entry and modify the IRET code to retry without NT set if NT was set.
Yup, I saw that, but I'm not sure how small the overhead really is. Is it sufficient to check the NT flag and retry? What about other instructions, which could also cause a #GP, it wouldn't make sense to retry them, even if the NT flag is set (for some unknown reason). Not sure if it will cause any harm, but at least retry _only_ IRET instructions sounds a bit complicated.
Quoted 10 lines
Thomas hpa, etc: any thoughts?
Quoted 4 lines
Regards,
Sebastian
Re: [PATCH 1/2] x86_64,entry: Filter RFLAGS.NT on entry from userspace
In reply to Andy LutomirskiQuoted 8 lines · Andy Lutomirski
It would certainly be possible to clear NT and retry IRET if IRET fails with NT set. This would have no overhead for anything relevant. That would be this alternative from my 0/2 email:
- Don't filter NT on sysenter. Instead, filter it on EFI entry and modify the IRET code to retry without NT set if NT was set.
Thomas hpa, etc: any thoughts?
Filter it right away. That's solid and obvious. Anything else is just complex and prone for future brown paperbag failures.
We get the context switch benefit from it, so there is some compensation for the extra cycles.
Thanks,
tglx
Re: [PATCH 1/2] x86_64,entry: Filter RFLAGS.NT on entry from userspace
Quoted 5 lines · Thomas Gleixner
Quoted 4 lines · Andy Lutomirski
It would certainly be possible to clear NT and retry IRET if IRET fails with NT set. This would have no overhead for anything relevant. That would be this alternative from my 0/2 email:
Quoted 7 lines
Quoted 4 lines
- Don't filter NT on sysenter. Instead, filter it on EFI entry and modify the IRET code to retry without NT set if NT was set.
Thomas hpa, etc: any thoughts?
Filter it right away. That's solid and obvious. Anything else is just complex and prone for future brown paperbag failures.
Yeah, agreed. That's exactly what these patches do, although, if you put them in -tip and want to keep the stable CC, it's probably worth fixing the address (oops).
Quoted 3 lines
We get the context switch benefit from it, so there is some compensation for the extra cycles.
If we ever want those cycles back, I bet that the compat sysenter path could be trimmed down a lot. For example, I think that all of the zero-extension stuff is unnecessary now that we have the magic syscall wrappers for all (?) syscalls.
--Andy
Re: [PATCH 1/2] x86_64,entry: Filter RFLAGS.NT on entry from userspace
Quoted 9 lines · Andy Lutomirski
Quoted 8 lines · Thomas Gleixner
Quoted 7 lines · Andy Lutomirski
It would certainly be possible to clear NT and retry IRET if IRET fails with NT set. This would have no overhead for anything relevant. That would be this alternative from my 0/2 email:
- Don't filter NT on sysenter. Instead, filter it on EFI entry and modify the IRET code to retry without NT set if NT was set.
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Thomas hpa, etc: any thoughts?
Filter it right away. That's solid and obvious. Anything else is just complex and prone for future brown paperbag failures.
Yeah, agreed. That's exactly what these patches do, although, if you put them in -tip and want to keep the stable CC, it's probably worth fixing the address (oops).
Even more oops as you failed to update it in your reply again ....
Quoted 7 lines
Quoted 2 lines
We get the context switch benefit from it, so there is some compensation for the extra cycles.
If we ever want those cycles back, I bet that the compat sysenter path could be trimmed down a lot. For example, I think that all of the zero-extension stuff is unnecessary now that we have the magic syscall wrappers for all (?) syscalls.
Emphasis on "(?)". So yes, once we verified that ....
Thanks,
tglx
H. Peter Anvin
reviewerRe: [PATCH 1/2] x86_64,entry: Filter RFLAGS.NT on entry from userspace
Quoted 8 lines · Thomas Gleixner
Quoted 5 lines
If we ever want those cycles back, I bet that the compat sysenter path could be trimmed down a lot. For example, I think that all of the zero-extension stuff is unnecessary now that we have the magic syscall wrappers for all (?) syscalls.
Emphasis on "(?)". So yes, once we verified that ....
I don't think that's true. Many system calls use exactly the same entry point for compat and noncompat calls. I don't see any value in replicating that code in every system call.
The only time we need to do anything horribly special is when we have an argument which is a signed long, which fortunately is not at all common.
-hpa
H. Peter Anvin
reviewerRe: [PATCH 1/2] x86_64,entry: Filter RFLAGS.NT on entry from userspace
Quoted 10 lines · H. Peter Anvin
Quoted 8 lines · Thomas Gleixner
Quoted 5 lines
If we ever want those cycles back, I bet that the compat sysenter path could be trimmed down a lot. For example, I think that all of the zero-extension stuff is unnecessary now that we have the magic syscall wrappers for all (?) syscalls.
Emphasis on "(?)". So yes, once we verified that ....
Quoted 7 lines
I don't think that's true. Many system calls use exactly the same entry point for compat and noncompat calls. I don't see any value in replicating that code in every system call.
The only time we need to do anything horribly special is when we have an argument which is a signed long, which fortunately is not at all common.
Also, we still need to shuffle registers around to match the x86-64 calling convention. Zero extension as part of that shuffle is free.
-hpa
H. Peter Anvin
reviewerRe: [PATCH 1/2] x86_64,entry: Filter RFLAGS.NT on entry from userspace
In reply to Thomas GleixnerNicely enough we get the context switch benefit without any cost in the comon case of 64-bit apps.
Quoted 20 lines · Thomas Gleixner
Quoted 2 lines · Andy Lutomirski
It would certainly be possible to clear NT and retry IRET if IRET fails with NT set. This would have no overhead for anything
relevant.
Quoted 6 lines
That would be this alternative from my 0/2 email:
- Don't filter NT on sysenter. Instead, filter it on EFI entry and modify the IRET code to retry without NT set if NT was set.
Thomas hpa, etc: any thoughts?
Filter it right away. That's solid and obvious. Anything else is just complex and prone for future brown paperbag failures.
We get the context switch benefit from it, so there is some compensation for the extra cycles.
Thanks,
tglx
Signature · 1 line
Sent from my mobile phone. Please pardon brevity and lack of formatting.
Re: [PATCH 1/2] x86_64,entry: Filter RFLAGS.NT on entry from userspace
In reply to Andy LutomirskiQuoted 39 lines · On Tue, 30 Sep 2014 12:40:35 -0700 Andy Lutomirski <[email protected]> wrote:
The NT flag doesn't do anything in long mode other than causing IRET to #GP. Oddly, CPL3 code can still net NT using popf.
Entry via hardware or software interrupt clears NT automatically, so the only relevant entries are fast syscalls.
This patch programs the CPU to clear NT on entry via SYSCALL (both 32-bit and 64-bit, by my reading of the AMD APM). It also clears NT (and some other flags) in software on SYSENTER.
I haven't touched anything on 32-bit kernels.
If user code causes kernel code to run with NT set, then there's at least some (small) chance that it could cause trouble. For example, user code could cause a call to EFI code with NT set, and who knows what would happen. Apparently Wine sometimes does this (!), and, if an IRET return happens, Wine will segfault.
I think that Wine should be fixed to stop setting NT when a syscall happens, but handling NT more gracefully is still nice.
The syscall mask change comes from a variant of this patch by Anish Bhatt.
Reported-by Anish Bhatt <[email protected]>Signed-off-by Andy Lutomirski <[email protected]>--- arch/x86/ia32/ia32entry.S | 10 +++++++++- arch/x86/kernel/cpu/common.c | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-)arch/x86/ia32/ia32entry.S +0
-0@@ -143,7 +143,15 @@ ENTRY(ia32_sysenter_target) pushq_cfi %r10 CFI_REL_OFFSET rip,0
arch/x86/ia32/ia32entry.S ENTRY(ia32_sysenter_target) · line 153
Quoted 10 lines
arch/x86/ia32/ia32entry.S +8
-1pushq_cfi %rax - cld + + /* + * Sysenter doesn't filter flags, so we should filter them + * ourselves. + */ + pushfq_cfi + andl $~(X86_EFLAGS_TF|X86_EFLAGS_DF|X86_EFLAGS_NT|X86_EFLAGS_IOPL),(%rsp) + popfq_cfi
Can't you just push a constant and pop that onto flags instead? It's not like we care what's in there on entry to the kernel.
Quoted 17 lines
arch/x86/ia32/ia32entry.S +1
-0+ SAVE_ARGS 0,1,0 /* no need to do an access_ok check here because rbp has been 32bit zero extended */arch/x86/kernel/cpu/common.c +1
-1@@ -1184,7 +1184,7 @@ void syscall_init(void)/* Flags to clear on syscall */ wrmsrl (MSR_SYSCALL_MASK ,X86_EFLAGS_TF |X86_EFLAGS_DF |X86_EFLAGS_IF | -X86_EFLAGS_IOPL |X86_EFLAGS_AC ); +X86_EFLAGS_IOPL |X86_EFLAGS_AC |X86_EFLAGS_NT ); }/*
Re: [PATCH 1/2] x86_64,entry: Filter RFLAGS.NT on entry from userspace
Quoted 47 lines · Chuck Ebbert
Quoted 44 lines · On Tue, 30 Sep 2014 12:40:35 -0700 Andy Lutomirski <[email protected]> wrote:
The NT flag doesn't do anything in long mode other than causing IRET to #GP. Oddly, CPL3 code can still net NT using popf.
Entry via hardware or software interrupt clears NT automatically, so the only relevant entries are fast syscalls.
This patch programs the CPU to clear NT on entry via SYSCALL (both 32-bit and 64-bit, by my reading of the AMD APM). It also clears NT (and some other flags) in software on SYSENTER.
I haven't touched anything on 32-bit kernels.
If user code causes kernel code to run with NT set, then there's at least some (small) chance that it could cause trouble. For example, user code could cause a call to EFI code with NT set, and who knows what would happen. Apparently Wine sometimes does this (!), and, if an IRET return happens, Wine will segfault.
I think that Wine should be fixed to stop setting NT when a syscall happens, but handling NT more gracefully is still nice.
The syscall mask change comes from a variant of this patch by Anish Bhatt.
Reported-by Anish Bhatt <[email protected]>Signed-off-by Andy Lutomirski <[email protected]>--- arch/x86/ia32/ia32entry.S | 10 +++++++++- arch/x86/kernel/cpu/common.c | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-)arch/x86/ia32/ia32entry.S +3
-1@@ -143,7 +143,15 @@ ENTRY(ia32_sysenter_target) pushq_cfi %r10 CFI_REL_OFFSET rip,0 pushq_cfi %rax - cld + + /* + * Sysenter doesn't filter flags, so we should filter them
Quoted 8 lines
arch/x86/ia32/ia32entry.S ENTRY(ia32_sysenter_target) · line 153
Quoted 5 lines
arch/x86/ia32/ia32entry.S +5
-0+ * ourselves. + */ + pushfq_cfi + andl $~(X86_EFLAGS_TF|X86_EFLAGS_DF|X86_EFLAGS_NT|X86_EFLAGS_IOPL),(%rsp) + popfq_cfiCan't you just push a constant and pop that onto flags instead? It's not like we care what's in there on entry to the kernel.
I don't see why not. I'll test on Xen just to be sure.
--Andy
PATCH 2/2 x86_64: Don't save flags on context switch
In reply to Andy LutomirskiNow that the kernel always runs with clean flags (in particular, NT is clear), there is no need to save and restore flags on every context switch.
--- arch/x86/include/asm/switch_to.h | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
arch/x86/include/asm/switch_to.h +7 -3
@@ -79,8 +79,8 @@ do { \#else /* CONFIG_X86_32 */ /* frame pointer must be last for get_wchan */ -#define SAVE_CONTEXT "pushf ; pushq %%rbp ; movq %%rsi,%%rbp\n\t" -#define RESTORE_CONTEXT "movq %%rbp,%%rsi ; popq %%rbp ; popf\t" +#define SAVE_CONTEXT "pushq %%rbp ; movq %%rsi,%%rbp\n\t" +#define RESTORE_CONTEXT "movq %%rbp,%%rsi ; popq %%rbp\t"#define __EXTRA_CLOBBER \ , "rcx", "rbx", "rdx", "r8", "r9", "r10", "r11", \
@@ -100,7 +100,11 @@ do { \#define __switch_canary_iparam#endif /* CC_STACKPROTECTOR */ -/* Save restore flags to clear handle leaking NT */ +/* +* There is no need to save or restore flags, because flags are always +* clean in kernel mode, with the possible exception of IOPL. Kernel IOPL +* has no effect. +*/ #define switch_to (prev, next, last) \ asm volatile(SAVE_CONTEXT \ "movq %%rsp,%P[threadrsp](%[prev])\n\t"/* save RSP */ \
Signature · 1 line
1.9.3
Quoted 8 lines · Andy Lutomirski
Anish Bhatt noticed that user programs can set RFLAGS.NT before syscall or sysenter, and the kernel entry code doesn't filter out NT. This causes kernel C code and, depending on thread flags, the exit slow path to run with NT set.
The former is a little bit scary (imagine calling into EFI with NT set), and the latter will fail with #GP and send a spurious SIGSEGV.
Quoted 7 lines
One answer would be "don't do that". But the kernel can do better here.
These patches, which I'm not completely thrilled by, filter NT on all kernel entries. For syscall (both bitnesses), this is free. For sysenter, it costs 15 cycles or so. As a consolation prize, we can speed up context switches by avoiding saving and restoring flags.
That's a nice reason not to do any of the other ugly variants.
Thanks,
tglx
Quoted 12 lines · Thomas Gleixner
Quoted 11 lines · Andy Lutomirski
Anish Bhatt noticed that user programs can set RFLAGS.NT before syscall or sysenter, and the kernel entry code doesn't filter out NT. This causes kernel C code and, depending on thread flags, the exit slow path to run with NT set.
The former is a little bit scary (imagine calling into EFI with NT set), and the latter will fail with #GP and send a spurious SIGSEGV.
One answer would be "don't do that". But the kernel can do better here.
Quoted 6 lines
Quoted 4 lines
These patches, which I'm not completely thrilled by, filter NT on all kernel entries. For syscall (both bitnesses), this is free. For sysenter, it costs 15 cycles or so. As a consolation prize, we can speed up context switches by avoiding saving and restoring flags.
That's a nice reason not to do any of the other ugly variants.
We could do something hideous:
Don't filter NT in sysexit or on context switch. Instead, handle it in bad_iret.
Up side: all common cases are maximally fast.
Down side: Ugly. And malicious processes can leak NT, causing return to a different process to fault, thereby adding a thousand or two cycles (or possibly a lot more if the fault hits in the middle of espfix64. Egads.)
This is not intended to be a serious suggestion...
--Andy