llvm dev - Jul 2018 - RFC: Speculative Load Hardening (a Spectre variant #1 mitigation)

FYI to all: I've updated the design document to include the newly disclosed
variants 1.1 and 1.2 (collectively called Bounds Check Bypass Store or
BCBS).

There is no change to the proposed implementation which can already
robustly mitigate these variants.

I've also updated my patch as we have very significant interest in getting
at least an early "beta" version of this into the tree and available
for
experiments right away. Would really appreciate folks making review
comments ASAP and bearing with us and tolerating some amount of post-commit
iteration here.

Notably:

On Mon, Jul 9, 2018 at 5:03 AM Kristof Beyls <Kristof.Beyls at arm.com>
wrote:
> Hi Chandler,
>
> I've just uploaded a sequence of patches that implement a similar
> technique for
> AArch64.
>
This is awesome. =D I can't wait to start wiring this together.

> A small difference of approach is that I went for introducing an intrinsic
> that
> can make any integer or pointer value "speculation-safe", i.e.
the
> intrinsic
> returns the value of its only parameter when correctly speculating, and
> returns
> 0 when miss-speculating.
> The intrinsic is close to what Philip Reames suggested on
> https://reviews.llvm.org/D41761.
>
Cool, we'll definitely need *some* intrinsic in the IR to help model source
annotations. I still need to think a bit about the interface and model for
this...

>
> Then a later patch (D49072) adds automatic mitigation by inserting the
> intrinsic
> in necessary locations.
>
I was never able to get automatic mitigation with an intrinsic to avoid
really signiifcant performance problems in the x86 backend. I'll look
through your approach to see if you figured out a technique that works
better than the ones I tried here....

> I believe this approach has the advantage that:
> a) it makes it possible to only insert a mitigation in specific locations
> if
>    the programmer is capable of inserting intrinsics manually.
>
This is definitely an area of great interest long-term.

> b) it becomes easier to explore different options for implementing
> automatic
>    protection - it's just a matter of writing different ways on how the
>    intrinsic is injected into the program. See D49072 for how this is
> relatively
>    easy.
>
As above, I actually tried this and it backfired in terms of code quality.
I'll definitely look at this and either try to explain the problem I hit or
if you've dodged nicely, we can rework things to move in this direction.

> I've split the patches according to the following functionality:
> - LLVM: https://reviews.llvm.org/D49069: Introduce control flow
> speculation tracking for AArch64.
> - LLVM: https://reviews.llvm.org/D49070: Introduce
> llvm.speculation_safe_value intrinsic.
> - LLVM: https://reviews.llvm.org/D49071: Enable lowering of
> llvm.speculation_safe_value to DSB/ISB pair.
> - LLVM: https://reviews.llvm.org/D49072: Enable automatic mitigation
> against control flow speculation.
> - Clang: https://reviews.llvm.org/D49073: Introducing
> __builtin_speculation_safe_value.
>
> I'll be on a long holiday soon, so there may be delays to me reacting
on
> review feedback.
>
Sure. Given the sudden but very strong interest we have from some users,
I'm going to try and make progress landing at least the initial version of
the x86 stuff. But I very much want to iterate on it and get it and the
AArch64 stuff you've got here to line up and work together. I really like
the overall direction.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180711/6b9e8913/attachment.html>

On 7/11/2018 4:02 AM, Chandler Carruth via llvm-dev
wrote:
>
>     I believe this approach has the advantage that:
>     a) it makes it possible to only insert a mitigation in specific
>     locations if
>        the programmer is capable of inserting intrinsics manually.
>
>
> This is definitely an area of great interest long-term.
Annotating specific loads that need to be protected seems like a trap to 
me.  See https://reviews.llvm.org/D41761#989799 .  (And Bounds Check 
Bypass Store variants open up other possibilities, like overwriting a 
spill slot.)

Maybe we can come up with some workable approach to "whitelist"
certain
pointers: a pointer could be marked "speculatively-dereferenceable(N)"
if it points to N bytes of non-secret data.  (We could apply this as 
load metadata, like !dereferenceable, or it could be explicitly applied 
using an intrinsic.)

-Eli

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
Foundation Collaborative Project

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180711/1f2ef831/attachment.html>

On 11 Jul 2018, at 13:02, Chandler Carruth <chandlerc at
google.com<mailto:chandlerc at google.com>> wrote:

FYI to all: I've updated the design document to include the newly disclosed
variants 1.1 and 1.2 (collectively called Bounds Check Bypass Store or BCBS).

There is no change to the proposed implementation which can already robustly
mitigate these variants.

I've also updated my patch as we have very significant interest in getting
at least an early "beta" version of this into the tree and available
for experiments right away. Would really appreciate folks making review comments
ASAP and bearing with us and tolerating some amount of post-commit iteration
here.

Notably:

On Mon, Jul 9, 2018 at 5:03 AM Kristof Beyls <Kristof.Beyls at
arm.com<mailto:Kristof.Beyls at arm.com>> wrote:
Hi Chandler,

I've just uploaded a sequence of patches that implement a similar technique
for
AArch64.

This is awesome. =D I can't wait to start wiring this together.

A small difference of approach is that I went for introducing an intrinsic that
can make any integer or pointer value "speculation-safe", i.e. the
intrinsic
returns the value of its only parameter when correctly speculating, and returns
0 when miss-speculating.
The intrinsic is close to what Philip Reames suggested on
https://reviews.llvm.org/D41761.

Cool, we'll definitely need *some* intrinsic in the IR to help model source
annotations. I still need to think a bit about the interface and model for
this...


Then a later patch (D49072) adds automatic mitigation by inserting the intrinsic
in necessary locations.

I was never able to get automatic mitigation with an intrinsic to avoid really
signiifcant performance problems in the x86 backend. I'll look through your
approach to see if you figured out a technique that works better than the ones I
tried here….

My guess is that the key is that the automatic intrinsic insertion happens only
really really late in my implementation - i.e. in the same pass that also
inserts the speculation tracking.
One could say that there really isn’t that much difference between inserting an
intrinsic this way and just inserting instructions directly, since the intrinsic
that gets inserted gets lowered almost immediately by the same pass later on.
It still has the advantage that the automatic protection code remains simple and
the actual lowering from intrinsic to instruction sequences is shared between
the automatic mode and the user-inserted intrinsic mode.


I believe this approach has the advantage that:
a) it makes it possible to only insert a mitigation in specific locations if
   the programmer is capable of inserting intrinsics manually.

This is definitely an area of great interest long-term.

b) it becomes easier to explore different options for implementing automatic
   protection - it's just a matter of writing different ways on how the
   intrinsic is injected into the program. See D49072 for how this is relatively
   easy.

As above, I actually tried this and it backfired in terms of code quality.
I'll definitely look at this and either try to explain the problem I hit or
if you've dodged nicely, we can rework things to move in this direction.


I've split the patches according to the following functionality:
- LLVM: https://reviews.llvm.org/D49069: Introduce control flow speculation
tracking for AArch64.
- LLVM: https://reviews.llvm.org/D49070: Introduce llvm.speculation_safe_value
intrinsic.
- LLVM: https://reviews.llvm.org/D49071: Enable lowering of
llvm.speculation_safe_value to DSB/ISB pair.
- LLVM: https://reviews.llvm.org/D49072: Enable automatic mitigation against
control flow speculation.
- Clang: https://reviews.llvm.org/D49073: Introducing
__builtin_speculation_safe_value.

I'll be on a long holiday soon, so there may be delays to me reacting on
review feedback.

Sure. Given the sudden but very strong interest we have from some users, I'm
going to try and make progress landing at least the initial version of the x86
stuff. But I very much want to iterate on it and get it and the AArch64 stuff
you've got here to line up and work together. I really like the overall
direction.

It sounds great that there are beta testers eager to experiment with this
approach, so I agree that pressing on with committing something that they can
experiment with is the right approach here. Their feedback can only make the end
solution better.

Thanks!

Kristof

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180711/bd1dcb74/attachment.html>

I understand where the whitelist idea comes from, but if you are executing in a
protected domain you need to assume all data needs to be hidden.  The only
exception is something explicitly passed back across the domain boundary, per
specification of some API.  This is just a reality of system security.

I say this as someone who spent half of a long career in software security: 
People really excel at building insecure systems.  Giving them more tools to
make mistakes with is totally the wrong approach.  I went back to compilers
because dealing with those folks was giving me ulcers.

I hope this doesn't come across as harsh or anything, I have very strong
feelings on this and I'm not great at expressing it.
--paulr

From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] On Behalf Of
Friedman, Eli via llvm-dev
Sent: Wednesday, July 11, 2018 2:24 PM
To: Chandler Carruth; Kristof Beyls
Cc: llvm-dev; nd
Subject: Re: [llvm-dev] RFC: Speculative Load Hardening (a Spectre variant #1
mitigation)

On 7/11/2018 4:02 AM, Chandler Carruth via llvm-dev wrote:

I believe this approach has the advantage that:
a) it makes it possible to only insert a mitigation in specific locations if
   the programmer is capable of inserting intrinsics manually.

This is definitely an area of great interest long-term.

Annotating specific loads that need to be protected seems like a trap to me. 
See https://reviews.llvm.org/D41761#989799 .  (And Bounds Check Bypass Store
variants open up other possibilities, like overwriting a spill slot.)

Maybe we can come up with some workable approach to "whitelist"
certain pointers: a pointer could be marked
"speculatively-dereferenceable(N)" if it points to N bytes of
non-secret data.  (We could apply this as load metadata, like !dereferenceable,
or it could be explicitly applied using an intrinsic.)

-Eli


--

Employee of Qualcomm Innovation Center, Inc.

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
Foundation Collaborative Project
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180711/cff790f0/attachment.html>