Re: [RFC] x86: Speculative execution warnings

2019-05-16 Thread Nadav Amit
> On May 14, 2019, at 10:15 AM, Andy Lutomirski  wrote:
> 
> 
> 
> On May 14, 2019, at 10:00 AM, Nadav Amit  wrote:
> 
>>> On May 14, 2019, at 1:00 AM, Paul Turner  wrote:
>>> 
>>> From: Nadav Amit 
>>> Date: Fri, May 10, 2019 at 7:45 PM
>>> To: 
>>> Cc: Borislav Petkov, , Nadav Amit, Andy
>>> Lutomirsky, Ingo Molnar, Peter Zijlstra, Thomas Gleixner, Jann Horn
>>> 
 It may be useful to check in runtime whether certain assertions are
 violated even during speculative execution. This can allow to avoid
 adding unnecessary memory fences and at the same time check that no data
 leak channels exist.
 
 For example, adding such checks can show that allocating zeroed pages
 can return speculatively non-zeroed pages (the first qword is not
 zero).  [This might be a problem when the page-fault handler performs
 software page-walk, for example.]
 
 Introduce SPEC_WARN_ON(), which checks in runtime whether a certain
 condition is violated during speculative execution. The condition should
 be computed without branches, e.g., using bitwise operators. The check
 will wait for the condition to be realized (i.e., not speculated), and
 if the assertion is violated, a warning will be thrown.
 
 Warnings can be provided in one of two modes: precise and imprecise.
 Both mode are not perfect. The precise mode does not always make it easy
 to understand which assertion was broken, but instead points to a point
 in the execution somewhere around the point in which the assertion was
 violated.  In addition, it prints a warning for each violation (unlike
 WARN_ONCE() like behavior).
 
 The imprecise mode, on the other hand, can sometimes throw the wrong
 indication, specifically if the control flow has changed between the
 speculative execution and the actual one. Note that it is not a
 false-positive, it just means that the output would mislead the user to
 think the wrong assertion was broken.
 
 There are some more limitations. Since the mechanism requires an
 indirect branch, it should not be used in production systems that are
 susceptible for Spectre v2. The mechanism requires TSX and performance
 counters that are only available in skylake+. There is a hidden
 assumption that TSX is not used in the kernel for anything else, other
 than this mechanism.
>>> 
>>> Nice trick!
>> 
>> “Illusion." [ ignore if you don’t know the reference ]
>> 
>>> Can you eliminate the indirect call by forcing an access fault to
>>> abort the transaction instead, e.g. "cmove 0, $1”?
>>> 
>>> (If this works, it may also allow support on older architectures as
>>> the RTM_RETIRED.ABORT* events go back further I believe?)
>> 
>> I don’t think it would work. The whole problem is that we need a counter
>> that is updated during execution and not retirement. I tried several
>> counters and could not find other appropriate ones.
>> 
>> The idea behind the implementation is to affect the control flow through
>> data dependency. I may be able to do something similar without an indirect
>> branch. I’ll take a page, put the XABORT on the page and make the page NX.
>> Then, a direct jump would go to this page. The conditional-mov would change
>> the PTE to X if the assertion is violated. There should be a page-walk even
>> if the CPU finds the entry in the TLB, since this entry is NX.
> 
> I think you only get a page walk if the TLB entry is not-present. I’d be a
> bit surprised if the CPU is willing to execute, even speculatively, from
> speculatively written data. Good luck!

I guess you are right (although I didn’t try). IIRC, Jann Horn once
explained to me that if CPUs used PTEs that were written speculatively, this
would have been a correctness issue, since the PTE needs to get to the TLB
before it is used.

I’ll try a different path (not concrete idea which), assuming there is an
interest.



Re: [RFC] x86: Speculative execution warnings

2019-05-14 Thread Andy Lutomirski



On May 14, 2019, at 10:00 AM, Nadav Amit  wrote:

>> On May 14, 2019, at 1:00 AM, Paul Turner  wrote:
>> 
>> From: Nadav Amit 
>> Date: Fri, May 10, 2019 at 7:45 PM
>> To: 
>> Cc: Borislav Petkov, , Nadav Amit, Andy
>> Lutomirsky, Ingo Molnar, Peter Zijlstra, Thomas Gleixner, Jann Horn
>> 
>>> It may be useful to check in runtime whether certain assertions are
>>> violated even during speculative execution. This can allow to avoid
>>> adding unnecessary memory fences and at the same time check that no data
>>> leak channels exist.
>>> 
>>> For example, adding such checks can show that allocating zeroed pages
>>> can return speculatively non-zeroed pages (the first qword is not
>>> zero).  [This might be a problem when the page-fault handler performs
>>> software page-walk, for example.]
>>> 
>>> Introduce SPEC_WARN_ON(), which checks in runtime whether a certain
>>> condition is violated during speculative execution. The condition should
>>> be computed without branches, e.g., using bitwise operators. The check
>>> will wait for the condition to be realized (i.e., not speculated), and
>>> if the assertion is violated, a warning will be thrown.
>>> 
>>> Warnings can be provided in one of two modes: precise and imprecise.
>>> Both mode are not perfect. The precise mode does not always make it easy
>>> to understand which assertion was broken, but instead points to a point
>>> in the execution somewhere around the point in which the assertion was
>>> violated.  In addition, it prints a warning for each violation (unlike
>>> WARN_ONCE() like behavior).
>>> 
>>> The imprecise mode, on the other hand, can sometimes throw the wrong
>>> indication, specifically if the control flow has changed between the
>>> speculative execution and the actual one. Note that it is not a
>>> false-positive, it just means that the output would mislead the user to
>>> think the wrong assertion was broken.
>>> 
>>> There are some more limitations. Since the mechanism requires an
>>> indirect branch, it should not be used in production systems that are
>>> susceptible for Spectre v2. The mechanism requires TSX and performance
>>> counters that are only available in skylake+. There is a hidden
>>> assumption that TSX is not used in the kernel for anything else, other
>>> than this mechanism.
>> 
>> Nice trick!
> 
> “Illusion." [ ignore if you don’t know the reference ]
> 
>> 
>> Can you eliminate the indirect call by forcing an access fault to
>> abort the transaction instead, e.g. "cmove 0, $1”?
>> 
>> (If this works, it may also allow support on older architectures as
>> the RTM_RETIRED.ABORT* events go back further I believe?)
> 
> I don’t think it would work. The whole problem is that we need a counter
> that is updated during execution and not retirement. I tried several
> counters and could not find other appropriate ones.
> 
> The idea behind the implementation is to affect the control flow through
> data dependency. I may be able to do something similar without an indirect
> branch. I’ll take a page, put the XABORT on the page and make the page NX.
> Then, a direct jump would go to this page. The conditional-mov would change
> the PTE to X if the assertion is violated. There should be a page-walk even
> if the CPU finds the entry in the TLB, since this entry is NX.
> 

I think you only get a page walk if the TLB entry is not-present.  I’d be a bit 
surprised if the CPU is willing to execute, even speculatively, from 
speculatively written data. Good luck!

Re: [RFC] x86: Speculative execution warnings

2019-05-14 Thread Nadav Amit
> On May 14, 2019, at 1:00 AM, Paul Turner  wrote:
> 
> From: Nadav Amit 
> Date: Fri, May 10, 2019 at 7:45 PM
> To: 
> Cc: Borislav Petkov, , Nadav Amit, Andy
> Lutomirsky, Ingo Molnar, Peter Zijlstra, Thomas Gleixner, Jann Horn
> 
>> It may be useful to check in runtime whether certain assertions are
>> violated even during speculative execution. This can allow to avoid
>> adding unnecessary memory fences and at the same time check that no data
>> leak channels exist.
>> 
>> For example, adding such checks can show that allocating zeroed pages
>> can return speculatively non-zeroed pages (the first qword is not
>> zero).  [This might be a problem when the page-fault handler performs
>> software page-walk, for example.]
>> 
>> Introduce SPEC_WARN_ON(), which checks in runtime whether a certain
>> condition is violated during speculative execution. The condition should
>> be computed without branches, e.g., using bitwise operators. The check
>> will wait for the condition to be realized (i.e., not speculated), and
>> if the assertion is violated, a warning will be thrown.
>> 
>> Warnings can be provided in one of two modes: precise and imprecise.
>> Both mode are not perfect. The precise mode does not always make it easy
>> to understand which assertion was broken, but instead points to a point
>> in the execution somewhere around the point in which the assertion was
>> violated.  In addition, it prints a warning for each violation (unlike
>> WARN_ONCE() like behavior).
>> 
>> The imprecise mode, on the other hand, can sometimes throw the wrong
>> indication, specifically if the control flow has changed between the
>> speculative execution and the actual one. Note that it is not a
>> false-positive, it just means that the output would mislead the user to
>> think the wrong assertion was broken.
>> 
>> There are some more limitations. Since the mechanism requires an
>> indirect branch, it should not be used in production systems that are
>> susceptible for Spectre v2. The mechanism requires TSX and performance
>> counters that are only available in skylake+. There is a hidden
>> assumption that TSX is not used in the kernel for anything else, other
>> than this mechanism.
> 
> Nice trick!

“Illusion." [ ignore if you don’t know the reference ]

> 
> Can you eliminate the indirect call by forcing an access fault to
> abort the transaction instead, e.g. "cmove 0, $1”?
> 
> (If this works, it may also allow support on older architectures as
> the RTM_RETIRED.ABORT* events go back further I believe?)

I don’t think it would work. The whole problem is that we need a counter
that is updated during execution and not retirement. I tried several
counters and could not find other appropriate ones.

The idea behind the implementation is to affect the control flow through
data dependency. I may be able to do something similar without an indirect
branch. I’ll take a page, put the XABORT on the page and make the page NX.
Then, a direct jump would go to this page. The conditional-mov would change
the PTE to X if the assertion is violated. There should be a page-walk even
if the CPU finds the entry in the TLB, since this entry is NX.

The only problem is that this would be more expensive when the assertion is
not broken. It might also lead to alerts on the wrong code (i.e., one
assertion is violated, the other one fires).

I’ll give some more thoughts and experiments.

>> The basic idea behind the implementation is to use a performance counter
>> that updates also during speculative execution as an indication for
>> assertion failure. By using conditional-mov, which is not predicted,
>> to affect the control flow, the condition is realized before the event
>> that affects the PMU is triggered.
>> 
>> Enable this feature by setting "spec_warn=on" or "spec_warn=precise"
>> kernel parameter. I did not run performance numbers but I guess the
>> overhead should not be too high.
>> 
>> I did not run too many tests, but brief experiments suggest that it does
>> work. Let me know if I missed anything and whether you think this can be
>> useful. To be frank, the exact use cases are not super clear, and there
>> are various possible extensions (e.g., ensuring the speculation window
>> is long enough by adding data dependencies). I would appreciate your
>> inputs.
>> 
>> Cc: Andy Lutomirsky 
>> Cc: Ingo Molnar 
>> Cc: Peter Zijlstra 
>> Cc: Thomas Gleixner 
>> Cc: Jann Horn 
>> Signed-off-by: Nadav Amit 
>> ---
>> arch/x86/Kconfig |   4 +
>> arch/x86/include/asm/nospec-branch.h |  30 +
>> arch/x86/kernel/Makefile |   1 +
>> arch/x86/kernel/nospec.c | 185 +++
>> 4 files changed, 220 insertions(+)
>> create mode 100644 arch/x86/kernel/nospec.c
>> 
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index 62fc3fda1a05..2cc57c2172be 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -2887,6 +2887,10 @@ config X86_DMA_REMAP
>> config HAVE_GENERIC_GUP

Re: [RFC] x86: Speculative execution warnings

2019-05-14 Thread Paul Turner
From: Nadav Amit 
Date: Fri, May 10, 2019 at 7:45 PM
To: 
Cc: Borislav Petkov, , Nadav Amit, Andy
Lutomirsky, Ingo Molnar, Peter Zijlstra, Thomas Gleixner, Jann Horn

> It may be useful to check in runtime whether certain assertions are
> violated even during speculative execution. This can allow to avoid
> adding unnecessary memory fences and at the same time check that no data
> leak channels exist.
>
> For example, adding such checks can show that allocating zeroed pages
> can return speculatively non-zeroed pages (the first qword is not
> zero).  [This might be a problem when the page-fault handler performs
> software page-walk, for example.]
>
> Introduce SPEC_WARN_ON(), which checks in runtime whether a certain
> condition is violated during speculative execution. The condition should
> be computed without branches, e.g., using bitwise operators. The check
> will wait for the condition to be realized (i.e., not speculated), and
> if the assertion is violated, a warning will be thrown.
>
> Warnings can be provided in one of two modes: precise and imprecise.
> Both mode are not perfect. The precise mode does not always make it easy
> to understand which assertion was broken, but instead points to a point
> in the execution somewhere around the point in which the assertion was
> violated.  In addition, it prints a warning for each violation (unlike
> WARN_ONCE() like behavior).
>
> The imprecise mode, on the other hand, can sometimes throw the wrong
> indication, specifically if the control flow has changed between the
> speculative execution and the actual one. Note that it is not a
> false-positive, it just means that the output would mislead the user to
> think the wrong assertion was broken.
>
> There are some more limitations. Since the mechanism requires an
> indirect branch, it should not be used in production systems that are
> susceptible for Spectre v2. The mechanism requires TSX and performance
> counters that are only available in skylake+. There is a hidden
> assumption that TSX is not used in the kernel for anything else, other
> than this mechanism.
>

Nice trick!

Can you eliminate the indirect call by forcing an access fault to
abort the transaction instead, e.g. "cmove 0, $1"?

(If this works, it may also allow support on older architectures as
the RTM_RETIRED.ABORT* events go back further I believe?)

> The basic idea behind the implementation is to use a performance counter
> that updates also during speculative execution as an indication for
> assertion failure. By using conditional-mov, which is not predicted,
> to affect the control flow, the condition is realized before the event
> that affects the PMU is triggered.
>
> Enable this feature by setting "spec_warn=on" or "spec_warn=precise"
> kernel parameter. I did not run performance numbers but I guess the
> overhead should not be too high.
>
> I did not run too many tests, but brief experiments suggest that it does
> work. Let me know if I missed anything and whether you think this can be
> useful. To be frank, the exact use cases are not super clear, and there
> are various possible extensions (e.g., ensuring the speculation window
> is long enough by adding data dependencies). I would appreciate your
> inputs.
>
> Cc: Andy Lutomirsky 
> Cc: Ingo Molnar 
> Cc: Peter Zijlstra 
> Cc: Thomas Gleixner 
> Cc: Jann Horn 
> Signed-off-by: Nadav Amit 
> ---
>  arch/x86/Kconfig |   4 +
>  arch/x86/include/asm/nospec-branch.h |  30 +
>  arch/x86/kernel/Makefile |   1 +
>  arch/x86/kernel/nospec.c | 185 +++
>  4 files changed, 220 insertions(+)
>  create mode 100644 arch/x86/kernel/nospec.c
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 62fc3fda1a05..2cc57c2172be 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2887,6 +2887,10 @@ config X86_DMA_REMAP
>  config HAVE_GENERIC_GUP
> def_bool y
>
> +config DEBUG_SPECULATIVE_EXECUTION
> +   bool "Debug speculative execution"
> +   depends on X86_64
> +
>  source "drivers/firmware/Kconfig"
>
>  source "arch/x86/kvm/Kconfig"
> diff --git a/arch/x86/include/asm/nospec-branch.h 
> b/arch/x86/include/asm/nospec-branch.h
> index dad12b767ba0..3f1af6378304 100644
> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -290,6 +290,36 @@ static inline void 
> indirect_branch_prediction_barrier(void)
>  /* The Intel SPEC CTRL MSR base value cache */
>  extern u64 x86_spec_ctrl_base;
>
> +#ifdef CONFIG_DEBUG_SPECULATIVE_EXECUTION
> +
> +extern bool spec_check(unsigned long cond);
> +
> +DECLARE_STATIC_KEY_FALSE(spec_test_key);
> +DECLARE_STATIC_KEY_FALSE(spec_test_precise_key);
> +
> +#define SPEC_WARN_ON(cond) \
> +do {   \
> +   bool _error;\
> + 

Re: [RFC] x86: Speculative execution warnings

2019-05-10 Thread Randy Dunlap
On 5/10/19 12:25 PM, Nadav Amit wrote:
> It may be useful to check in runtime whether certain assertions are
> violated even during speculative execution. This can allow to avoid
> adding unnecessary memory fences and at the same time check that no data
> leak channels exist.
> 
> For example, adding such checks can show that allocating zeroed pages
> can return speculatively non-zeroed pages (the first qword is not
> zero).  [This might be a problem when the page-fault handler performs
> software page-walk, for example.]
> 
> Introduce SPEC_WARN_ON(), which checks in runtime whether a certain
> condition is violated during speculative execution. The condition should
> be computed without branches, e.g., using bitwise operators. The check
> will wait for the condition to be realized (i.e., not speculated), and
> if the assertion is violated, a warning will be thrown.
> 
> Warnings can be provided in one of two modes: precise and imprecise.
> Both mode are not perfect. The precise mode does not always make it easy
> to understand which assertion was broken, but instead points to a point
> in the execution somewhere around the point in which the assertion was
> violated.  In addition, it prints a warning for each violation (unlike
> WARN_ONCE() like behavior).
> 
> The imprecise mode, on the other hand, can sometimes throw the wrong
> indication, specifically if the control flow has changed between the
> speculative execution and the actual one. Note that it is not a
> false-positive, it just means that the output would mislead the user to
> think the wrong assertion was broken.
> 
> There are some more limitations. Since the mechanism requires an
> indirect branch, it should not be used in production systems that are
> susceptible for Spectre v2. The mechanism requires TSX and performance
> counters that are only available in skylake+. There is a hidden
> assumption that TSX is not used in the kernel for anything else, other
> than this mechanism.
> 
> The basic idea behind the implementation is to use a performance counter
> that updates also during speculative execution as an indication for
> assertion failure. By using conditional-mov, which is not predicted,
> to affect the control flow, the condition is realized before the event
> that affects the PMU is triggered.
> 
> Enable this feature by setting "spec_warn=on" or "spec_warn=precise"
> kernel parameter. I did not run performance numbers but I guess the
> overhead should not be too high.

Hi,
If this progresses, please document spec_warn={on|precise} in
Documentation/admin-guide/kernel-parameters.txt.

> I did not run too many tests, but brief experiments suggest that it does
> work. Let me know if I missed anything and whether you think this can be
> useful. To be frank, the exact use cases are not super clear, and there
> are various possible extensions (e.g., ensuring the speculation window
> is long enough by adding data dependencies). I would appreciate your
> inputs.
> 
> Cc: Andy Lutomirsky 
> Cc: Ingo Molnar 
> Cc: Peter Zijlstra 
> Cc: Thomas Gleixner 
> Cc: Jann Horn 
> Signed-off-by: Nadav Amit 
> ---
>  arch/x86/Kconfig |   4 +
>  arch/x86/include/asm/nospec-branch.h |  30 +
>  arch/x86/kernel/Makefile |   1 +
>  arch/x86/kernel/nospec.c | 185 +++
>  4 files changed, 220 insertions(+)
>  create mode 100644 arch/x86/kernel/nospec.c

thanks.
-- 
~Randy


[RFC] x86: Speculative execution warnings

2019-05-10 Thread Nadav Amit
It may be useful to check in runtime whether certain assertions are
violated even during speculative execution. This can allow to avoid
adding unnecessary memory fences and at the same time check that no data
leak channels exist.

For example, adding such checks can show that allocating zeroed pages
can return speculatively non-zeroed pages (the first qword is not
zero).  [This might be a problem when the page-fault handler performs
software page-walk, for example.]

Introduce SPEC_WARN_ON(), which checks in runtime whether a certain
condition is violated during speculative execution. The condition should
be computed without branches, e.g., using bitwise operators. The check
will wait for the condition to be realized (i.e., not speculated), and
if the assertion is violated, a warning will be thrown.

Warnings can be provided in one of two modes: precise and imprecise.
Both mode are not perfect. The precise mode does not always make it easy
to understand which assertion was broken, but instead points to a point
in the execution somewhere around the point in which the assertion was
violated.  In addition, it prints a warning for each violation (unlike
WARN_ONCE() like behavior).

The imprecise mode, on the other hand, can sometimes throw the wrong
indication, specifically if the control flow has changed between the
speculative execution and the actual one. Note that it is not a
false-positive, it just means that the output would mislead the user to
think the wrong assertion was broken.

There are some more limitations. Since the mechanism requires an
indirect branch, it should not be used in production systems that are
susceptible for Spectre v2. The mechanism requires TSX and performance
counters that are only available in skylake+. There is a hidden
assumption that TSX is not used in the kernel for anything else, other
than this mechanism.

The basic idea behind the implementation is to use a performance counter
that updates also during speculative execution as an indication for
assertion failure. By using conditional-mov, which is not predicted,
to affect the control flow, the condition is realized before the event
that affects the PMU is triggered.

Enable this feature by setting "spec_warn=on" or "spec_warn=precise"
kernel parameter. I did not run performance numbers but I guess the
overhead should not be too high.

I did not run too many tests, but brief experiments suggest that it does
work. Let me know if I missed anything and whether you think this can be
useful. To be frank, the exact use cases are not super clear, and there
are various possible extensions (e.g., ensuring the speculation window
is long enough by adding data dependencies). I would appreciate your
inputs.

Cc: Andy Lutomirsky 
Cc: Ingo Molnar 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Cc: Jann Horn 
Signed-off-by: Nadav Amit 
---
 arch/x86/Kconfig |   4 +
 arch/x86/include/asm/nospec-branch.h |  30 +
 arch/x86/kernel/Makefile |   1 +
 arch/x86/kernel/nospec.c | 185 +++
 4 files changed, 220 insertions(+)
 create mode 100644 arch/x86/kernel/nospec.c

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 62fc3fda1a05..2cc57c2172be 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2887,6 +2887,10 @@ config X86_DMA_REMAP
 config HAVE_GENERIC_GUP
def_bool y
 
+config DEBUG_SPECULATIVE_EXECUTION
+   bool "Debug speculative execution"
+   depends on X86_64
+
 source "drivers/firmware/Kconfig"
 
 source "arch/x86/kvm/Kconfig"
diff --git a/arch/x86/include/asm/nospec-branch.h 
b/arch/x86/include/asm/nospec-branch.h
index dad12b767ba0..3f1af6378304 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -290,6 +290,36 @@ static inline void indirect_branch_prediction_barrier(void)
 /* The Intel SPEC CTRL MSR base value cache */
 extern u64 x86_spec_ctrl_base;
 
+#ifdef CONFIG_DEBUG_SPECULATIVE_EXECUTION
+
+extern bool spec_check(unsigned long cond);
+
+DECLARE_STATIC_KEY_FALSE(spec_test_key);
+DECLARE_STATIC_KEY_FALSE(spec_test_precise_key);
+
+#define SPEC_WARN_ON(cond) \
+do {   \
+   bool _error;\
+   \
+   if (!static_branch_unlikely(_test_key))\
+   break;  \
+   \
+   _error = spec_check((unsigned long)(cond)); \
+   \
+   if (static_branch_unlikely(_test_precise_key)) \
+   break;  \
+   \
+