[PATCH] D41723: Introduce the "retpoline" x86 mitigation technique for variant #2 of the speculative execution vulnerabilities disclosed today, specifically identified by CVE-2017-5715, "Branch Target Injection", and is one of the two halves to Spectre..

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

In https://reviews.llvm.org/D41723#968179, @royger wrote:

> In https://reviews.llvm.org/D41723#967987, @chandlerc wrote:
>
> > Add support for externally provided thunks. This is an independent feature;
> >  when combined with the overall retpoline feature it suppresses the thunk
> >  emission and rotates the names to be distinct names that an external build
> >  system for the kernel (for example) can provide.
> >
> > I've added some minimal documentation about the semantic requirements of these
> >  thunks to the commit log, although it is fairly obvious. More comprehensive
> >  documentation will be part of the large follow-up effort around docs.
>
>
> Thanks! I'm however not seeing the updated commit message that contains the usage documentation of the new option in the differential revision.


Yeah, not sure why phab does't update that -- it will be updated when it lands.

> AFAICT from the code, the new option is going to be named "mretpoline_external_thunk", could we have a more generic name that could be used by all options, like:
> 
> -mindirect-thunk={retpoline,external}
> 
> This should also allow clang to implement new techniques as they become available, without having to add new options for each one of them.

Adding options is actually cheaper than adding option parsing like this...

I'm hesitant to name the option in the external case something completely generic, because it isn't as generic as it seems IMO. We hide indirect calls and indirect branches, but we *don't* hide returns which are technically still indirects... The reason is that we are assuming that rewriting arbitrary indirects into a return is an effective mitigation. If it weren't, we'd also need to rewrite returns. This is a large component of the discussion in the linked detailed article around RSB filling -- we have to use some *other* mechanisms to defend ret, and then we rely on them being "safe".

As a consequence to all of this, this flag to clang shouldn't be used for *arbitrary* mitigations. It really should be used for mitigations stemming from return-based trampolining of indirect branches.

If we want truly generic thunk support in the future, we should add that under a separate flag name, and it should likely apply to returns as well as other forms of indirects. I know there has been some discussion about using `lfence` to protect indirect branch and call on AMD processors, but I'm still extremely dubious about this being an advisable mitigation. I have *significantly* more confidence in the retpoline technique being a truly effective mitigation against the attacks, and the performance we observe in the wild really is completely acceptable (for the Linux kernel, hard to even measure outside of contrived micro-benchmarks).


https://reviews.llvm.org/D41723