[RFC PATCH] x86: entry: flush the cache if syscall error

[RFC PATCH] x86: entry: flush the cache if syscall error

[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.

Signed-off-by: Kristen Carlson Accardi <kristen@linux.intel.com>
---
 arch/x86/Kconfig        |  8 ++++++++
 arch/x86/entry/common.c | 20 ++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index c5ff296bc5d1..8a67642ff9fe 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -445,6 +445,14 @@ 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 y
+	help
+	  Select to allow the L1 cache to be cleared upon return of
+	  an error code from a syscall. This will reduce the likelyhood of
+	  speculative execution style attacks on syscalls.
+
 config INTEL_RDT
 	bool "Intel Resource Director Technology support"
 	default n
diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index 3b2490b81918..77beff541013 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -268,6 +268,22 @@ __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)) {
+		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;
+
+		if (static_cpu_has(X86_FEATURE_FLUSH_L1D)) {
+			wrmsrl(MSR_IA32_FLUSH_CMD, L1D_FLUSH);
+			return;
+		}
+	}
+}
+
 #ifdef CONFIG_X86_64
 __visible void do_syscall_64(unsigned long nr, struct pt_regs *regs)
 {
@@ -290,6 +306,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 +356,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

Re: [RFC PATCH] x86: entry: flush the cache if syscall error

[Jann Horn]

On Mon, Sep 10, 2018 at 9:14 PM Kristen Carlson Accardi
<kristen@linux.intel.com> wrote:
> 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.

What kind of performance impact does this have on a process that e.g.
attempts to access a large number of paths to which it has no access,
and what is the impact on the hyperthread?

(You may want to also CC the X86 maintainers (especially Andy
Lutomirski) and LKML on this series relatively early - people on the
kernel-hardening list tend to have a different focus compared to the
relevant maintainers, so there's some feedback that you'll probably
only get once you submit this to places other than the
kernel-hardening@ list.)

> Signed-off-by: Kristen Carlson Accardi <kristen@linux.intel.com>
> ---
>  arch/x86/Kconfig        |  8 ++++++++
>  arch/x86/entry/common.c | 20 ++++++++++++++++++++
>  2 files changed, 28 insertions(+)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index c5ff296bc5d1..8a67642ff9fe 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -445,6 +445,14 @@ 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 y
> +       help
> +         Select to allow the L1 cache to be cleared upon return of
> +         an error code from a syscall. This will reduce the likelyhood of
> +         speculative execution style attacks on syscalls.

s/L1/L1D/ ?

>  config INTEL_RDT
>         bool "Intel Resource Director Technology support"
>         default n
> diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
> index 3b2490b81918..77beff541013 100644
> --- a/arch/x86/entry/common.c
> +++ b/arch/x86/entry/common.c
> @@ -268,6 +268,22 @@ __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)) {
> +               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;
> +
> +               if (static_cpu_has(X86_FEATURE_FLUSH_L1D)) {

Style nit: Maybe merge this condition into the first if() ?

> +                       wrmsrl(MSR_IA32_FLUSH_CMD, L1D_FLUSH);
> +                       return;
> +               }
> +       }
> +}
> +
>  #ifdef CONFIG_X86_64
>  __visible void do_syscall_64(unsigned long nr, struct pt_regs *regs)
>  {
> @@ -290,6 +306,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 +356,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
>

Re: [RFC PATCH] x86: entry: flush the cache if syscall error

[Greg KH]

On Mon, Sep 10, 2018 at 12:10:02PM -0700, Kristen Carlson Accardi wrote:
> 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.
> 
> Signed-off-by: Kristen Carlson Accardi <kristen@linux.intel.com>
> ---
>  arch/x86/Kconfig        |  8 ++++++++
>  arch/x86/entry/common.c | 20 ++++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index c5ff296bc5d1..8a67642ff9fe 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -445,6 +445,14 @@ 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 y

"y" normally is only for "your machine will not boot without this
option", and I don't think that's the case here :)

> +	help
> +	  Select to allow the L1 cache to be cleared upon return of
> +	  an error code from a syscall. This will reduce the likelyhood of
> +	  speculative execution style attacks on syscalls.

Shouldn't this help text refer to the fact that this needs CPU support
for this type of functionality?

I like the idea, as a "gadget" normally only is used when an
out-of-bands check happens, which implies someone could be trying to do
something "bad", nice job.

thanks,

greg k-h

Re: [RFC PATCH] x86: entry: flush the cache if syscall error

[Kristen C Accardi]

On Mon, 2018-09-10 at 22:32 +0200, Jann Horn wrote:
> On Mon, Sep 10, 2018 at 9:14 PM Kristen Carlson Accardi
> <kristen@linux.intel.com> wrote:
> > 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.
> 
> What kind of performance impact does this have on a process that e.g.
> attempts to access a large number of paths to which it has no access,
> and what is the impact on the hyperthread?

This will undoubtedly have a performance impact - I can certainly run
some benchmarks to find out just how much in this scenario.

> 
> (You may want to also CC the X86 maintainers (especially Andy
> Lutomirski) and LKML on this series relatively early - people on the
> kernel-hardening list tend to have a different focus compared to the
> relevant maintainers, so there's some feedback that you'll probably
> only get once you submit this to places other than the
> kernel-hardening@ list.)

Thank you yes, I admit I decided to try to float this idea past a
friendlier audience first :).

> 
> > Signed-off-by: Kristen Carlson Accardi <kristen@linux.intel.com>
> > ---
> >  arch/x86/Kconfig        |  8 ++++++++
> >  arch/x86/entry/common.c | 20 ++++++++++++++++++++
> >  2 files changed, 28 insertions(+)
> > 
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index c5ff296bc5d1..8a67642ff9fe 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -445,6 +445,14 @@ 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 y
> > +       help
> > +         Select to allow the L1 cache to be cleared upon return of
> > +         an error code from a syscall. This will reduce the
> > likelyhood of
> > +         speculative execution style attacks on syscalls.
> 
> s/L1/L1D/ ?

I can change this - the documentation for this msr mentioned that on
some processors the icache might be impacted as well - think that's
worth mentioning?

> 
> >  config INTEL_RDT
> >         bool "Intel Resource Director Technology support"
> >         default n
> > diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
> > index 3b2490b81918..77beff541013 100644
> > --- a/arch/x86/entry/common.c
> > +++ b/arch/x86/entry/common.c
> > @@ -268,6 +268,22 @@ __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)) {
> > +               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;
> > +
> > +               if (static_cpu_has(X86_FEATURE_FLUSH_L1D)) {
> 
> Style nit: Maybe merge this condition into the first if() ?
> 
> > +                       wrmsrl(MSR_IA32_FLUSH_CMD, L1D_FLUSH);
> > +                       return;
> > +               }
> > +       }
> > +}
> > +
> >  #ifdef CONFIG_X86_64
> >  __visible void do_syscall_64(unsigned long nr, struct pt_regs
> > *regs)
> >  {
> > @@ -290,6 +306,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 +356,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
> > 

Re: [RFC PATCH] x86: entry: flush the cache if syscall error

[Kristen C Accardi]

On Tue, 2018-09-11 at 10:41 +0200, Greg KH wrote:
> On Mon, Sep 10, 2018 at 12:10:02PM -0700, Kristen Carlson Accardi
> wrote:
> > 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.
> > 
> > Signed-off-by: Kristen Carlson Accardi <kristen@linux.intel.com>
> > ---
> >  arch/x86/Kconfig        |  8 ++++++++
> >  arch/x86/entry/common.c | 20 ++++++++++++++++++++
> >  2 files changed, 28 insertions(+)
> > 
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index c5ff296bc5d1..8a67642ff9fe 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -445,6 +445,14 @@ 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 y
> 
> "y" normally is only for "your machine will not boot without this
> option", and I don't think that's the case here :)

true. :). I wish there was a way to indicate that this option provided
enhanced security so that people could just select all of these
features at once and I could make it select based on that option.

> 
> > +	help
> > +	  Select to allow the L1 cache to be cleared upon return
> > of
> > +	  an error code from a syscall. This will reduce the
> > likelyhood of
> > +	  speculative execution style attacks on syscalls.
> 
> Shouldn't this help text refer to the fact that this needs CPU
> support
> for this type of functionality?

OK, I will change it.

> 
> I like the idea, as a "gadget" normally only is used when an
> out-of-bands check happens, which implies someone could be trying to
> do
> something "bad", nice job.
> 
> thanks,
> 
> greg k-h

Thanks!

Re: [RFC PATCH] x86: entry: flush the cache if syscall error

[Eric Biggers]

On Mon, Sep 10, 2018 at 12:10:02PM -0700, Kristen Carlson Accardi wrote:
> 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.

Which L1 cache?  There's no guarantee the task stayed on the same CPU...

- Eric

Re: [RFC PATCH] x86: entry: flush the cache if syscall error

[Jann Horn]

On Tue, Sep 11, 2018 at 5:58 PM Kristen C Accardi
<kristen@linux.intel.com> wrote:
> On Mon, 2018-09-10 at 22:32 +0200, Jann Horn wrote:
> > On Mon, Sep 10, 2018 at 9:14 PM Kristen Carlson Accardi
> > <kristen@linux.intel.com> wrote:
> > > 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.
[...]
> > > +config SYSCALL_FLUSH
> > > +       bool "Clear L1 Cache on syscall errors"
> > > +       default y
> > > +       help
> > > +         Select to allow the L1 cache to be cleared upon return of
> > > +         an error code from a syscall. This will reduce the
> > > likelyhood of
> > > +         speculative execution style attacks on syscalls.
> >
> > s/L1/L1D/ ?
>
> I can change this - the documentation for this msr mentioned that on
> some processors the icache might be impacted as well - think that's
> worth mentioning?

Ah, whoops, I didn't realize that it isn't as clear-cut as "just flush
L1D". I should've looked for the documentation first... nevermind, I
guess. I don't want to turn this into a documentation bikeshed.

Re: [RFC PATCH] x86: entry: flush the cache if syscall error

[Jann Horn]

On Mon, Sep 10, 2018 at 9:14 PM Kristen Carlson Accardi
<kristen@linux.intel.com> wrote:
> 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.

How much protection does this provide, given that it e.g. doesn't
flush L2/L3 and doesn't prevent data leakage through hyperthreading
and cache coherency? Is an L2/L3-based attack expected to be harder
than an L1D-based one?

Re: [RFC PATCH] x86: entry: flush the cache if syscall error

[Kristen C Accardi]

On Tue, 2018-09-11 at 09:06 -0700, Eric Biggers wrote:
> On Mon, Sep 10, 2018 at 12:10:02PM -0700, Kristen Carlson Accardi
> wrote:
> > 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.
> 
> Which L1 cache?  There's no guarantee the task stayed on the same
> CPU...

While this is true, it is unlikely that the task switched CPUs for this
type of flow (i.e. an error path, presumably caught early-ish), and
worst case this would just mean we were wiping the wrong cache. I can
add a comment to indicate this scenario.

Re: [RFC PATCH] x86: entry: flush the cache if syscall error

[Kristen C Accardi]

On Tue, 2018-09-11 at 20:02 +0200, Jann Horn wrote:
> On Mon, Sep 10, 2018 at 9:14 PM Kristen Carlson Accardi
> <kristen@linux.intel.com> wrote:
> > 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.
> 
> How much protection does this provide, given that it e.g. doesn't
> flush L2/L3 and doesn't prevent data leakage through hyperthreading
> and cache coherency? Is an L2/L3-based attack expected to be harder
> than an L1D-based one?

My reasoning here is that L2/L3 caches can be partitioned using
something like CAT (maybe), but L1 cannot. So IMO L1 is the case that
needs coverage. Also, while this doesn't address a specific exploit,
the idea is that attacks on data in L1D are more common, and the
performance penalty for L2/L3 flushes would be too high without a
specific exploit in mind.
 

Re: [RFC PATCH] x86: entry: flush the cache if syscall error

[Eric Biggers]

On Wed, Sep 12, 2018 at 10:29:49AM -0700, Kristen C Accardi wrote:
> On Tue, 2018-09-11 at 09:06 -0700, Eric Biggers wrote:
> > On Mon, Sep 10, 2018 at 12:10:02PM -0700, Kristen Carlson Accardi
> > wrote:
> > > 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.
> > 
> > Which L1 cache?  There's no guarantee the task stayed on the same
> > CPU...
> 
> While this is true, it is unlikely that the task switched CPUs for this
> type of flow (i.e. an error path, presumably caught early-ish), 

How you do know it's unlikely?  What degrees of freedom might an attacker have
in controlling this?

> worst case this would just mean we were wiping the wrong cache. I can
> add a comment to indicate this scenario.
> 

IOW, the protection may be useless?

- Eric

Re: [RFC PATCH] x86: entry: flush the cache if syscall error

[Rik van Riel]

[-- Attachment #1: Type: text/plain, Size: 1323 bytes --]

On Wed, 2018-09-12 at 10:45 -0700, Eric Biggers wrote:
> On Wed, Sep 12, 2018 at 10:29:49AM -0700, Kristen C Accardi wrote:
> > On Tue, 2018-09-11 at 09:06 -0700, Eric Biggers wrote:
> > > On Mon, Sep 10, 2018 at 12:10:02PM -0700, Kristen Carlson Accardi
> > > wrote:
> > > > 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.
> > > 
> > > Which L1 cache?  There's no guarantee the task stayed on the same
> > > CPU...
> > 
> > While this is true, it is unlikely that the task switched CPUs for
> > this
> > type of flow (i.e. an error path, presumably caught early-ish), 
> 
> How you do know it's unlikely?  What degrees of freedom might an
> attacker have
> in controlling this?
> 
> > worst case this would just mean we were wiping the wrong cache. I
> > can
> > add a comment to indicate this scenario.
> > 
> 
> IOW, the protection may be useless?

If the task gets moved to a different CPU, won't that
completely foil a timing attack?

In other words, this protection would protect against
an attack on the same CPU, and is unnecessary when a
task switches CPUs?

What am I missing?

-- 
All Rights Reversed.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]