x86: entry: flush the cache if syscall error
Subsystems (2)
PATCH 1/1 x86: entry: flush the cache if syscall error
This patch aims to make it harder to perform cache timing attacks on data left behind by system calls. If we have an error returned from a syscall, flush the L1 cache.
It's important to note that this patch is not addressing any specific exploit, nor is it intended to be a complete defense against anything. It is intended to be a low cost way of eliminating some of side effects of a failed system call.
A performance test using sysbench on one hyperthread and a script which attempts to repeatedly access files it does not have permission to access on the other hyperthread found no significant performance impact.
--- arch/x86/Kconfig | 9 +++++++++ arch/x86/entry/common.c | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+)
arch/x86/Kconfig +9 -0
@@ -445,6 +445,15 @@ config RETPOLINE code are eliminated. Since this includes the syscall entry path, it is not entirely pointless. +config SYSCALL_FLUSH + bool "Clear L1 Cache on syscall errors" + default n + help + Selecting 'y' allows the L1 cache to be cleared upon return of + an error code from a syscall if the CPU supports "flush_l1d". + This may reduce the likelyhood of speculative execution style + attacks on syscalls. + config INTEL_RDT bool "Intel Resource Director Technology support" default n
arch/x86/entry/common.c +18 -0
@@ -268,6 +268,20 @@ __visible inline void syscall_return_slowpath(struct pt_regs *regs)prepare_exit_to_usermode (regs); } +__visible inline void l1_cache_flush(struct pt_regs *regs) +{ +if (IS_ENABLED (CONFIG_SYSCALL_FLUSH ) && +static_cpu_has (X86_FEATURE_FLUSH_L1D )) { +if (regs->ax ==0 || regs->ax == -EAGAIN || + regs->ax == -EEXIST || regs->ax == -ENOENT || + regs->ax == -EXDEV || regs->ax == -ETIMEDOUT || + regs->ax == -ENOTCONN || regs->ax == -EINPROGRESS ) +return ; + +wrmsrl (MSR_IA32_FLUSH_CMD ,L1D_FLUSH ); + } +} +#ifdef CONFIG_X86_64 __visible void do_syscall_64(unsigned long nr, struct pt_regs *regs) {
@@ -290,6 +304,8 @@ __visible void do_syscall_64(unsigned long nr, struct pt_regs *regs) regs->ax = sys_call_table[nr](regs); } +l1_cache_flush (regs); +syscall_return_slowpath (regs); }#endif
@@ -338,6 +354,8 @@ static __always_inline void do_syscall_32_irqs_on(struct pt_regs *regs)#endif /* CONFIG_IA32_EMULATION */ } +l1_cache_flush (regs); +syscall_return_slowpath (regs); }
Signature · 1 line
2.14.4
Andy Lutomirski
maintainerQuoted 18 lines · Kristen Carlson Accardi
This patch aims to make it harder to perform cache timing attacks on data left behind by system calls. If we have an error returned from a syscall, flush the L1 cache.
It's important to note that this patch is not addressing any specific exploit, nor is it intended to be a complete defense against anything. It is intended to be a low cost way of eliminating some of side effects of a failed system call.
A performance test using sysbench on one hyperthread and a script which attempts to repeatedly access files it does not have permission to access on the other hyperthread found no significant performance impact.
arch/x86/entry/common.c +3
-0+__visible inline void l1_cache_flush(struct pt_regs *regs) +{ + if (IS_ENABLED(CONFIG_SYSCALL_FLUSH) &&
arch/x86/entry/common.c __visible inline void syscall_return_slowpath(struct pt_regs *regs) · line 283
Quoted 10 lines
arch/x86/entry/common.c +10
-0+ static_cpu_has(X86_FEATURE_FLUSH_L1D)) { +if (regs->ax ==0 || regs->ax == -EAGAIN || + regs->ax == -EEXIST || regs->ax == -ENOENT || + regs->ax == -EXDEV || regs->ax == -ETIMEDOUT || + regs->ax == -ENOTCONN || regs->ax == -EINPROGRESS ) +return ; + +wrmsrl (MSR_IA32_FLUSH_CMD ,L1D_FLUSH ); + } +}
Ugh.
What exactly is this trying to protect against? And how many cycles should we expect L1D_FLUSH to take?
ISTM that, if we have a situation where the L1D can be read by user code, we lose, via hyperthreading, successful syscalls, /dev/random, and may other vectors. This seems like a small mitigation at a rather large cost.
Quoted 40 lines · Andy Lutomirski
Quoted 25 lines · Kristen Carlson Accardi
This patch aims to make it harder to perform cache timing attacks on data
left behind by system calls. If we have an error returned from a syscall,
flush the L1 cache.It's important to note that this patch is not addressing any specific
exploit, nor is it intended to be a complete defense against anything.
It is intended to be a low cost way of eliminating some of side effects
of a failed system call.A performance test using sysbench on one hyperthread and a script which
attempts to repeatedly access files it does not have permission to access
on the other hyperthread found no significant performance impact.arch/x86/entry/common.c +4
-0+__visible inline void l1_cache_flush(struct pt_regs *regs) +{ +if (IS_ENABLED (CONFIG_SYSCALL_FLUSH ) && +static_cpu_has (X86_FEATURE_FLUSH_L1D )) {arch/x86/entry/common.c __visible inline void syscall_return_slowpath(struct pt_regs *regs) · line 283
Quoted 10 lines
arch/x86/entry/common.c +9
-1+if (regs->ax ==0 || regs->ax == -EAGAIN || + regs->ax == -EEXIST || regs->ax == -ENOENT || + regs->ax == -EXDEV || regs->ax == -ETIMEDOUT || + regs->ax == -ENOTCONN || regs->ax == -EINPROGRESS) + return; + +wrmsrl (MSR_IA32_FLUSH_CMD ,L1D_FLUSH ); + } +}Ugh.
Quoted 2 lines
What exactly is this trying to protect against? And how many cycles should we expect
L1D_FLUSHto take?
As I mentioned in the commit message, this is not addressing any specific exploit. It is removing any side effects from a failed system call in the L1 cache.
Quoted 6 lines
ISTM that, if we have a situation where the L1D can be read by user code, we lose, via hyperthreading, successful syscalls,
/dev/random, and may other vectors. This seems like a small mitigation at a rather
large cost.
I pinned an evil task to one hyperthread that just caused L1 flushes by issuing failed system calls. On the other hyperthread, I ran a performance benchmark (sysbench). I did not see any difference between the baseline and the kernel with the patch applied. Is there a more appropriate test you'd be interested in seeing the results of? I'd be happy to design a different test.
Quoted 3 lines
Ugh.
What exactly is this trying to protect against? And how many cycles
Most attacks by speculation rely upon leaving footprints in the L1 cache. They also almost inevitably resolve non speculatively to errors. If you look through all the 'yet another potential spectre case' patches people have found they would have been rendered close to useless by this change.
It's a way to deal with the ones we don't know about, all the ones the tools won't find and it has pretty much zero cost
(If you are bored strace an entire days desktop session, bang it through a script or two to extract the number of triggerig error returns and do the maths...)
Quoted 1 line
should we expect
L1D_FLUSHto take?
More to the point you pretty much never trigger it. Errors are not the normal path in real code. The original version of this code emptied the L1 the hard way - and even then it was in the noise for real workloads we tried.
You can argue that the other thread could be some evil task that deliberately triggers flushes, but it can already thrash the L1 on processors that share L1 between threads using perfectly normal memory instructions.
Alan
Andy Lutomirski
maintainerQuoted 9 lines · Alan Cox
Quoted 4 lines
Ugh.
What exactly is this trying to protect against? And how many cycles
Most attacks by speculation rely upon leaving footprints in the L1 cache. They also almost inevitably resolve non speculatively to errors. If you look through all the 'yet another potential spectre case' patches people have found they would have been rendered close to useless by this change.
Can you give an example? AFAIK the interesting Meltdown-like attacks are unaffected because Meltdown doesn't actually need the target data to be in L1D. And most of the Spectre-style attacks would have been blocked by doing LFENCE on the error cases (and somehow making sure that the CPU doesn't speculate around the LFENCE without noticing it). But this patch is doing an L1D flush, which, as far as I've heard, isn't actually relevant.
Quoted 15 lines
It's a way to deal with the ones we don't know about, all the ones theion tools won't find and it has pretty much zero cost
(If you are bored strace an entire days desktop session, bang it through a script or two to extract the number of triggerig error returns and do the maths...)
Quoted 1 line
should we expect
L1D_FLUSHto take?More to the point you pretty much never trigger it. Errors are not the normal path in real code. The original version of this code emptied the L1 the hard way - and even then it was in the noise for real workloads we tried.
Quoted 5 lines
You can argue that the other thread could be some evil task that deliberately triggers flushes, but it can already thrash the L1 on processors that share L1 between threads using perfectly normal memory instructions.
That's not what I meant. I meant that, if an attacker can run code on *both* logical threads on the same CPU, then they can run their attack code on the other logical thread before the L1D_FLUSH command takes effect.
I care about the performance of single-threaded workloads, though. How slow is this thing? No one cares about syscall performance on
regular desktop sessions except for gaming. But people do care about
syscall performance on all kinds of crazy server, database, etc workloads. And compilation. And HPC stuff, although that mostly doesn't involve syscalls. So: benchmarks, please. And estimated cycle counts, please, on at least a couple of relevant CPU generations.
On Meltdown-affected CPUs, we're doing a CR3 write anyway, which is fully serializing, so it's slow. But AFAIK that *already* blocks most of these attacks except L1TF, and L1TF has (hopefully!) been fixed anyway on Linux.
Quoted 2 lines · Andy Lutomirski
What exactly is this trying to protect against? And how many cycles should we expect
L1D_FLUSHto take?
As far as I could measure, I got 1660 cycles per wrmsr 0x10b, 0x1 on a Skylake chip, and 1220 cycles on a Skylake-SP.
Quoted 7 lines · Samuel Neves
Quoted 2 lines · Andy Lutomirski
What exactly is this trying to protect against? And how many cycles should we expect
L1D_FLUSHto take?As far as I could measure, I got 1660 cycles per wrmsr
0x10b,0x1on a Skylake chip, and 1220 cycles on a Skylake-SP.
Is that with L1D mostly empty, with L1D mostly full with clean lines, or with L1D full of dirty lines that need to be written back?
Quoted 4 lines · Jann Horn
Quoted 2 lines · Samuel Neves
On Thu, Oct 11, 2018 at 8:25 PM Andy Lutomirski <[email protected]> wrote:
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
What exactly is this trying to protect against? And how many cycles should we expect
L1D_FLUSHto take?As far as I could measure, I got 1660 cycles per wrmsr
0x10b,0x1on a Skylake chip, and 1220 cycles on a Skylake-SP.Is that with L1D mostly empty, with L1D mostly full with clean lines, or with L1D full of dirty lines that need to be written back?
Mostly empty, as this is flushing repeatedly without bothering to refill L1d with anything.
On Skylake the (averaged) uops breakdown is something like
port 0: 255
port 1: 143
port 2: 176
port 3: 177
port 4: 524
port 5: 273
port 6: 616
port 7: 182
The number of port 4 dispatches is very close to the number of cache lines, suggesting one write per line (with respective 176+177+182 port {2, 3, 7} address generations).
Furthermore, I suspect it also clears L1i cache. For 2^20 wrmsr executions, we have around 2^20 frontend_retired_l1i_miss events, but a negligible amount of frontend_retired_l2_miss ones.
Andy Lutomirski
maintainerQuoted 47 lines · Kristen Carlson Accardi
This patch aims to make it harder to perform cache timing attacks on data left behind by system calls. If we have an error returned from a syscall, flush the L1 cache.
It's important to note that this patch is not addressing any specific exploit, nor is it intended to be a complete defense against anything. It is intended to be a low cost way of eliminating some of side effects of a failed system call.
A performance test using sysbench on one hyperthread and a script which attempts to repeatedly access files it does not have permission to access on the other hyperthread found no significant performance impact.
Suggested-by Alan Cox <[email protected]>Signed-off-by Kristen Carlson Accardi <[email protected]>--- arch/x86/Kconfig | 9 +++++++++ arch/x86/entry/common.c | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+)arch/x86/Kconfig +9
-0@@ -445,6 +445,15 @@ config RETPOLINE code are eliminated. Since this includes the syscall entry path, it is not entirely pointless. +config SYSCALL_FLUSH + bool "Clear L1 Cache on syscall errors" + default n + help + Selecting 'y' allows the L1 cache to be cleared upon return of + an error code from a syscall if the CPU supports "flush_l1d". + This may reduce the likelyhood of speculative execution style + attacks on syscalls. + config INTEL_RDT bool "Intel Resource Director Technology support" default narch/x86/entry/common.c +0
-0@@ -268,6 +268,20 @@ __visible inline void syscall_return_slowpath(struct pt_regs *regs)prepare_exit_to_usermode (regs);
arch/x86/entry/common.c __visible inline void syscall_return_slowpath(struct pt_regs *regs) · line 278
Quoted 10 lines
arch/x86/entry/common.c +8
-0} +__visible inline void l1_cache_flush(struct pt_regs *regs) +{ +if (IS_ENABLED (CONFIG_SYSCALL_FLUSH ) && +static_cpu_has (X86_FEATURE_FLUSH_L1D )) { +if (regs->ax ==0 || regs->ax == -EAGAIN || + regs->ax == -EEXIST || regs->ax == -ENOENT || + regs->ax == -EXDEV || regs->ax == -ETIMEDOUT || + regs->ax == -ENOTCONN || regs->ax == -EINPROGRESS )
What about ax > 0? (Or more generally, any ax outside the range of -1 .. -4095 or whatever the error range is.) As it stands, it looks like you'll flush on successful read(), write(), recv(), etc, and that could seriously hurt performance on real workloads.
Quoted 30 lines
arch/x86/entry/common.c +10
-1+return ; + +wrmsrl (MSR_IA32_FLUSH_CMD ,L1D_FLUSH ); + } +} +#ifdef CONFIG_X86_64 __visible void do_syscall_64(unsigned long nr, struct pt_regs *regs) {@@ -290,6 +304,8 @@ __visible void do_syscall_64(unsigned long nr, struct pt_regs *regs) regs->ax = sys_call_table[nr](regs); } +l1_cache_flush (regs); +syscall_return_slowpath (regs); }#endif @@ -338,6 +354,8 @@ static __always_inline void do_syscall_32_irqs_on(struct pt_regs *regs)#endif /* CONFIG_IA32_EMULATION */ } +l1_cache_flush (regs); +syscall_return_slowpath (regs); } --2.14.4
Quoted 51 lines · Andy Lutomirski
Quoted 49 lines · Kristen Carlson Accardi
This patch aims to make it harder to perform cache timing attacks on data left behind by system calls. If we have an error returned from a syscall, flush the L1 cache.
It's important to note that this patch is not addressing any specific exploit, nor is it intended to be a complete defense against anything. It is intended to be a low cost way of eliminating some of side effects of a failed system call.
A performance test using sysbench on one hyperthread and a script which attempts to repeatedly access files it does not have permission to access on the other hyperthread found no significant performance impact.
Suggested-by Alan Cox <[email protected]>Signed-off-by Kristen Carlson Accardi <[email protected]>--- arch/x86/Kconfig | 9 +++++++++ arch/x86/entry/common.c | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+)arch/x86/Kconfig +9
-0@@ -445,6 +445,15 @@ config RETPOLINE code are eliminated. Since this includes the syscall entry path, it is not entirely pointless. +config SYSCALL_FLUSH + bool "Clear L1 Cache on syscall errors" + default n + help + Selecting 'y' allows the L1 cache to be cleared upon return of + an error code from a syscall if the CPU supports "flush_l1d". + This may reduce the likelyhood of speculative execution style + attacks on syscalls. + config INTEL_RDT bool "Intel Resource Director Technology support" default narch/x86/entry/common.c +0
-0@@ -268,6 +268,20 @@ __visible inline void syscall_return_slowpath(struct pt_regs *regs)prepare_exit_to_usermode (regs); }
Quoted 13 lines
arch/x86/entry/common.c __visible inline void syscall_return_slowpath(struct pt_regs *regs) · line 278
Quoted 8 lines
arch/x86/entry/common.c +8
-0+__visible inline void l1_cache_flush(struct pt_regs *regs) +{ +if (IS_ENABLED (CONFIG_SYSCALL_FLUSH ) && +static_cpu_has (X86_FEATURE_FLUSH_L1D )) { +if (regs->ax ==0 || regs->ax == -EAGAIN || + regs->ax == -EEXIST || regs->ax == -ENOENT || + regs->ax == -EXDEV || regs->ax == -ETIMEDOUT || + regs->ax == -ENOTCONN || regs->ax == -EINPROGRESS )What about ax > 0? (Or more generally, any ax outside the range of -1 .. -4095 or whatever the error range is.) As it stands, it looks like you'll flush on successful
read(),write(),recv(), etc, and that could seriously hurt performance on real workloads.
Seems like just changing this with "ax == 0" into "ax >= 0" would solve that?
I think this looks like a good idea. It might be worth adding a comment about the checks to explain why those errors are whitelisted. It's a cheap and effective mitigation for "unknown future problems" that doesn't degrade normal workloads.
Quoted 3 lines
arch/x86/entry/common.c __visible inline void syscall_return_slowpath(struct pt_regs *regs) · line 281
Quoted 3 lines
arch/x86/entry/common.c +3
-0+return ; + +wrmsrl (MSR_IA32_FLUSH_CMD ,L1D_FLUSH );
What about CPUs without FLUSH_L1D? Could it be done manually with a memcpy or something?
-Kees
Quoted 27 lines
Quoted 27 lines
arch/x86/entry/common.c +7
-1+ } +} +#ifdef CONFIG_X86_64 __visible void do_syscall_64(unsigned long nr, struct pt_regs *regs) {@@ -290,6 +304,8 @@ __visible void do_syscall_64(unsigned long nr, struct pt_regs *regs) regs->ax = sys_call_table[nr](regs); } +l1_cache_flush (regs); +syscall_return_slowpath (regs); }#endif @@ -338,6 +354,8 @@ static __always_inline void do_syscall_32_irqs_on(struct pt_regs *regs)#endif /* CONFIG_IA32_EMULATION */ } +l1_cache_flush (regs); +syscall_return_slowpath (regs); } --2.14.4
Signature · 2 lines
Kees Cook Pixel Security
Andy Lutomirski
maintainerQuoted 67 lines · Kees Cook
Quoted 51 lines · Andy Lutomirski
Quoted 49 lines · Kristen Carlson Accardi
This patch aims to make it harder to perform cache timing attacks on data left behind by system calls. If we have an error returned from a syscall, flush the L1 cache.
It's important to note that this patch is not addressing any specific exploit, nor is it intended to be a complete defense against anything. It is intended to be a low cost way of eliminating some of side effects of a failed system call.
A performance test using sysbench on one hyperthread and a script which attempts to repeatedly access files it does not have permission to access on the other hyperthread found no significant performance impact.
Suggested-by Alan Cox <[email protected]>Signed-off-by Kristen Carlson Accardi <[email protected]>--- arch/x86/Kconfig | 9 +++++++++ arch/x86/entry/common.c | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+)arch/x86/Kconfig +9
-0@@ -445,6 +445,15 @@ config RETPOLINE code are eliminated. Since this includes the syscall entry path, it is not entirely pointless. +config SYSCALL_FLUSH + bool "Clear L1 Cache on syscall errors" + default n + help + Selecting 'y' allows the L1 cache to be cleared upon return of + an error code from a syscall if the CPU supports "flush_l1d". + This may reduce the likelyhood of speculative execution style + attacks on syscalls. + config INTEL_RDT bool "Intel Resource Director Technology support" default narch/x86/entry/common.c +0
-0@@ -268,6 +268,20 @@ __visible inline void syscall_return_slowpath(struct pt_regs *regs)prepare_exit_to_usermode (regs); }Quoted 13 lines
arch/x86/entry/common.c __visible inline void syscall_return_slowpath(struct pt_regs *regs) · line 278
Quoted 8 lines
arch/x86/entry/common.c +8
-0+__visible inline void l1_cache_flush(struct pt_regs *regs) +{ +if (IS_ENABLED (CONFIG_SYSCALL_FLUSH ) && +static_cpu_has (X86_FEATURE_FLUSH_L1D )) { +if (regs->ax ==0 || regs->ax == -EAGAIN || + regs->ax == -EEXIST || regs->ax == -ENOENT || + regs->ax == -EXDEV || regs->ax == -ETIMEDOUT || + regs->ax == -ENOTCONN || regs->ax == -EINPROGRESS )What about ax > 0? (Or more generally, any ax outside the range of -1 .. -4095 or whatever the error range is.) As it stands, it looks like you'll flush on successful
read(),write(),recv(), etc, and that could seriously hurt performance on real workloads.
Quoted 1 line
Seems like just changing this with "ax == 0" into "ax >= 0" would solve that?
I can easily imagine that there are other errors for which performance matters. EBUSY comes to mind.
Quoted 5 lines
I think this looks like a good idea. It might be worth adding a comment about the checks to explain why those errors are whitelisted. It's a cheap and effective mitigation for "unknown future problems" that doesn't degrade normal workloads.
I still want to see two pieces of information before I ack a patch like this:
- How long does L1D_FLUSH take, roughly?
- An example of a type of attack that would be mitigated.
For the latter, I assume that the goal is to mitigate against attacks where a syscall speculatively loads something sensitive and then fails. But, before it fails, it leaks the information it speculatively loaded, and that leak ended up in L1D but *not* in other cache levels. And somehow the L1D can't be probed quickly enough in a parallel thread to meaningfully get the information out of L1D. Or maybe it's trying to mitigate against the use of failing syscalls to get some sensitive data into L1D and then using something like L1TF to read it out.
But this really needs to be clarified. Alan said that a bunch of the "yet another Spectre variant" attacks would have been mitigated by this patch. An explanation of *how* would be in order.
And we should seriously consider putting this kind of thing under CONFIG_SPECULATIVE_MITIGATIONS or similar. The idea being that it doesn't mitigate a clear known attack family.
Quoted 73 lines · Andy Lutomirski
Quoted 66 lines · Kees Cook
Quoted 51 lines · Andy Lutomirski
Quoted 49 lines · Kristen Carlson Accardi
This patch aims to make it harder to perform cache timing attacks on data left behind by system calls. If we have an error returned from a syscall, flush the L1 cache.
It's important to note that this patch is not addressing any specific exploit, nor is it intended to be a complete defense against anything. It is intended to be a low cost way of eliminating some of side effects of a failed system call.
A performance test using sysbench on one hyperthread and a script which attempts to repeatedly access files it does not have permission to access on the other hyperthread found no significant performance impact.
Suggested-by Alan Cox <[email protected]>Signed-off-by Kristen Carlson Accardi <[email protected]>--- arch/x86/Kconfig | 9 +++++++++ arch/x86/entry/common.c | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+)arch/x86/Kconfig +9
-0@@ -445,6 +445,15 @@ config RETPOLINE code are eliminated. Since this includes the syscall entry path, it is not entirely pointless. +config SYSCALL_FLUSH + bool "Clear L1 Cache on syscall errors" + default n + help + Selecting 'y' allows the L1 cache to be cleared upon return of + an error code from a syscall if the CPU supports "flush_l1d". + This may reduce the likelyhood of speculative execution style + attacks on syscalls. + config INTEL_RDT bool "Intel Resource Director Technology support" default narch/x86/entry/common.c +0
-0@@ -268,6 +268,20 @@ __visible inline void syscall_return_slowpath(struct pt_regs *regs)prepare_exit_to_usermode (regs); }Quoted 13 lines
arch/x86/entry/common.c __visible inline void syscall_return_slowpath(struct pt_regs *regs) · line 278
Quoted 8 lines
arch/x86/entry/common.c +8
-0+__visible inline void l1_cache_flush(struct pt_regs *regs) +{ +if (IS_ENABLED (CONFIG_SYSCALL_FLUSH ) && +static_cpu_has (X86_FEATURE_FLUSH_L1D )) { +if (regs->ax ==0 || regs->ax == -EAGAIN || + regs->ax == -EEXIST || regs->ax == -ENOENT || + regs->ax == -EXDEV || regs->ax == -ETIMEDOUT || + regs->ax == -ENOTCONN || regs->ax == -EINPROGRESS )What about ax > 0? (Or more generally, any ax outside the range of -1 .. -4095 or whatever the error range is.) As it stands, it looks like you'll flush on successful
read(),write(),recv(), etc, and that could seriously hurt performance on real workloads.Quoted 1 line
Seems like just changing this with "ax == 0" into "ax >= 0" would solve that?
I can easily imagine that there are other errors for which performance matters. EBUSY comes to mind.
Quoted 1 line
Quoted 8 lines
Quoted 4 lines
I think this looks like a good idea. It might be worth adding a comment about the checks to explain why those errors are whitelisted. It's a cheap and effective mitigation for "unknown future problems" that doesn't degrade normal workloads.
I still want to see two pieces of information before I ack a patch like this:
- How long does
L1D_FLUSHtake, roughly?
(Especially if L1D is dirty and you can't just wipe it all.)
Quoted 8 lines
- An example of a type of attack that would be mitigated.
For the latter, I assume that the goal is to mitigate against attacks where a syscall speculatively loads something sensitive and then fails. But, before it fails, it leaks the information it speculatively loaded, and that leak ended up in L1D but *not* in other cache levels. And somehow the L1D can't be probed quickly enough in a parallel thread to meaningfully get the information out of L1D.
And the attacker can't delay the syscall return somehow, e.g. on a fully-preemptible kernel by preempting the syscall, or by tarpitting a userspace memory access in the error path.
Quoted 12 lines
Or
maybe it's trying to mitigate against the use of failing syscalls to get some sensitive data into L1D and then using something like L1TF to read it out.But this really needs to be clarified. Alan said that a bunch of the "yet another Spectre variant" attacks would have been mitigated by this patch. An explanation of *how* would be in order.
And we should seriously consider putting this kind of thing under
CONFIG_SPECULATIVE_MITIGATIONSor similar. The idea being that it doesn't mitigate a clear known attack family.
Quoted 3 lines
But this really needs to be clarified. Alan said that a bunch of the "yet another Spectre variant" attacks would have been mitigated by this patch. An explanation of *how* would be in order.
Today you have the situation where something creates a speculative disclosure gadget. So we run around and we try and guess where to fix them all with lfence. If you miss one then it leaves a trace in the L1D cache, which is what you measure.
In almost every case we have looked at when you leave a footprint in the L1D you resolve to an error path so the syscall errors.
In other words every time we fail to find a
if (foo < limit) {
gadget(array[foo]);
} else
return -EINVAL;we turn that from being an easy to use gadget into something really tricky because by the time the code flow has gotten back to the caller the breadcrumbs have been eaten by the L1D flush.
The current process of trying to find them all with smatch and the like is a game of whack-a-mole that will go on for a long long time. In the meantime (and until the tools get better) it's nice to have an option that takes a totally non-hot path (the fast path change is a single test for >= 0) and provides additional defence in depth.
They are not hot paths: when I started playing with this idea I did indeed strace my entire desktop for a day. There are couple of other cases I would add to the pass list (EAGAIN, EWOULDBLOCK)). Tracing other stuff you see the same - real world workloads simply don't generate vast spews of erroring syscalls.
Look just how many examples we are still committing like de916736aaaadddbd6061472969f667b14204aa9 7b6924d94a60c6b8c1279ca003e8744e6cd9e8b1 46feb6b495f7628a6dbf36c4e6d80faf378372d4 55690c07b44a82cc3359ce0c233f4ba7d80ba145
This approach turns the ones we miss into
if (type > =
[speculatively create breadcrumb]
condition resolves, return -EINVAL
L1D flush
No breadcrumbs
At best you have a microscopic window to attack it on the SMT pair.
Alan
Quoted 14 lines · Alan Cox
Quoted 3 lines
But this really needs to be clarified. Alan said that a bunch of the "yet another Spectre variant" attacks would have been mitigated by this patch. An explanation of *how* would be in order.
Today you have the situation where something creates a speculative disclosure gadget. So we run around and we try and guess where to fix them all with lfence. If you miss one then it leaves a trace in the L1D cache, which is what you measure.
In almost every case we have looked at when you leave a footprint in the L1D you resolve to an error path so the syscall errors.
In other words every time we fail to find a
Quoted 8 lines
if (foo < limit) { gadget(array[foo]); } else return -EINVAL;we turn that from being an easy to use gadget into something really tricky because by the time the code flow has gotten back to the caller the breadcrumbs have been eaten by the L1D flush.
My understanding is that the standard “breadcrumb” is that a cache line is fetched into L1D, and that the cacheline in question will go into L1D even if it was previously not cached at all. So flushing L1D will cause the timing from a probe to be different, but the breadcrumb is still there, and the attack will still work.
Am I wrong?
Quoted 3 lines
At best you have a microscopic window to attack it on the SMT pair.
So only the extra clever attackers will pull it off. This isn’t all that reassuring.
If I had the time to try to break this, I would set it up so that the cache lines that get probed are cached remotely, and I’d spin, waiting until one of them gets stolen. The spin loop would be much faster this way.
Or I would get a fancy new CPU and use UMONITOR and, unless UMONITOR is much cleverer than I suspect it is, the gig is up. The time window for the attack could be as small as you want, and UMONITOR will catch it.
(Have I mentioned that I think that Intel needs to remove UMONITOR, PCOMMIT-style? That instruction is simply the wrong solution to whatever problem it’s trying to solve.)
Quoted 2 lines
Alan
Quoted 1 line
My understanding is that the standard “breadcrumb” is that a cache line is fetched into L1D, and that the cacheline in question will go into L1D even if it was previously not cached at all. So flushing L1D will cause the timing from a probe to be different, but the breadcrumb is still there, and the attack will still work.
Flush not write back. The L1D is empty (or full of other stuff the way the prototype I tested did it as x86 lacked a true L1 flushing primitive)
Quoted 3 lines
Quoted 1 line
At best you have a microscopic window to attack it on the SMT pair.
So only the extra clever attackers will pull it off. This isn’t all that reassuring.
It's going to be very hard for them to do that. You don't really have the timing control in userspace to do that sort of thing easily.
At the end of the day it's an additional hardenening not a fix. It's designed to provide extra protection for the cases we don't know about until we find them and lfence them. All of this (and all sidechannel of any kind) is merely about reducing the bandwidth an attacker can achieve.
The idea is that it's cheap enough that it's worth doing.
Quoted 1 line
Or I would get a fancy new CPU and use UMONITOR and, unless UMONITOR is much cleverer than I suspect it is, the gig is up. The time window for the attack could be as small as you want, and UMONITOR will catch it.
That would be an interesting line of attack. You would still have to navigate a fair bit of noise.
Alan
Quoted 5 lines · Alan Cox
Quoted 2 lines
My understanding is that the standard “breadcrumb” is that a cache line is fetched into L1D, and that the cacheline in question will go into L1D even if it was previously not cached at all. So flushing L1D will cause the timing from a probe to be different, but the breadcrumb is still there, and the attack will still work.
Flush not write back. The L1D is empty (or full of other stuff the way the prototype I tested did it as x86 lacked a true L1 flushing primitive)
What about the TLB? Once KPTI is unnecessary, are you going to need an extra TLB flush here to prevent PRIME+PROBE on the TLB?
Also: Are you assuming that the attacker would only be able to do PRIME+PROBE, not FLUSH+RELOAD or other attacks that rely on shared memory (like FLUSH+FLUSH, or the MESI-based stuff that detects when a line stops being in modified state on another core)?
Quoted 20 lines
Quoted 3 lines
Quoted 1 line
At best you have a microscopic window to attack it on the SMT pair.
So only the extra clever attackers will pull it off. This isn’t all that reassuring.
It's going to be very hard for them to do that. You don't really have the timing control in userspace to do that sort of thing easily.
At the end of the day it's an additional hardenening not a fix. It's designed to provide extra protection for the cases we don't know about until we find them and lfence them. All of this (and all sidechannel of any kind) is merely about reducing the bandwidth an attacker can achieve.
The idea is that it's cheap enough that it's worth doing.
Quoted 1 line
Or I would get a fancy new CPU and use UMONITOR and, unless UMONITOR is much cleverer than I suspect it is, the gig is up. The time window for the attack could be as small as you want, and UMONITOR will catch it.
That would be an interesting line of attack. You would still have to navigate a fair bit of noise.
Alan
Quoted 5 lines · Alan Cox
Quoted 2 lines
My understanding is that the standard “breadcrumb” is that a cache line is fetched into L1D, and that the cacheline in question will go into L1D even if it was previously not cached at all. So flushing L1D will cause the timing from a probe to be different, but the breadcrumb is still there, and the attack will still work.
Flush not write back. The L1D is empty (or full of other stuff the way the prototype I tested did it as x86 lacked a true L1 flushing primitive)
I'm not sure I follow what you're saying. If an attacker is learning some information based on whether a given cacheline is in L1D, I'm asking why the attacker can't learn exactly the same information based on whether the cache line is in L2. Or using any of the other variants that Jann is talking about.
Adding ~1600 cycles plus the slowdown due to the fact that the cache got flushed to a code path that we hope isn't hot to mitigate one particular means of exploiting potential bugs seems a bit dubious to me.
Quoted 83 lines · Kees Cook
Quoted 72 lines · Andy Lutomirski
Quoted 70 lines · Kristen Carlson Accardi
This patch aims to make it harder to perform cache timing attacks on data
left behind by system calls. If we have an error returned from a syscall,
flush the L1 cache.It's important to note that this patch is not addressing any specific
exploit, nor is it intended to be a complete defense against anything.
It is intended to be a low cost way of eliminating some of side effects
of a failed system call.A performance test using sysbench on one hyperthread and a script which
attempts to repeatedly access files it does not have permission to access
on the other hyperthread found no significant performance impact.Suggested-by Alan Cox <[email protected]>Signed-off-by Kristen Carlson Accardi <[email protected]>--- arch/x86/Kconfig | 9 +++++++++ arch/x86/entry/common.c | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+)arch/x86/Kconfig +5
-0@@ -445,6 +445,15 @@ config RETPOLINE code are eliminated. Since this includes the syscall entry path, it is not entirely pointless. +config SYSCALL_FLUSH + bool "Clear L1 Cache on syscall errors" + default n + help + Selecting 'y' allows the L1 cache to be cleared uponreturn of
+ an error code from a syscall if the CPU supports
"flush_l1d".
+ This may reduce the likelyhood of speculative execution
stylearch/x86/Kconfig +2
-0+ attacks on syscalls. + config INTEL_RDT bool "Intel Resource Director Technology support" default narch/x86/entry/common.c +7
-0@@ -268,6 +268,20 @@ __visible inline void syscall_return_slowpath(struct pt_regs *regs)prepare_exit_to_usermode (regs); } +__visible inline void l1_cache_flush(struct pt_regs *regs) +{ +if (IS_ENABLED (CONFIG_SYSCALL_FLUSH ) && +static_cpu_has (X86_FEATURE_FLUSH_L1D )) { +if (regs->ax ==0 || regs->ax == -EAGAIN || + regs->ax == -EEXIST || regs->ax == -ENOENT || + regs->ax == -EXDEV || regs->ax == -ETIMEDOUT|| + regs->ax == -ENOTCONN || regs->ax ==Quoted 8 lines
Quoted 1 line
-EINPROGRESS)What about ax > 0? (Or more generally, any ax outside the range of -1
.. -4095 or whatever the error range is.) As it stands, it looks like
you'll flush on successfulread(),write(),recv(), etc, and that could seriously hurt performance on real workloads.
Quoted 2 lines
Seems like just changing this with "ax == 0" into "ax >= 0" would solve that?
thanks, will do.
Quoted 6 lines
I think this looks like a good idea. It might be worth adding a comment about the checks to explain why those errors are whitelisted. It's a cheap and effective mitigation for "unknown future problems" that doesn't degrade normal workloads.
Quoted 6 lines
Quoted 3 lines
arch/x86/entry/common.c __visible inline void syscall_return_slowpath(struct pt_regs *regs) · line 281
Quoted 3 lines
arch/x86/entry/common.c +3
-0+return ; + +wrmsrl (MSR_IA32_FLUSH_CMD ,L1D_FLUSH );What about CPUs without
FLUSH_L1D? Could it be done manually with a memcpy or something?
It could - my original implementation (pre l1d_flush msr) did, but it did come with some additional cost in that I allocated per-cpu memory to keep a 32K buffer around that I could memcpy. It also sacrificed completeness for simplicity by not taking into account cases where L1 was not 32K. As far as I know this msr is pretty widely deployed, even on older hardware.
Quoted 13 lines · Kristen C Accardi
Quoted 11 lines · Kees Cook
I think this looks like a good idea. It might be worth adding a comment about the checks to explain why those errors are whitelisted. It's a cheap and effective mitigation for "unknown future problems" that doesn't degrade normal workloads.
Quoted 3 lines
arch/x86/entry/common.c __visible inline void syscall_return_slowpath(struct pt_regs *regs) · line 281
Quoted 3 lines
arch/x86/entry/common.c +3
-0+return ; + +wrmsrl (MSR_IA32_FLUSH_CMD ,L1D_FLUSH );What about CPUs without
FLUSH_L1D? Could it be done manually with a memcpy or something?
Quoted 6 lines
It could - my original implementation (pre
l1d_flushmsr) did, but it did come with some additional cost in that I allocated per-cpu memory to keep a 32K buffer around that I could memcpy. It also sacrificed completeness for simplicity by not taking into account cases where L1 was not 32K. As far as I know this msr is pretty widely deployed, even on older hardware.
You don't need that per cpu thing, really. We have both the MSR flush and the software fallback in KVM vmx_l1d_flush(). The performance impact is pretty much the same between the MSR flush and the software fallback.
Vs. deployment of the MSR. I'm not sure how wide this is supported on older CPUs as there have been limitations on microcode patch space, but I can't find the support matrix right now. Thanks Intel for reshuffling the webpage every other week!
Thanks,
tglx
Quoted 66 lines · Kees Cook
Quoted 51 lines · Andy Lutomirski
Quoted 49 lines · Kristen Carlson Accardi
This patch aims to make it harder to perform cache timing attacks on data left behind by system calls. If we have an error returned from a syscall, flush the L1 cache.
It's important to note that this patch is not addressing any specific exploit, nor is it intended to be a complete defense against anything. It is intended to be a low cost way of eliminating some of side effects of a failed system call.
A performance test using sysbench on one hyperthread and a script which attempts to repeatedly access files it does not have permission to access on the other hyperthread found no significant performance impact.
Suggested-by Alan Cox <[email protected]>Signed-off-by Kristen Carlson Accardi <[email protected]>--- arch/x86/Kconfig | 9 +++++++++ arch/x86/entry/common.c | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+)arch/x86/Kconfig +9
-0@@ -445,6 +445,15 @@ config RETPOLINE code are eliminated. Since this includes the syscall entry path, it is not entirely pointless. +config SYSCALL_FLUSH + bool "Clear L1 Cache on syscall errors" + default n + help + Selecting 'y' allows the L1 cache to be cleared upon return of + an error code from a syscall if the CPU supports "flush_l1d". + This may reduce the likelyhood of speculative execution style + attacks on syscalls. + config INTEL_RDT bool "Intel Resource Director Technology support" default narch/x86/entry/common.c +0
-0@@ -268,6 +268,20 @@ __visible inline void syscall_return_slowpath(struct pt_regs *regs)prepare_exit_to_usermode (regs); }Quoted 13 lines
arch/x86/entry/common.c __visible inline void syscall_return_slowpath(struct pt_regs *regs) · line 278
Quoted 8 lines
arch/x86/entry/common.c +8
-0+__visible inline void l1_cache_flush(struct pt_regs *regs) +{ +if (IS_ENABLED (CONFIG_SYSCALL_FLUSH ) && +static_cpu_has (X86_FEATURE_FLUSH_L1D )) { +if (regs->ax ==0 || regs->ax == -EAGAIN || + regs->ax == -EEXIST || regs->ax == -ENOENT || + regs->ax == -EXDEV || regs->ax == -ETIMEDOUT || + regs->ax == -ENOTCONN || regs->ax == -EINPROGRESS )What about ax > 0? (Or more generally, any ax outside the range of -1 .. -4095 or whatever the error range is.) As it stands, it looks like you'll flush on successful
read(),write(),recv(), etc, and that could seriously hurt performance on real workloads.
Quoted 1 line
Seems like just changing this with "ax == 0" into "ax >= 0" would solve that?
As spender points out on twitter
(https://twitter.com/grsecurity/status/1050497259937370118 - thanks, spender!), struct pt_regs stores register values as "unsigned long", and so you'll need to use something like IS_ERR_VALUE().
Quoted 7 lines · Jann Horn
Quoted 1 line · Kees Cook
Seems like just changing this with "ax == 0" into "ax >= 0" would solve that?
As spender points out on twitter
(https://twitter.com/grsecurity/status/1050497259937370118 - thanks, spender!), structpt_regsstores register values as "unsigned long", and so you'll need to use something likeIS_ERR_VALUE().
Ha, so it's not only me who noticed, but then I'm not reading twitter...
Thanks,
tglx
Quoted 16 lines · Kees Cook
Quoted 14 lines · Andy Lutomirski
On Thu, Oct 11, 2018 at 11:55 AM Kristen Carlson Accardi
arch/x86/entry/common.c __visible inline void syscall_return_slowpath(struct pt_regs *regs) · line 278
Quoted 8 lines
arch/x86/entry/common.c +8
-0+__visible inline void l1_cache_flush(struct pt_regs *regs) +{ +if (IS_ENABLED (CONFIG_SYSCALL_FLUSH ) && +static_cpu_has (X86_FEATURE_FLUSH_L1D )) { +if (regs->ax ==0 || regs->ax == -EAGAIN || + regs->ax == -EEXIST || regs->ax == -ENOENT || + regs->ax == -EXDEV || regs->ax == -ETIMEDOUT || + regs->ax == -ENOTCONN || regs->ax == -EINPROGRESS )What about ax > 0? (Or more generally, any ax outside the range of -1 .. -4095 or whatever the error range is.) As it stands, it looks like you'll flush on successful
read(),write(),recv(), etc, and that could seriously hurt performance on real workloads.
Quoted 6 lines
Seems like just changing this with "ax == 0" into "ax >= 0" would solve that?
I think this looks like a good idea. It might be worth adding a comment about the checks to explain why those errors are whitelisted. It's a cheap and effective mitigation for "unknown future problems" that doesn't degrade normal workloads.
pt_regs->ax is unsigned long, so you want to check this with IS_ERR_VALUE() first.
if (!IS_ERR_VALUE(regs->ax)) return;
and then you really want to have something smarter than a gazillion of whitelisted error value checks, which effectively compile into a gazillion conditonal branches.
Thanks,
tglx