[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 llvm-commits]

On Fri, Jan 5, 2018 at 12:14 PM Rafael Avila de Espindola <
rafael.espindola at gmail.com> wrote:

> Chandler Carruth <chandlerc at gmail.com> writes:
>
> >>
> >> > There is one other important source of indirect branches in x86 ELF
> >> > binaries: the PLT. These patches also include support for LLD to
> >> > generate PLT entries that perform a retpoline-style indirection.
> >>
> >> I see that we now generate similar code for -fno-plt. We should have a
> >> test for that (nonlazybind at the llvm level).
> >>
> >
> > ? The LLVM code generation for PLT-calls shouldn't be impacted by
> retpoline
> > at all? It should just be the contents of the PLT section itself? Maybe
> I'm
> > just misunderstanding...
> >
> > If you're wondering whether LLVM will correctly retpoline a -fno-plt call
> > that it directly emits, I believe it uses the same instruction patterns
> as
> > any other indirect call and so should be covered already.
>
> Yes, that is what I was wondering. It is not obvious for someone not
> current on the X86 backend if -fno-plt is implemented in the same code
> path. So the request is just to add a llvm test for -fno-plt (i.e., a
> test showing what nonlazybind codegens to now).
>
> Hopefully all that you need is a version of
> test/CodeGen/X86/fast-isel-noplt-pic.ll with
>
> +attributes #0 = { "target-features"="+retpoline" }
>

I directly added a nonlazybind call to the retpoline test file and checked
it both on 32-bit and 64-bit, and both SDISel and FastISel. Thanks for this!


>
> >> > +Function *X86RetpolineThunks::createThunkFunction(Module &M,
> StringRef
> >> Name) {
> >> > +  LLVMContext &Ctx = M.getContext();
> >> > +  auto Type = FunctionType::get(Type::getVoidTy(Ctx), false);
> >> > +  Function *F =
> >> > +      Function::Create(Type, GlobalValue::LinkOnceODRLinkage, Name,
> &M);
> >> > +  F->setVisibility(GlobalValue::HiddenVisibility);
> >>
> >> We should probably put this in a comdat so the linker keeps only one.
> >>
> >
> > Is LinkOnceODR not sufficient? We don't need a comdat *group* here. I
> > thought you didn't need explicit comdats unless you're grouping multiple
> > globals....
>
> It is not sufficient. LLVM used to create implicit comdats, but we don't
> do that anymore.
>
> Given
>
> define linkonce_odr void @foo() {
>   ret void
> }
>
> llc puts it in .text or
>
> .section        .text.foo,"ax", at progbits
>
> if -function-sections is used.
>
> Given
>
> $foo = comdat any
> define linkonce_odr void @foo() comdat {
>   ret void
> }
>
> We always produce
>
> .section        .text.foo,"axG", at progbits,foo,comdat
>

Sure, I've enhanced the tests to check for these sections and added the
comdat generation to the code.



>
>
> >> This pass converts each indirectbr to a switch. One feature we had in
> >> the old clang expansion of computed gotos is that each function would
> >> have a single switch. This would avoid code explosion in direct threaded
> >> interpreters where after each opcode there is a jump that can go any
> >> other opcode.
> >>
> >
> > I'll see if this is easy to implement, but if it's going to take some
> time
> > I might suggest we defer this to a follow-up patch.
> >
> >
> > I think Rui got the rest of your comments, but give a shout if I've
> missed
> > anything and thanks for review!
>
> A FIXME or bug report for future improvement is fine.
>

I'm playing with this, but if i don't get something working before an LGTM,
I'll go with just a FIXME.


Thanks again!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180106/4e3d5b02/attachment.html>