* [PATCH] x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32
@ 2016-02-23 21:19 Andy Lutomirski
2016-02-24 15:46 ` Brian Gerst
2016-02-25 5:53 ` [tip:x86/urgent] " tip-bot for Andy Lutomirski
0 siblings, 2 replies; 21+ messages in thread
From: Andy Lutomirski @ 2016-02-23 21:19 UTC (permalink / raw)
To: x86; +Cc: Borislav Petkov, linux-kernel, Andy Lutomirski
Both before and after 5f310f739b4c ("x86/entry/32: Re-implement
SYSENTER using the new C path"), we relied on a uaccess very early
in the SYSENTER path to clear AC. After that change, though, we can
potentially make it all the way into C code with AC set, which
enlarges the attack surface for SMAP bypass by doing SYSENTER with
AC set.
Strengthen the SMAP protection by addding the missing ASM_CLAC right
at the beginning.
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
This is probably an x86/urgent candidate. It fixes a minor
hardening regression in 4.4.
It's lightly tested. It's hard to test well right now because the
4.5 series is completely broken for 32-bit SMAP sytems.
arch/x86/entry/entry_32.S | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index f3facd40fd2d..9d6165c171eb 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -294,6 +294,7 @@ sysenter_past_esp:
pushl $__USER_DS /* pt_regs->ss */
pushl %ebp /* pt_regs->sp (stashed in bp) */
pushfl /* pt_regs->flags (except IF = 0) */
+ ASM_CLAC /* Clear AC after saving FLAGS */
orl $X86_EFLAGS_IF, (%esp) /* Fix IF */
pushl $__USER_CS /* pt_regs->cs */
pushl $0 /* pt_regs->ip = 0 (placeholder) */
--
2.5.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH] x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 2016-02-23 21:19 [PATCH] x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 Andy Lutomirski @ 2016-02-24 15:46 ` Brian Gerst 2016-02-24 16:56 ` Andy Lutomirski 2016-02-25 5:53 ` [tip:x86/urgent] " tip-bot for Andy Lutomirski 1 sibling, 1 reply; 21+ messages in thread From: Brian Gerst @ 2016-02-24 15:46 UTC (permalink / raw) To: Andy Lutomirski Cc: the arch/x86 maintainers, Borislav Petkov, Linux Kernel Mailing List On Tue, Feb 23, 2016 at 4:19 PM, Andy Lutomirski <luto@kernel.org> wrote: > Both before and after 5f310f739b4c ("x86/entry/32: Re-implement > SYSENTER using the new C path"), we relied on a uaccess very early > in the SYSENTER path to clear AC. After that change, though, we can > potentially make it all the way into C code with AC set, which > enlarges the attack surface for SMAP bypass by doing SYSENTER with > AC set. > > Strengthen the SMAP protection by addding the missing ASM_CLAC right > at the beginning. > > Signed-off-by: Andy Lutomirski <luto@kernel.org> > --- > > This is probably an x86/urgent candidate. It fixes a minor > hardening regression in 4.4. > > It's lightly tested. It's hard to test well right now because the > 4.5 series is completely broken for 32-bit SMAP sytems. > > arch/x86/entry/entry_32.S | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S > index f3facd40fd2d..9d6165c171eb 100644 > --- a/arch/x86/entry/entry_32.S > +++ b/arch/x86/entry/entry_32.S > @@ -294,6 +294,7 @@ sysenter_past_esp: > pushl $__USER_DS /* pt_regs->ss */ > pushl %ebp /* pt_regs->sp (stashed in bp) */ > pushfl /* pt_regs->flags (except IF = 0) */ > + ASM_CLAC /* Clear AC after saving FLAGS */ > orl $X86_EFLAGS_IF, (%esp) /* Fix IF */ > pushl $__USER_CS /* pt_regs->cs */ > pushl $0 /* pt_regs->ip = 0 (placeholder) */ > -- > 2.5.0 > It looks like entry_INT80_compat is also missing a CLAC. -- Brian Gerst ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 2016-02-24 15:46 ` Brian Gerst @ 2016-02-24 16:56 ` Andy Lutomirski 0 siblings, 0 replies; 21+ messages in thread From: Andy Lutomirski @ 2016-02-24 16:56 UTC (permalink / raw) To: Brian Gerst Cc: Andy Lutomirski, the arch/x86 maintainers, Borislav Petkov, Linux Kernel Mailing List On Wed, Feb 24, 2016 at 7:46 AM, Brian Gerst <brgerst@gmail.com> wrote: > On Tue, Feb 23, 2016 at 4:19 PM, Andy Lutomirski <luto@kernel.org> wrote: >> Both before and after 5f310f739b4c ("x86/entry/32: Re-implement >> SYSENTER using the new C path"), we relied on a uaccess very early >> in the SYSENTER path to clear AC. After that change, though, we can >> potentially make it all the way into C code with AC set, which >> enlarges the attack surface for SMAP bypass by doing SYSENTER with >> AC set. >> >> Strengthen the SMAP protection by addding the missing ASM_CLAC right >> at the beginning. >> >> Signed-off-by: Andy Lutomirski <luto@kernel.org> >> --- >> >> This is probably an x86/urgent candidate. It fixes a minor >> hardening regression in 4.4. >> >> It's lightly tested. It's hard to test well right now because the >> 4.5 series is completely broken for 32-bit SMAP sytems. >> >> arch/x86/entry/entry_32.S | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S >> index f3facd40fd2d..9d6165c171eb 100644 >> --- a/arch/x86/entry/entry_32.S >> +++ b/arch/x86/entry/entry_32.S >> @@ -294,6 +294,7 @@ sysenter_past_esp: >> pushl $__USER_DS /* pt_regs->ss */ >> pushl %ebp /* pt_regs->sp (stashed in bp) */ >> pushfl /* pt_regs->flags (except IF = 0) */ >> + ASM_CLAC /* Clear AC after saving FLAGS */ >> orl $X86_EFLAGS_IF, (%esp) /* Fix IF */ >> pushl $__USER_CS /* pt_regs->cs */ >> pushl $0 /* pt_regs->ip = 0 (placeholder) */ >> -- >> 2.5.0 >> > > It looks like entry_INT80_compat is also missing a CLAC. > Indeed, and that's a much worse bug. For better or for worse, I think it's been buggy since the beginning (3.10). Patch coming. --Andy -- Andy Lutomirski AMA Capital Management, LLC ^ permalink raw reply [flat|nested] 21+ messages in thread
* [tip:x86/urgent] x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 2016-02-23 21:19 [PATCH] x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 Andy Lutomirski 2016-02-24 15:46 ` Brian Gerst @ 2016-02-25 5:53 ` tip-bot for Andy Lutomirski 2016-02-25 6:00 ` H. Peter Anvin 1 sibling, 1 reply; 21+ messages in thread From: tip-bot for Andy Lutomirski @ 2016-02-25 5:53 UTC (permalink / raw) To: linux-tip-commits Cc: dvlasenk, bp, luto, hpa, luto, linux-kernel, torvalds, tglx, brgerst, mingo, peterz Commit-ID: 04d1d281dcfe683a53cddfab8371fc8bb302b069 Gitweb: http://git.kernel.org/tip/04d1d281dcfe683a53cddfab8371fc8bb302b069 Author: Andy Lutomirski <luto@kernel.org> AuthorDate: Tue, 23 Feb 2016 13:19:29 -0800 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Wed, 24 Feb 2016 08:43:04 +0100 x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 Both before and after 5f310f739b4c ("x86/entry/32: Re-implement SYSENTER using the new C path"), we relied on a uaccess very early in the SYSENTER path to clear AC. After that change, though, we can potentially make it all the way into C code with AC set, which enlarges the attack surface for SMAP bypass by doing SYSENTER with AC set. Strengthen the SMAP protection by addding the missing ASM_CLAC right at the beginning. Signed-off-by: Andy Lutomirski <luto@kernel.org> Cc: Andy Lutomirski <luto@amacapital.net> Cc: Borislav Petkov <bp@alien8.de> Cc: Brian Gerst <brgerst@gmail.com> Cc: Denys Vlasenko <dvlasenk@redhat.com> Cc: H. Peter Anvin <hpa@zytor.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Link: http://lkml.kernel.org/r/3e36be110724896e32a4a1fe73bacb349d3cba94.1456262295.git.luto@kernel.org Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/entry/entry_32.S | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S index 77d8c51..bb3e376 100644 --- a/arch/x86/entry/entry_32.S +++ b/arch/x86/entry/entry_32.S @@ -294,6 +294,7 @@ sysenter_past_esp: pushl $__USER_DS /* pt_regs->ss */ pushl %ebp /* pt_regs->sp (stashed in bp) */ pushfl /* pt_regs->flags (except IF = 0) */ + ASM_CLAC /* Clear AC after saving FLAGS */ orl $X86_EFLAGS_IF, (%esp) /* Fix IF */ pushl $__USER_CS /* pt_regs->cs */ pushl $0 /* pt_regs->ip = 0 (placeholder) */ ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [tip:x86/urgent] x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 2016-02-25 5:53 ` [tip:x86/urgent] " tip-bot for Andy Lutomirski @ 2016-02-25 6:00 ` H. Peter Anvin 2016-02-25 8:07 ` Andy Lutomirski [not found] ` <CALCETrWqCnhxvQ5qNp_O_7K7KW1H3FmHiX=mp+C5oeBEx=3YVA@mail.gmail.com> 0 siblings, 2 replies; 21+ messages in thread From: H. Peter Anvin @ 2016-02-25 6:00 UTC (permalink / raw) To: peterz, brgerst, mingo, tglx, torvalds, luto, linux-kernel, luto, bp, dvlasenk, linux-tip-commits On 02/24/16 21:53, tip-bot for Andy Lutomirski wrote: > Commit-ID: 04d1d281dcfe683a53cddfab8371fc8bb302b069 > Gitweb: http://git.kernel.org/tip/04d1d281dcfe683a53cddfab8371fc8bb302b069 > Author: Andy Lutomirski <luto@kernel.org> > AuthorDate: Tue, 23 Feb 2016 13:19:29 -0800 > Committer: Ingo Molnar <mingo@kernel.org> > CommitDate: Wed, 24 Feb 2016 08:43:04 +0100 > > x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 > > Both before and after 5f310f739b4c ("x86/entry/32: Re-implement > SYSENTER using the new C path"), we relied on a uaccess very early > in the SYSENTER path to clear AC. After that change, though, we can > potentially make it all the way into C code with AC set, which > enlarges the attack surface for SMAP bypass by doing SYSENTER with > AC set. > > Strengthen the SMAP protection by addding the missing ASM_CLAC right > at the beginning. > Hmmm... this potentially adds a *lot* of unnecessary cycles to this path. Could we reinstate the early uaccess? -hpa ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [tip:x86/urgent] x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 2016-02-25 6:00 ` H. Peter Anvin @ 2016-02-25 8:07 ` Andy Lutomirski 2016-02-25 8:11 ` Andy Lutomirski 2016-02-25 8:14 ` Ingo Molnar [not found] ` <CALCETrWqCnhxvQ5qNp_O_7K7KW1H3FmHiX=mp+C5oeBEx=3YVA@mail.gmail.com> 1 sibling, 2 replies; 21+ messages in thread From: Andy Lutomirski @ 2016-02-25 8:07 UTC (permalink / raw) To: H. Peter Anvin Cc: Peter Zijlstra, Brian Gerst, Ingo Molnar, Thomas Gleixner, Linus Torvalds, Andrew Lutomirski, linux-kernel, Borislav Petkov, Denys Vlasenko, linux-tip-commits [resend -- thank you Gmail for sucking] On Wed, Feb 24, 2016 at 10:00 PM, H. Peter Anvin <hpa@zytor.com> wrote: > On 02/24/16 21:53, tip-bot for Andy Lutomirski wrote: >> Commit-ID: 04d1d281dcfe683a53cddfab8371fc8bb302b069 >> Gitweb: http://git.kernel.org/tip/04d1d281dcfe683a53cddfab8371fc8bb302b069 >> Author: Andy Lutomirski <luto@kernel.org> >> AuthorDate: Tue, 23 Feb 2016 13:19:29 -0800 >> Committer: Ingo Molnar <mingo@kernel.org> >> CommitDate: Wed, 24 Feb 2016 08:43:04 +0100 >> >> x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 >> >> Both before and after 5f310f739b4c ("x86/entry/32: Re-implement >> SYSENTER using the new C path"), we relied on a uaccess very early >> in the SYSENTER path to clear AC. After that change, though, we can >> potentially make it all the way into C code with AC set, which >> enlarges the attack surface for SMAP bypass by doing SYSENTER with >> AC set. >> >> Strengthen the SMAP protection by addding the missing ASM_CLAC right >> at the beginning. >> > > Hmmm... this potentially adds a *lot* of unnecessary cycles to this > path. Could we reinstate the early uaccess? > I think that's more trouble than it's worth, and it'll undo a bunch of the context tracking cleanups that deferring it made possible, especially since this only matters in a configuration (32-bit SMAP) that no one uses. [1] *However*, I just realized that I have no idea why the 32-bit sysenter path is safe against NT being set. I fixed it on compat, and now I'm confused as to the status on 32-bit. If we need to fix up NT, I think we can fold AC into that. Also, I'll try to benchmark this soon. [1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=de9e478b9d49f3a0214310d921450cf5bb4a21e6 (it didn't even boot through most of 4.5-rc) ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [tip:x86/urgent] x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 2016-02-25 8:07 ` Andy Lutomirski @ 2016-02-25 8:11 ` Andy Lutomirski 2016-02-25 8:14 ` Ingo Molnar 1 sibling, 0 replies; 21+ messages in thread From: Andy Lutomirski @ 2016-02-25 8:11 UTC (permalink / raw) To: H. Peter Anvin Cc: Peter Zijlstra, Brian Gerst, Ingo Molnar, Thomas Gleixner, Linus Torvalds, Andrew Lutomirski, linux-kernel, Borislav Petkov, Denys Vlasenko, linux-tip-commits On Thu, Feb 25, 2016 at 12:07 AM, Andy Lutomirski <luto@amacapital.net> wrote: > [resend -- thank you Gmail for sucking] > > On Wed, Feb 24, 2016 at 10:00 PM, H. Peter Anvin <hpa@zytor.com> wrote: >> On 02/24/16 21:53, tip-bot for Andy Lutomirski wrote: >>> Commit-ID: 04d1d281dcfe683a53cddfab8371fc8bb302b069 >>> Gitweb: http://git.kernel.org/tip/04d1d281dcfe683a53cddfab8371fc8bb302b069 >>> Author: Andy Lutomirski <luto@kernel.org> >>> AuthorDate: Tue, 23 Feb 2016 13:19:29 -0800 >>> Committer: Ingo Molnar <mingo@kernel.org> >>> CommitDate: Wed, 24 Feb 2016 08:43:04 +0100 >>> >>> x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 >>> >>> Both before and after 5f310f739b4c ("x86/entry/32: Re-implement >>> SYSENTER using the new C path"), we relied on a uaccess very early >>> in the SYSENTER path to clear AC. After that change, though, we can >>> potentially make it all the way into C code with AC set, which >>> enlarges the attack surface for SMAP bypass by doing SYSENTER with >>> AC set. >>> >>> Strengthen the SMAP protection by addding the missing ASM_CLAC right >>> at the beginning. >>> >> >> Hmmm... this potentially adds a *lot* of unnecessary cycles to this >> path. Could we reinstate the early uaccess? >> > > I think that's more trouble than it's worth, and it'll undo a bunch of > the context tracking cleanups that deferring it made possible, > especially since this only matters in a configuration (32-bit SMAP) > that no one uses. [1] > > *However*, I just realized that I have no idea why the 32-bit sysenter > path is safe against NT being set. I fixed it on compat, and now I'm > confused as to the status on 32-bit. If we need to fix up NT, I think > we can fold AC into that. > It is, indeed, broken. My test case doesn't notice because opportunistic sysexit papers over the issue. Grump. > Also, I'll try to benchmark this soon. > > [1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=de9e478b9d49f3a0214310d921450cf5bb4a21e6 > (it didn't even boot through most of 4.5-rc) -- Andy Lutomirski AMA Capital Management, LLC ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [tip:x86/urgent] x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 2016-02-25 8:07 ` Andy Lutomirski 2016-02-25 8:11 ` Andy Lutomirski @ 2016-02-25 8:14 ` Ingo Molnar 2016-02-25 8:29 ` Mike Galbraith 1 sibling, 1 reply; 21+ messages in thread From: Ingo Molnar @ 2016-02-25 8:14 UTC (permalink / raw) To: Andy Lutomirski Cc: H. Peter Anvin, Peter Zijlstra, Brian Gerst, Thomas Gleixner, Linus Torvalds, Andrew Lutomirski, linux-kernel, Borislav Petkov, Denys Vlasenko, linux-tip-commits * Andy Lutomirski <luto@amacapital.net> wrote: > >> Strengthen the SMAP protection by addding the missing ASM_CLAC right at the > >> beginning. > > > > Hmmm... this potentially adds a *lot* of unnecessary cycles to this path. > > Could we reinstate the early uaccess? > > I think that's more trouble than it's worth, and it'll undo a bunch of the > context tracking cleanups that deferring it made possible, especially since this > only matters in a configuration (32-bit SMAP) that no one uses. [1] But but ... 'context tracking' is not really something that a regular distro kernel cares about much - it's a nohz-full special AFAICS. So if the only reason to keep this overhead is to simplify context tracking then I'm pretty sure we want to burden context-tracking with that, not the common fast path. Anyway, maybe we are 'lucky': > *However*, I just realized that I have no idea why the 32-bit sysenter > path is safe against NT being set. I fixed it on compat, and now I'm > confused as to the status on 32-bit. If we need to fix up NT, I think > we can fold AC into that. > > Also, I'll try to benchmark this soon. Sounds good, thanks! Ingo ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [tip:x86/urgent] x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 2016-02-25 8:14 ` Ingo Molnar @ 2016-02-25 8:29 ` Mike Galbraith 2016-02-25 8:40 ` Ingo Molnar 0 siblings, 1 reply; 21+ messages in thread From: Mike Galbraith @ 2016-02-25 8:29 UTC (permalink / raw) To: Ingo Molnar, Andy Lutomirski Cc: H. Peter Anvin, Peter Zijlstra, Brian Gerst, Thomas Gleixner, Linus Torvalds, Andrew Lutomirski, linux-kernel, Borislav Petkov, Denys Vlasenko, linux-tip-commits On Thu, 2016-02-25 at 09:14 +0100, Ingo Molnar wrote: > But but ... 'context tracking' is not really something that a regular distro > kernel cares about much - it's a nohz-full special AFAICS. (psst.. distros are shipping it) ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [tip:x86/urgent] x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 2016-02-25 8:29 ` Mike Galbraith @ 2016-02-25 8:40 ` Ingo Molnar 2016-02-25 9:08 ` Mike Galbraith 0 siblings, 1 reply; 21+ messages in thread From: Ingo Molnar @ 2016-02-25 8:40 UTC (permalink / raw) To: Mike Galbraith Cc: Andy Lutomirski, H. Peter Anvin, Peter Zijlstra, Brian Gerst, Thomas Gleixner, Linus Torvalds, Andrew Lutomirski, linux-kernel, Borislav Petkov, Denys Vlasenko, linux-tip-commits, Frédéric Weisbecker * Mike Galbraith <umgwanakikbuti@gmail.com> wrote: > On Thu, 2016-02-25 at 09:14 +0100, Ingo Molnar wrote: > > > But but ... 'context tracking' is not really something that a regular distro > > kernel cares about much - it's a nohz-full special AFAICS. Let me qualify that: with the timer code maintenance hat on I really love all nohz variants (the deeper the better), but now I have my x86 maintainer hat on, and as such I'm really annoyed at those nohz folks adding overhead to the syscall hot path! ;-) > (psst.. distros are shipping it) Yeah, indeed, Fedora does - but AFAICS: fomalhaut:~> grep NO_HZ /boot/config-4.1.13-100.fc21.x86_64 CONFIG_NO_HZ_COMMON=y # CONFIG_NO_HZ_IDLE is not set CONFIG_NO_HZ_FULL=y # CONFIG_NO_HZ_FULL_ALL is not set # CONFIG_NO_HZ_FULL_SYSIDLE is not set CONFIG_NO_HZ=y CONFIG_RCU_FAST_NO_HZ=y ... which won't result in actual full-nohz CPUs unless you boot it with a special boot parameter, right? What is the easiest way to query which/how many CPUs are in nohz-full mode and do context tracking? I somehow thought /proc/timer_* had that info, but that does not appear to be the case. Thanks, Ingo ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [tip:x86/urgent] x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 2016-02-25 8:40 ` Ingo Molnar @ 2016-02-25 9:08 ` Mike Galbraith 0 siblings, 0 replies; 21+ messages in thread From: Mike Galbraith @ 2016-02-25 9:08 UTC (permalink / raw) To: Ingo Molnar Cc: Andy Lutomirski, H. Peter Anvin, Peter Zijlstra, Brian Gerst, Thomas Gleixner, Linus Torvalds, Andrew Lutomirski, linux-kernel, Borislav Petkov, Denys Vlasenko, linux-tip-commits, Frédéric Weisbecker On Thu, 2016-02-25 at 09:40 +0100, Ingo Molnar wrote: > * Mike Galbraith <umgwanakikbuti@gmail.com> wrote: > > > On Thu, 2016-02-25 at 09:14 +0100, Ingo Molnar wrote: > > > > > But but ... 'context tracking' is not really something that a regular distro > > > kernel cares about much - it's a nohz-full special AFAICS. > > Let me qualify that: with the timer code maintenance hat on I really love all nohz > variants (the deeper the better), but now I have my x86 maintainer hat on, and as > such I'm really annoyed at those nohz folks adding overhead to the syscall hot > path! ;-) > > > (psst.. distros are shipping it) > > Yeah, indeed, Fedora does - but AFAICS: > > fomalhaut:~> grep NO_HZ /boot/config-4.1.13-100.fc21.x86_64 > CONFIG_NO_HZ_COMMON=y > # CONFIG_NO_HZ_IDLE is not set > CONFIG_NO_HZ_FULL=y > # CONFIG_NO_HZ_FULL_ALL is not set > # CONFIG_NO_HZ_FULL_SYSIDLE is not set > CONFIG_NO_HZ=y > CONFIG_RCU_FAST_NO_HZ=y > > ... which won't result in actual full-nohz CPUs unless you boot it with a special > boot parameter, right? Yeah, you have to manually enable it unless you (in a suicidal moment) enable CONFIG_NO_HZ_FULL_ALL. > What is the easiest way to query which/how many CPUs are in nohz-full mode and do > context tracking? I somehow thought /proc/timer_* had that info, but that does not > appear to be the case. /sys/devices/system/cpu/nohz_full -Mike ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <CALCETrWqCnhxvQ5qNp_O_7K7KW1H3FmHiX=mp+C5oeBEx=3YVA@mail.gmail.com>]
* Re: [tip:x86/urgent] x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 [not found] ` <CALCETrWqCnhxvQ5qNp_O_7K7KW1H3FmHiX=mp+C5oeBEx=3YVA@mail.gmail.com> @ 2016-02-25 13:47 ` Brian Gerst 2016-02-25 15:42 ` Brian Gerst 0 siblings, 1 reply; 21+ messages in thread From: Brian Gerst @ 2016-02-25 13:47 UTC (permalink / raw) To: Andy Lutomirski Cc: H. Peter Anvin, Linux Kernel Mailing List, linux-tip-commits, Thomas Gleixner, Borislav Petkov, Peter Zijlstra, Ingo Molnar, Linus Torvalds, Denys Vlasenko On Thu, Feb 25, 2016 at 3:03 AM, Andy Lutomirski <luto@amacapital.net> wrote: > > On Feb 24, 2016 10:01 PM, "H. Peter Anvin" <hpa@zytor.com> wrote: >> >> On 02/24/16 21:53, tip-bot for Andy Lutomirski wrote: >> > Commit-ID: 04d1d281dcfe683a53cddfab8371fc8bb302b069 >> > Gitweb: >> > http://git.kernel.org/tip/04d1d281dcfe683a53cddfab8371fc8bb302b069 >> > Author: Andy Lutomirski <luto@kernel.org> >> > AuthorDate: Tue, 23 Feb 2016 13:19:29 -0800 >> > Committer: Ingo Molnar <mingo@kernel.org> >> > CommitDate: Wed, 24 Feb 2016 08:43:04 +0100 >> > >> > x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 >> > >> > Both before and after 5f310f739b4c ("x86/entry/32: Re-implement >> > SYSENTER using the new C path"), we relied on a uaccess very early >> > in the SYSENTER path to clear AC. After that change, though, we can >> > potentially make it all the way into C code with AC set, which >> > enlarges the attack surface for SMAP bypass by doing SYSENTER with >> > AC set. >> > >> > Strengthen the SMAP protection by addding the missing ASM_CLAC right >> > at the beginning. >> > >> >> Hmmm... this potentially adds a *lot* of unnecessary cycles to this >> path. Could we reinstate the early uaccess? > > I think that's more trouble than it's worth, and it'll undo a bunch of the > context tracking cleanups that deferring it made possible, especially since > this only matters in a configuration (32-bit SMAP) that no one uses. [1] > > *However*, I just realized that I have no idea why the 32-bit sysenter path > is safe against NT being set. I fixed it on compat, and now I'm confused as > to the status on 32-bit. If we need to fix up NT, I think we can fold AC > into that. 32-bit still saves eflags in switch_to(), so NT can't leak to other tasks. But for consistency it should get the same treatment as 64-bit (clear NT in sysenter entry and drop saving eflags in switch_to). -- Brian Gerst ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [tip:x86/urgent] x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 2016-02-25 13:47 ` Brian Gerst @ 2016-02-25 15:42 ` Brian Gerst 2016-02-25 18:20 ` Andy Lutomirski 0 siblings, 1 reply; 21+ messages in thread From: Brian Gerst @ 2016-02-25 15:42 UTC (permalink / raw) To: Andy Lutomirski Cc: H. Peter Anvin, Linux Kernel Mailing List, linux-tip-commits, Thomas Gleixner, Borislav Petkov, Peter Zijlstra, Ingo Molnar, Linus Torvalds, Denys Vlasenko On Thu, Feb 25, 2016 at 8:47 AM, Brian Gerst <brgerst@gmail.com> wrote: > On Thu, Feb 25, 2016 at 3:03 AM, Andy Lutomirski <luto@amacapital.net> wrote: >> >> On Feb 24, 2016 10:01 PM, "H. Peter Anvin" <hpa@zytor.com> wrote: >>> >>> On 02/24/16 21:53, tip-bot for Andy Lutomirski wrote: >>> > Commit-ID: 04d1d281dcfe683a53cddfab8371fc8bb302b069 >>> > Gitweb: >>> > http://git.kernel.org/tip/04d1d281dcfe683a53cddfab8371fc8bb302b069 >>> > Author: Andy Lutomirski <luto@kernel.org> >>> > AuthorDate: Tue, 23 Feb 2016 13:19:29 -0800 >>> > Committer: Ingo Molnar <mingo@kernel.org> >>> > CommitDate: Wed, 24 Feb 2016 08:43:04 +0100 >>> > >>> > x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 >>> > >>> > Both before and after 5f310f739b4c ("x86/entry/32: Re-implement >>> > SYSENTER using the new C path"), we relied on a uaccess very early >>> > in the SYSENTER path to clear AC. After that change, though, we can >>> > potentially make it all the way into C code with AC set, which >>> > enlarges the attack surface for SMAP bypass by doing SYSENTER with >>> > AC set. >>> > >>> > Strengthen the SMAP protection by addding the missing ASM_CLAC right >>> > at the beginning. >>> > >>> >>> Hmmm... this potentially adds a *lot* of unnecessary cycles to this >>> path. Could we reinstate the early uaccess? >> >> I think that's more trouble than it's worth, and it'll undo a bunch of the >> context tracking cleanups that deferring it made possible, especially since >> this only matters in a configuration (32-bit SMAP) that no one uses. [1] >> >> *However*, I just realized that I have no idea why the 32-bit sysenter path >> is safe against NT being set. I fixed it on compat, and now I'm confused as >> to the status on 32-bit. If we need to fix up NT, I think we can fold AC >> into that. > > 32-bit still saves eflags in switch_to(), so NT can't leak to other > tasks. But for consistency it should get the same treatment as 64-bit > (clear NT in sysenter entry and drop saving eflags in switch_to). Looking deeper, the sysexit path doesn't restore eflags, so we'd need to manually restore AC and IOPL. -- Brian Gerst ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [tip:x86/urgent] x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 2016-02-25 15:42 ` Brian Gerst @ 2016-02-25 18:20 ` Andy Lutomirski 2016-02-25 18:30 ` Linus Torvalds 0 siblings, 1 reply; 21+ messages in thread From: Andy Lutomirski @ 2016-02-25 18:20 UTC (permalink / raw) To: Brian Gerst Cc: H. Peter Anvin, Linux Kernel Mailing List, linux-tip-commits, Thomas Gleixner, Borislav Petkov, Peter Zijlstra, Ingo Molnar, Linus Torvalds, Denys Vlasenko On Thu, Feb 25, 2016 at 7:42 AM, Brian Gerst <brgerst@gmail.com> wrote: > On Thu, Feb 25, 2016 at 8:47 AM, Brian Gerst <brgerst@gmail.com> wrote: >> On Thu, Feb 25, 2016 at 3:03 AM, Andy Lutomirski <luto@amacapital.net> wrote: >>> >>> On Feb 24, 2016 10:01 PM, "H. Peter Anvin" <hpa@zytor.com> wrote: >>>> >>>> On 02/24/16 21:53, tip-bot for Andy Lutomirski wrote: >>>> > Commit-ID: 04d1d281dcfe683a53cddfab8371fc8bb302b069 >>>> > Gitweb: >>>> > http://git.kernel.org/tip/04d1d281dcfe683a53cddfab8371fc8bb302b069 >>>> > Author: Andy Lutomirski <luto@kernel.org> >>>> > AuthorDate: Tue, 23 Feb 2016 13:19:29 -0800 >>>> > Committer: Ingo Molnar <mingo@kernel.org> >>>> > CommitDate: Wed, 24 Feb 2016 08:43:04 +0100 >>>> > >>>> > x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 >>>> > >>>> > Both before and after 5f310f739b4c ("x86/entry/32: Re-implement >>>> > SYSENTER using the new C path"), we relied on a uaccess very early >>>> > in the SYSENTER path to clear AC. After that change, though, we can >>>> > potentially make it all the way into C code with AC set, which >>>> > enlarges the attack surface for SMAP bypass by doing SYSENTER with >>>> > AC set. >>>> > >>>> > Strengthen the SMAP protection by addding the missing ASM_CLAC right >>>> > at the beginning. >>>> > >>>> >>>> Hmmm... this potentially adds a *lot* of unnecessary cycles to this >>>> path. Could we reinstate the early uaccess? >>> >>> I think that's more trouble than it's worth, and it'll undo a bunch of the >>> context tracking cleanups that deferring it made possible, especially since >>> this only matters in a configuration (32-bit SMAP) that no one uses. [1] >>> >>> *However*, I just realized that I have no idea why the 32-bit sysenter path >>> is safe against NT being set. I fixed it on compat, and now I'm confused as >>> to the status on 32-bit. If we need to fix up NT, I think we can fold AC >>> into that. >> >> 32-bit still saves eflags in switch_to(), so NT can't leak to other >> tasks. But for consistency it should get the same treatment as 64-bit >> (clear NT in sysenter entry and drop saving eflags in switch_to). > > Looking deeper, the sysexit path doesn't restore eflags, so we'd need > to manually restore AC and IOPL. Ick. First things first, though -- there's no immediate reason to stop saving flags on 32-bit context switches. So we don't need to worry about IOPL except for sanity and/or consistency. Ideally we'd fix this up and restore flags on sysexit. At least failing to restore arithmetic flags isn't an info leak because the exit code clobbers them with entirely predictable data. I doubt anyone cares all that much if we clobber AC. I wrote a test for NT and the test fails for a different reason: our TF handling appears broken as well. (Our sysenter TF handling is *crap*, but it seems to work on 64-bit kernels at least.) My personal preference would be to add the missing popf. --Andy ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [tip:x86/urgent] x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 2016-02-25 18:20 ` Andy Lutomirski @ 2016-02-25 18:30 ` Linus Torvalds 2016-02-25 18:40 ` Andy Lutomirski 2016-02-25 19:31 ` Brian Gerst 0 siblings, 2 replies; 21+ messages in thread From: Linus Torvalds @ 2016-02-25 18:30 UTC (permalink / raw) To: Andy Lutomirski Cc: Brian Gerst, H. Peter Anvin, Linux Kernel Mailing List, linux-tip-commits, Thomas Gleixner, Borislav Petkov, Peter Zijlstra, Ingo Molnar, Denys Vlasenko On Thu, Feb 25, 2016 at 10:20 AM, Andy Lutomirski <luto@amacapital.net> wrote: > > Ideally we'd fix this up and restore flags on sysexit. At least > failing to restore arithmetic flags isn't an info leak because the > exit code clobbers them with entirely predictable data. I doubt > anyone cares all that much if we clobber AC. As long as the "clobber AC" is purely about clearing it, it's probably fine. Although there may be programs that set AC in order to actually get notified about alignment issues (perhaps for portability reasons, perhaps for small performance reasons). Clearing it will make those programs still work, but they lose the checking. > I wrote a test for NT and the test fails for a different reason: our > TF handling appears broken as well. (Our sysenter TF handling is > *crap*, but it seems to work on 64-bit kernels at least.) TF should be entirely immaterial for system calls. Why would we care? We need it for correct handling of real traps, but not for the system call case afaik. Returning with TF clear is the right thing, since we're not returning *to* the system call instruction, but the instruction after. > My personal preference would be to add the missing popf. I don't mind adding the popf, but it won't help for iopl. Only iret restores iopl, if I recall correctly (but maybe I don't, and I'm too lazy to take the 30 seconds to look it up). Linus ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [tip:x86/urgent] x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 2016-02-25 18:30 ` Linus Torvalds @ 2016-02-25 18:40 ` Andy Lutomirski 2016-02-25 19:31 ` Brian Gerst 1 sibling, 0 replies; 21+ messages in thread From: Andy Lutomirski @ 2016-02-25 18:40 UTC (permalink / raw) To: Linus Torvalds Cc: Brian Gerst, H. Peter Anvin, Linux Kernel Mailing List, linux-tip-commits, Thomas Gleixner, Borislav Petkov, Peter Zijlstra, Ingo Molnar, Denys Vlasenko On Thu, Feb 25, 2016 at 10:30 AM, Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Thu, Feb 25, 2016 at 10:20 AM, Andy Lutomirski <luto@amacapital.net> wrote: >> >> Ideally we'd fix this up and restore flags on sysexit. At least >> failing to restore arithmetic flags isn't an info leak because the >> exit code clobbers them with entirely predictable data. I doubt >> anyone cares all that much if we clobber AC. > > As long as the "clobber AC" is purely about clearing it, it's probably fine. > > Although there may be programs that set AC in order to actually get > notified about alignment issues (perhaps for portability reasons, > perhaps for small performance reasons). Clearing it will make those > programs still work, but they lose the checking. > >> I wrote a test for NT and the test fails for a different reason: our >> TF handling appears broken as well. (Our sysenter TF handling is >> *crap*, but it seems to work on 64-bit kernels at least.) > > TF should be entirely immaterial for system calls. Why would we care? > We need it for correct handling of real traps, but not for the system > call case afaik. Returning with TF clear is the right thing, since > we're not returning *to* the system call instruction, but the > instruction after. TF is very material to SYSENTER because Intel completely fucked up. SYSENTER with TF set causes SYSENTER to trap in the sense that an interrupt is delivered after SYSENTER, *in kernel mode*, *from CPL 0*, with whatever probably-bullshit stack pointer we have set up. We have overcomplicated code to fix up the resulting mess, but it doesn't seem to work right on 32-bit. So I'm thinking of rewriting it from scratch to make sense. > >> My personal preference would be to add the missing popf. > > I don't mind adding the popf, but it won't help for iopl. Only iret > restores iopl, if I recall correctly (but maybe I don't, and I'm too > lazy to take the 30 seconds to look it up). > OK, I'll make sure to check this. --Andy ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [tip:x86/urgent] x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 2016-02-25 18:30 ` Linus Torvalds 2016-02-25 18:40 ` Andy Lutomirski @ 2016-02-25 19:31 ` Brian Gerst 2016-02-25 19:39 ` Andy Lutomirski 2016-02-25 20:54 ` Linus Torvalds 1 sibling, 2 replies; 21+ messages in thread From: Brian Gerst @ 2016-02-25 19:31 UTC (permalink / raw) To: Linus Torvalds Cc: Andy Lutomirski, H. Peter Anvin, Linux Kernel Mailing List, linux-tip-commits, Thomas Gleixner, Borislav Petkov, Peter Zijlstra, Ingo Molnar, Denys Vlasenko On Thu, Feb 25, 2016 at 1:30 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Thu, Feb 25, 2016 at 10:20 AM, Andy Lutomirski <luto@amacapital.net> wrote: >> >> Ideally we'd fix this up and restore flags on sysexit. At least >> failing to restore arithmetic flags isn't an info leak because the >> exit code clobbers them with entirely predictable data. I doubt >> anyone cares all that much if we clobber AC. > > As long as the "clobber AC" is purely about clearing it, it's probably fine. > > Although there may be programs that set AC in order to actually get > notified about alignment issues (perhaps for portability reasons, > perhaps for small performance reasons). Clearing it will make those > programs still work, but they lose the checking. > >> I wrote a test for NT and the test fails for a different reason: our >> TF handling appears broken as well. (Our sysenter TF handling is >> *crap*, but it seems to work on 64-bit kernels at least.) > > TF should be entirely immaterial for system calls. Why would we care? > We need it for correct handling of real traps, but not for the system > call case afaik. Returning with TF clear is the right thing, since > we're not returning *to* the system call instruction, but the > instruction after. > >> My personal preference would be to add the missing popf. > > I don't mind adding the popf, but it won't help for iopl. Only iret > restores iopl, if I recall correctly (but maybe I don't, and I'm too > lazy to take the 30 seconds to look it up). > > Linus According to the SDM, popf will change IOPL only at CPL0, which is why Xen (which runs at CPL1 on 32-bit) has a paravirt hook for it. -- Brian Gerst ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [tip:x86/urgent] x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 2016-02-25 19:31 ` Brian Gerst @ 2016-02-25 19:39 ` Andy Lutomirski 2016-02-25 19:49 ` Brian Gerst 2016-02-25 20:54 ` Linus Torvalds 1 sibling, 1 reply; 21+ messages in thread From: Andy Lutomirski @ 2016-02-25 19:39 UTC (permalink / raw) To: Brian Gerst Cc: Linus Torvalds, H. Peter Anvin, Linux Kernel Mailing List, linux-tip-commits, Thomas Gleixner, Borislav Petkov, Peter Zijlstra, Ingo Molnar, Denys Vlasenko On Thu, Feb 25, 2016 at 11:31 AM, Brian Gerst <brgerst@gmail.com> wrote: > On Thu, Feb 25, 2016 at 1:30 PM, Linus Torvalds > <torvalds@linux-foundation.org> wrote: >> On Thu, Feb 25, 2016 at 10:20 AM, Andy Lutomirski <luto@amacapital.net> wrote: >>> >>> Ideally we'd fix this up and restore flags on sysexit. At least >>> failing to restore arithmetic flags isn't an info leak because the >>> exit code clobbers them with entirely predictable data. I doubt >>> anyone cares all that much if we clobber AC. >> >> As long as the "clobber AC" is purely about clearing it, it's probably fine. >> >> Although there may be programs that set AC in order to actually get >> notified about alignment issues (perhaps for portability reasons, >> perhaps for small performance reasons). Clearing it will make those >> programs still work, but they lose the checking. >> >>> I wrote a test for NT and the test fails for a different reason: our >>> TF handling appears broken as well. (Our sysenter TF handling is >>> *crap*, but it seems to work on 64-bit kernels at least.) >> >> TF should be entirely immaterial for system calls. Why would we care? >> We need it for correct handling of real traps, but not for the system >> call case afaik. Returning with TF clear is the right thing, since >> we're not returning *to* the system call instruction, but the >> instruction after. >> >>> My personal preference would be to add the missing popf. >> >> I don't mind adding the popf, but it won't help for iopl. Only iret >> restores iopl, if I recall correctly (but maybe I don't, and I'm too >> lazy to take the 30 seconds to look it up). >> >> Linus > > According to the SDM, popf will change IOPL only at CPL0, which is why > Xen (which runs at CPL1 on 32-bit) has a paravirt hook for it. But maybe we can ditch that paravirt hook and just modify regs->flags in sys_iopl. Xen never uses sysexit at all: /* XEN PV guests always use IRET path */ ALTERNATIVE "testl %eax, %eax; jz .Lsyscall_32_done", \ "jmp .Lsyscall_32_done", X86_FEATURE_XENPV and if we add the missing popf, we should be good to go. --Andy ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [tip:x86/urgent] x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 2016-02-25 19:39 ` Andy Lutomirski @ 2016-02-25 19:49 ` Brian Gerst 2016-02-25 19:52 ` Andy Lutomirski 0 siblings, 1 reply; 21+ messages in thread From: Brian Gerst @ 2016-02-25 19:49 UTC (permalink / raw) To: Andy Lutomirski Cc: Linus Torvalds, H. Peter Anvin, Linux Kernel Mailing List, linux-tip-commits, Thomas Gleixner, Borislav Petkov, Peter Zijlstra, Ingo Molnar, Denys Vlasenko On Thu, Feb 25, 2016 at 2:39 PM, Andy Lutomirski <luto@amacapital.net> wrote: > On Thu, Feb 25, 2016 at 11:31 AM, Brian Gerst <brgerst@gmail.com> wrote: >> On Thu, Feb 25, 2016 at 1:30 PM, Linus Torvalds >> <torvalds@linux-foundation.org> wrote: >>> On Thu, Feb 25, 2016 at 10:20 AM, Andy Lutomirski <luto@amacapital.net> wrote: >>>> >>>> Ideally we'd fix this up and restore flags on sysexit. At least >>>> failing to restore arithmetic flags isn't an info leak because the >>>> exit code clobbers them with entirely predictable data. I doubt >>>> anyone cares all that much if we clobber AC. >>> >>> As long as the "clobber AC" is purely about clearing it, it's probably fine. >>> >>> Although there may be programs that set AC in order to actually get >>> notified about alignment issues (perhaps for portability reasons, >>> perhaps for small performance reasons). Clearing it will make those >>> programs still work, but they lose the checking. >>> >>>> I wrote a test for NT and the test fails for a different reason: our >>>> TF handling appears broken as well. (Our sysenter TF handling is >>>> *crap*, but it seems to work on 64-bit kernels at least.) >>> >>> TF should be entirely immaterial for system calls. Why would we care? >>> We need it for correct handling of real traps, but not for the system >>> call case afaik. Returning with TF clear is the right thing, since >>> we're not returning *to* the system call instruction, but the >>> instruction after. >>> >>>> My personal preference would be to add the missing popf. >>> >>> I don't mind adding the popf, but it won't help for iopl. Only iret >>> restores iopl, if I recall correctly (but maybe I don't, and I'm too >>> lazy to take the 30 seconds to look it up). >>> >>> Linus >> >> According to the SDM, popf will change IOPL only at CPL0, which is why >> Xen (which runs at CPL1 on 32-bit) has a paravirt hook for it. > > But maybe we can ditch that paravirt hook and just modify regs->flags > in sys_iopl. Xen never uses sysexit at all: > > /* XEN PV guests always use IRET path */ > ALTERNATIVE "testl %eax, %eax; jz .Lsyscall_32_done", \ > "jmp .Lsyscall_32_done", X86_FEATURE_XENPV > > and if we add the missing popf, we should be good to go. IRET won't change IOPL either at CPL != 0, so Xen still needs that hook. -- Brian Gerst ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [tip:x86/urgent] x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 2016-02-25 19:49 ` Brian Gerst @ 2016-02-25 19:52 ` Andy Lutomirski 0 siblings, 0 replies; 21+ messages in thread From: Andy Lutomirski @ 2016-02-25 19:52 UTC (permalink / raw) To: Brian Gerst Cc: Linus Torvalds, H. Peter Anvin, Linux Kernel Mailing List, linux-tip-commits, Thomas Gleixner, Borislav Petkov, Peter Zijlstra, Ingo Molnar, Denys Vlasenko On Thu, Feb 25, 2016 at 11:49 AM, Brian Gerst <brgerst@gmail.com> wrote: > On Thu, Feb 25, 2016 at 2:39 PM, Andy Lutomirski <luto@amacapital.net> wrote: >> On Thu, Feb 25, 2016 at 11:31 AM, Brian Gerst <brgerst@gmail.com> wrote: >>> On Thu, Feb 25, 2016 at 1:30 PM, Linus Torvalds >>> <torvalds@linux-foundation.org> wrote: >>>> On Thu, Feb 25, 2016 at 10:20 AM, Andy Lutomirski <luto@amacapital.net> wrote: >>>>> >>>>> Ideally we'd fix this up and restore flags on sysexit. At least >>>>> failing to restore arithmetic flags isn't an info leak because the >>>>> exit code clobbers them with entirely predictable data. I doubt >>>>> anyone cares all that much if we clobber AC. >>>> >>>> As long as the "clobber AC" is purely about clearing it, it's probably fine. >>>> >>>> Although there may be programs that set AC in order to actually get >>>> notified about alignment issues (perhaps for portability reasons, >>>> perhaps for small performance reasons). Clearing it will make those >>>> programs still work, but they lose the checking. >>>> >>>>> I wrote a test for NT and the test fails for a different reason: our >>>>> TF handling appears broken as well. (Our sysenter TF handling is >>>>> *crap*, but it seems to work on 64-bit kernels at least.) >>>> >>>> TF should be entirely immaterial for system calls. Why would we care? >>>> We need it for correct handling of real traps, but not for the system >>>> call case afaik. Returning with TF clear is the right thing, since >>>> we're not returning *to* the system call instruction, but the >>>> instruction after. >>>> >>>>> My personal preference would be to add the missing popf. >>>> >>>> I don't mind adding the popf, but it won't help for iopl. Only iret >>>> restores iopl, if I recall correctly (but maybe I don't, and I'm too >>>> lazy to take the 30 seconds to look it up). >>>> >>>> Linus >>> >>> According to the SDM, popf will change IOPL only at CPL0, which is why >>> Xen (which runs at CPL1 on 32-bit) has a paravirt hook for it. >> >> But maybe we can ditch that paravirt hook and just modify regs->flags >> in sys_iopl. Xen never uses sysexit at all: >> >> /* XEN PV guests always use IRET path */ >> ALTERNATIVE "testl %eax, %eax; jz .Lsyscall_32_done", \ >> "jmp .Lsyscall_32_done", X86_FEATURE_XENPV >> >> and if we add the missing popf, we should be good to go. > > IRET won't change IOPL either at CPL != 0, so Xen still needs that hook. But xen_iret isn't the same thing as IRET at all. We don't use a real IRET instruction to switch between kernel and user mode on Xen PV. --Andy > > -- > Brian Gerst -- Andy Lutomirski AMA Capital Management, LLC ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [tip:x86/urgent] x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 2016-02-25 19:31 ` Brian Gerst 2016-02-25 19:39 ` Andy Lutomirski @ 2016-02-25 20:54 ` Linus Torvalds 1 sibling, 0 replies; 21+ messages in thread From: Linus Torvalds @ 2016-02-25 20:54 UTC (permalink / raw) To: Brian Gerst Cc: Andy Lutomirski, H. Peter Anvin, Linux Kernel Mailing List, linux-tip-commits, Thomas Gleixner, Borislav Petkov, Peter Zijlstra, Ingo Molnar, Denys Vlasenko On Thu, Feb 25, 2016 at 11:31 AM, Brian Gerst <brgerst@gmail.com> wrote: > > According to the SDM, popf will change IOPL only at CPL0, Ok, so that's good enough for the native case, and adding a "popf" would at least restore AC and IOPL. Linus ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2016-02-25 20:54 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-23 21:19 [PATCH] x86/entry/32: Add an ASM_CLAC to entry_SYSENTER_32 Andy Lutomirski
2016-02-24 15:46 ` Brian Gerst
2016-02-24 16:56 ` Andy Lutomirski
2016-02-25 5:53 ` [tip:x86/urgent] " tip-bot for Andy Lutomirski
2016-02-25 6:00 ` H. Peter Anvin
2016-02-25 8:07 ` Andy Lutomirski
2016-02-25 8:11 ` Andy Lutomirski
2016-02-25 8:14 ` Ingo Molnar
2016-02-25 8:29 ` Mike Galbraith
2016-02-25 8:40 ` Ingo Molnar
2016-02-25 9:08 ` Mike Galbraith
[not found] ` <CALCETrWqCnhxvQ5qNp_O_7K7KW1H3FmHiX=mp+C5oeBEx=3YVA@mail.gmail.com>
2016-02-25 13:47 ` Brian Gerst
2016-02-25 15:42 ` Brian Gerst
2016-02-25 18:20 ` Andy Lutomirski
2016-02-25 18:30 ` Linus Torvalds
2016-02-25 18:40 ` Andy Lutomirski
2016-02-25 19:31 ` Brian Gerst
2016-02-25 19:39 ` Andy Lutomirski
2016-02-25 19:49 ` Brian Gerst
2016-02-25 19:52 ` Andy Lutomirski
2016-02-25 20:54 ` Linus Torvalds
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.