Page MenuHomePhabricator

[RFC] Sceptre a Spectre variant 1 detector
Needs ReviewPublic

Authored by rob.lougher on Feb 22 2018, 12:42 PM.

Details

Reviewers
llvm-commits
Summary

Sceptre is an LLVM Utility pass to check a program at the IR level for Spectre variant 1 (bounds check bypass) vulnerabilities. The pass currently must be enabled with -mllvm -enable-sceptre. When it finds a vulnerability it outputs a diagnostic of the form:

warning: spectre.c:10:10: in function array1_load: found vulnerable load
note: inlined into function foo at: spectre.c:19:24
note: bounds check with index "index" is at: spectre.c:6:16: in function is_valid_idx
note: inlined into function foo at: spectre.c:18:6

As it runs after inlining it must be ran at "-O1" or above (recommendation is to run it at "-O2").

Note, as the pass is still under development I'm putting this up merely as an RFC (an email will be sent to llvm-dev linking to this). The pass has proved useful internally and I wanted to share it as soon as possible with the community.

Thanks,
Rob.

Robert Lougher
Sony Interactive Entertainment

Diff Detail

Event Timeline

rob.lougher created this revision.Feb 22 2018, 12:42 PM
rob.lougher edited the summary of this revision. (Show Details)Feb 22 2018, 12:43 PM
rob.lougher edited the summary of this revision. (Show Details)Feb 22 2018, 12:46 PM

Is the pass intentionally named "Sceptre"? That's amusing, but kind of confusing.

I'm a little wary of giving people a false sense of security, since this checker only looks for a very narrow set of patterns. (See discussion on https://reviews.llvm.org/D41761 .)

lib/Analysis/Sceptre.cpp
157

PHI nodes always have at least one incoming value.

I'm a little wary of giving people a false sense of security, since this checker only looks for a very narrow set of patterns. (See discussion on https://reviews.llvm.org/D41761 .)

I understand the "leave no door unlocked" argument, although I also think the argument fails to consider that some doors are *way* harder to reach than others and these practical barriers are effectively locks of their own. Nevertheless it's best to find as many as we can.

In his llvm-dev post, Rob says the checker can detect 9 of the 15 examples published by Paul Kocher (https://www.paulkocher.com/doc/MicrosoftCompilerSpectreMitigation.html). Do you have additional patterns beyond that set, that you think we should look for? Do you have suggestions for how to detect the patterns we don't find now? This *is* a WIP.

I think the focus on array bounds checks is overly narrow. You can get an almost identical effect in other ways. For example, if you use C++ inheritance, a member of an object might be a user-controlled integer on one path, and a pointer on a different path. Or if you have an array of pointers, you could read past the end of the array into uninintialized data.

You could write an analysis which is more comprehensive by analyzing every load, I think, rather than trying to figure out which loads are related to branches. A pointer to some known object is "safe", a constant offset from a "safe" pointer is also "safe", a SCEV AddRec whose base is a "safe" pointer is also "safe", etc., and then warn about any pointers the analysis can't prove safe. The tricky part of this would be handling pointers to pointers; not sure what sort of heuristic you would use.

This is cut and pasted from an email reply as it's not showing up after over 2 hours...

We went with this approach because we were trying to catch the most likely vulnerabilities while also trying to limit the number of false positives. As speculative execution of a branch is fundamental to the Spectre exploit, linking the loads to conditional branches was an obvious way to try and limit the number of possibilities. We've worked closely with our internal users to come up with something that meets their requirements, and as a side effect decided to share our implementation with the wider LLVM community. Our solution seems to match fairly closely that which we've observed to mitigate Variant 1 in the Linux Kernel [1] and by other compiler vendors such as Microsoft [2]. It's true that there is a danger of introducing a false sense of security. But the fact you can't guarantee to find all the vulnerabilies doesn't mean you do nothing. To echo Paul, you don't leave your house unlocked because you can't make it 100% burglar-proof!

[1] https://lwn.net/Articles/743265/

[2] https://blogs.msdn.microsoft.com/vcblog/2018/01/15/spectre-mitigations-in-msvc/

emaste added a subscriber: emaste.Feb 27 2018, 10:21 AM

Is the pass intentionally named "Sceptre"? That's amusing, but kind of confusing.

I agree, the name is amusing but I worry about the similarity between spectre and sceptre.

include/llvm/Analysis/Passes.h
101–102

Please reference CVE-2017-5753 in important comments along with "Spectre variant 1"