<div dir="ltr"><div>On Fri, Jan 5, 2018 at 11:54 AM, Rafael Avila de Espindola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br></div><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Chandler Carruth via Phabricator via llvm-commits<br>
<span class=""><<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> writes:<br>
<br>
> The goal is simple: avoid generating code which contains an indirect<br>
> branch that could have its prediction poisoned by an attacker. In many<br>
> cases, the compiler can simply use directed conditional branches and<br>
> a small search tree. LLVM already has support for lowering switches in<br>
> this way and the first step of this patch is to disable jump-table<br>
> lowering of switches and introduce a pass to rewrite explicit indirectbr<br>
> sequences into a switch over integers.<br>
<br>
</span>Amusingly that is what clang would do before r86256.<br>
<span class=""><br>
> There is one other important source of indirect branches in x86 ELF<br>
> binaries: the PLT. These patches also include support for LLD to<br>
> generate PLT entries that perform a retpoline-style indirection.<br>
<br>
</span>I see that we now generate similar code for -fno-plt. We should have a<br>
test for that (nonlazybind at the llvm level).<br>
<span class=""><br>
> The only other indirect branches remaining that we are aware of are from<br>
> precompiled runtimes (such as crt0.o and similar). The ones we have<br>
> found are not really attackable, and so we have not focused on them<br>
> here, but eventually these runtimes should also be replicated for<br>
> retpoline-ed configurations for completeness.<br>
><br>
> For kernels or other freestanding or fully static executables, the<br>
> compiler switch `-mretpoline` is sufficient to fully mitigate this<br>
> particular attack. For dynamic executables, you must compile *all*<br>
> libraries with `-mretpoline` and additionally link the dynamic<br>
> executable and all shared libraries with LLD and pass `-z retpolineplt`<br>
> (or use similar functionality from some other linker). We strongly<br>
> recommend also using `-z now` as non-lazy binding allows the<br>
> retpoline-mitigated PLT to be substantially smaller.<br>
<br>
</span>Since we have to rebuild everything and lazy binding is more expensive,<br>
-fno-plt should be the best mitigation, no?<br>
<span class=""><br>
> +Function *X86RetpolineThunks::<wbr>createThunkFunction(Module &M, StringRef Name) {<br>
</span>> + LLVMContext &Ctx = M.getContext();<br>
> + auto Type = FunctionType::get(Type::<wbr>getVoidTy(Ctx), false);<br>
> + Function *F =<br>
> + Function::Create(Type, GlobalValue::<wbr>LinkOnceODRLinkage, Name, &M);<br>
> + F->setVisibility(GlobalValue::<wbr>HiddenVisibility);<br>
<br>
We should probably put this in a comdat so the linker keeps only one.<br>
<br>
> Index: llvm/lib/CodeGen/<wbr>IndirectBrExpandPass.cpp<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- /dev/null<br>
> +++ llvm/lib/CodeGen/<wbr>IndirectBrExpandPass.cpp<br>
> @@ -0,0 +1,187 @@<br>
> +//===- IndirectBrExpandPass.cpp - Expand indirectbr to switch -------------===//<br>
> +//<br>
> +// The LLVM Compiler Infrastructure<br>
> +//<br>
> +// This file is distributed under the University of Illinois Open Source<br>
> +// License. See LICENSE.TXT for details.<br>
> +//<br>
> +//===------------------------<wbr>------------------------------<wbr>----------------===//<br>
> +/// \file<br>
> +///<br>
> +/// Implements an expansion pass to turn `indirectbr` instructions in the IR<br>
> +/// into `switch` instructions. This works by enumerating the basic blocks in<br>
> +/// a dense range of integers, replacing each `blockaddr` constant with the<br>
> +/// corresponding integer constant, and then building a switch that maps from<br>
> +/// the integers to the actual blocks.<br>
> +///<br>
> +/// While this is generically useful if a target is unable to codegen<br>
> +/// `indirectbr` natively, it is primarily useful when there is some desire to<br>
> +/// get the builtin non-jump-table lowering of a switch even when the input<br>
> +/// source contained an explicit indirect branch construct.<br>
<br>
This pass converts each indirectbr to a switch. One feature we had in<br>
the old clang expansion of computed gotos is that each function would<br>
have a single switch. This would avoid code explosion in direct threaded<br>
interpreters where after each opcode there is a jump that can go any<br>
other opcode.<br>
<br>
> Index: lld/test/ELF/x86-64-retpoline.<wbr>s<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- /dev/null<br>
> +++ lld/test/ELF/x86-64-retpoline.<wbr>s<br>
> @@ -0,0 +1,51 @@<br>
> +// REQUIRES: x86<br>
> +// RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t1.o<br>
> +// RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/shared.s -o %t2.o<br>
> +// RUN: ld.lld -shared %t2.o -o %t2.so<br>
> +<br>
> +// RUN: ld.lld -shared %t1.o %t2.so -o %t.exe -z retpolineplt<br>
> +// RUN: llvm-objdump -d %t.exe | FileCheck %s<br>
<br>
Please add a CHECK line before the following ones to show what section<br>
is being disassembled.<br>
<br>
You also want to check the address of the .got.plt to show what some of<br>
the magic numbers in the CHECK are.<br>
<br>
The same issue is present in other tests.<br></blockquote><div><br></div><div>Will do.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> +template <class ELFT><br>
> +void Retpoline<ELFT>::writeGotPlt(<wbr>uint8_t *Buf, const Symbol &S) const {<br>
> + write32le(Buf, S.getPltVA() + 21);<br>
<br>
Add a comment about where the 21 is from.<br></blockquote><div><br></div><div>It's now 16, and I think the comment isn't needed here because it would be a copy-n-paste of the previous comment for X86<ELFT>::writeGotPlt.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
> +template <class ELFT> void Retpoline<ELFT>::<wbr>writePltHeader(uint8_t *Buf) const {<br>
> + const uint8_t Insn[] = {<br>
> + 0xff, 0x35, 0, 0, 0, 0, // pushq GOTPLT+8(%rip)<br>
> + 0x4c, 0x8b, 0x1d, 0, 0, 0, 0, // mov GOTPLT+16(%rip), %r11<br>
> + 0xe8, 0x0e, 0x00, 0x00, 0x00, // callq next<br>
<span class="">> + 0xf3, 0x90, // loop: pause<br>
> + 0xeb, 0xfc, // jmp loop<br>
</span>> + 0x0f, 0x1f, 0x44, 0x00, 0x00, // nop<br>
> + 0x0f, 0x1f, 0x44, 0x00, 0x00, // nop; .align 16<br>
<br>
The nops are never executed, right? Could we use traps?<br></blockquote><div><br></div><div>Will use int3 instead of nop.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
> +template <class ELFT><br>
> +void Retpoline<ELFT>::writePlt(<wbr>uint8_t *Buf, uint64_t GotPltEntryAddr,<br>
> + uint64_t PltEntryAddr, int32_t Index,<br>
> + unsigned RelOff) const {<br>
> + const uint8_t Insn[] = {<br>
<span class="">> + 0x4c, 0x8b, 0x1d, 0, 0, 0, 0, // mov foo@GOTPLT(%rip), %r11<br>
> + 0xe8, 0x04, 0x00, 0x00, 0x00, // callq next<br>
> + 0xf3, 0x90, // loop: pause<br>
> + 0xeb, 0xfc, // jmp loop; .align 16<br>
> + 0x4c, 0x89, 0x1c, 0x24, // next: mov %r11, (%rsp)<br>
> + 0xc3, // ret<br>
<br>
</span>This code sequence (mov + ret) exists in the header. Can't we jump there<br>
and reduce the size a bit?<br>
<br>
> +void RetpolinePic::writeGotPlt(<wbr>uint8_t *Buf, const Symbol &S) const {<br>
> + write32le(Buf, S.getPltVA() + 16);<br>
<br>
Add a comment saying why 16.<br>
<br>
Cheers,<br>
Rafael<br>
</blockquote></div><br></div></div>