[llvm] 43f031d - Enable IBT(Indirect Branch Tracking) in JIT with CET(Control-flow Enforcement Technology)

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 6 13:11:13 PDT 2020


On 4/5/20 12:44 AM, Zhang, Xiang1 wrote:
> Hi Philip:
> 1,It is not fully by default.
>      it is enabling security mitigations(CET) in the JIT by default only when the JIT-self is compiled by CET enabled compiler and the JIT's target is X86 target.
Ok.
>     
>
> 2, I have send a patch to fix the test MCJIT/cet-code-model-lager.ll for the failure in other target.
>       7.Change #97321
>       Changed by Xiang1 Zhang <xiang1.zhang at intel.com>
>       Changed at Thu 02 Apr 2020 22:25:35
>       Repository https://github.com/llvm/llvm-project
>       Project llvm
>       Branch master
>       Revision fef2dab100dfc7c49ccf0ce2bacea409324b54ba
If this is failing build bots, please revert and reapply with the fix 
(after review if needed)
>   
>
> -----Original Message-----
> From: Philip Reames <listmail at philipreames.com>
> Sent: Saturday, April 4, 2020 12:18 AM
> To: Zhang, Xiang1 <xiang1.zhang at intel.com>; Xiang1 Zhang <llvmlistbot at llvm.org>; llvm-commits at lists.llvm.org
> Subject: Re: [llvm] 43f031d - Enable IBT(Indirect Branch Tracking) in JIT with CET(Control-flow Enforcement Technology)
>
> The description of this is a bit ambiguous and possibly concerning.  Is this enabling security mitigations in the JIT by default?  Or only when the JIT configuration explicitly opts in?
>
> If it's the former, please revert.  It is not acceptable to degrade the performance of all JIT users just because some may wish to enable a mitigation.  I'd even be willing to consider making it the default, but opt-out would be definitely required.
>
> Philip
>
> On 4/2/20 8:45 PM, Xiang1 Zhang via llvm-commits wrote:
>> Author: Xiang1 Zhang
>> Date: 2020-04-03T11:44:07+08:00
>> New Revision: 43f031d31264d20cfb8f1ebd606c66e57c231d4d
>>
>> URL:
>> https://github.com/llvm/llvm-project/commit/43f031d31264d20cfb8f1ebd60
>> 6c66e57c231d4d
>> DIFF:
>> https://github.com/llvm/llvm-project/commit/43f031d31264d20cfb8f1ebd60
>> 6c66e57c231d4d.diff
>>
>> LOG: Enable IBT(Indirect Branch Tracking) in JIT with CET(Control-flow
>> Enforcement Technology)
>>
>> Summary:
>> This patch comes from H.J.'s
>> https://github.com/hjl-tools/llvm-project/commit/2bd54ce7fa9e94fcd1118
>> b948e14d1b6fc54dfd2
>>
>> **This patch fix the failed llvm unit tests which running on CET
>> machine. **(e.g. ExecutionEngine/MCJIT/MCJITTests)
>>
>> The reason we enable IBT at "JIT compiled with CET" is mainly that:  the JIT don't know the its caller program is CET enable or not.
>> If JIT's caller program is non-CET, it is no problem JIT generate CET code or not.
>> But if JIT's caller program is CET enabled,  JIT must generate CET code or it will cause Control protection exceptions.
>>
>> I have test the patch at llvm-unit-test and llvm-test-suite at CET machine. It passed.
>> and H.J. also test it at building and running VNCserver(Virtual Network Console), it works too.
>> (if not apply this patch, VNCserver will crash at CET machine.)
>>
>> Reviewers: hjl.tools, craig.topper, LuoYuanke, annita.zhang, pengfei
>>
>> Subscribers: tstellar, efriedma, hiraditya, llvm-commits
>>
>> Tags: #llvm
>>
>> Differential Revision: https://reviews.llvm.org/D76900
>>
>> Added:
>>       llvm/test/CodeGen/X86/indirect-branch-tracking-cm-lager.ll
>>       llvm/test/ExecutionEngine/MCJIT/cet-code-model-lager.ll
>>
>> Modified:
>>       llvm/lib/Target/X86/X86IndirectBranchTracking.cpp
>>       llvm/lib/Target/X86/X86TargetMachine.cpp
>>       llvm/lib/Target/X86/X86TargetMachine.h
>>
>> Removed:
>>       
>>
>>
>> ######################################################################
>> ########## diff  --git
>> a/llvm/lib/Target/X86/X86IndirectBranchTracking.cpp
>> b/llvm/lib/Target/X86/X86IndirectBranchTracking.cpp
>> index 743df41a00ff..0a79b793a980 100644
>> --- a/llvm/lib/Target/X86/X86IndirectBranchTracking.cpp
>> +++ b/llvm/lib/Target/X86/X86IndirectBranchTracking.cpp
>> @@ -18,6 +18,7 @@
>>    #include "X86.h"
>>    #include "X86InstrInfo.h"
>>    #include "X86Subtarget.h"
>> +#include "X86TargetMachine.h"
>>    #include "llvm/ADT/Statistic.h"
>>    #include "llvm/CodeGen/MachineFunctionPass.h"
>>    #include "llvm/CodeGen/MachineInstrBuilder.h"
>> @@ -102,7 +103,16 @@ bool X86IndirectBranchTrackingPass::runOnMachineFunction(MachineFunction &MF) {
>>      // Check that the cf-protection-branch is enabled.
>>      Metadata *isCFProtectionSupported =
>>          
>> MF.getMMI().getModule()->getModuleFlag("cf-protection-branch");
>> -  if (!isCFProtectionSupported && !IndirectBranchTracking)
>> +  // NB: We need to enable IBT in jitted code if JIT compiler is CET
>> +  // enabled.
>> +  const X86TargetMachine *TM =
>> +      static_cast<const X86TargetMachine *>(&MF.getTarget()); #ifdef
>> +__CET__
>> +  bool isJITwithCET = TM->isJIT();
>> +#else
>> +  bool isJITwithCET = false;
>> +#endif
>> +  if (!isCFProtectionSupported && !IndirectBranchTracking &&
>> +!isJITwithCET)
>>        return false;
>>    
>>      // True if the current MF was changed and false otherwise.
>> @@ -111,10 +121,11 @@ bool X86IndirectBranchTrackingPass::runOnMachineFunction(MachineFunction &MF) {
>>      TII = SubTarget.getInstrInfo();
>>      EndbrOpcode = SubTarget.is64Bit() ? X86::ENDBR64 : X86::ENDBR32;
>>    
>> -  // Non-internal function or function whose address was taken, can
>> be
>> -  // accessed through indirect calls. Mark the first BB with ENDBR
>> instruction
>> -  // unless nocf_check attribute is used.
>> -  if ((MF.getFunction().hasAddressTaken() ||
>> +  // Large code model, non-internal function or function whose
>> + address  // was taken, can be accessed through indirect calls. Mark
>> + the first  // BB with ENDBR instruction unless nocf_check attribute is used.
>> +  if ((TM->getCodeModel() == CodeModel::Large ||
>> +       MF.getFunction().hasAddressTaken() ||
>>           !MF.getFunction().hasLocalLinkage()) &&
>>          !MF.getFunction().doesNoCfCheck()) {
>>        auto MBB = MF.begin();
>> @@ -136,8 +147,8 @@ bool X86IndirectBranchTrackingPass::runOnMachineFunction(MachineFunction &MF) {
>>            Changed |= addENDBR(MBB, std::next(I));
>>    
>>          if (EHPadIBTNeeded && I->isEHLabel()) {
>> -          Changed |= addENDBR(MBB, std::next(I));
>> -          EHPadIBTNeeded = false;
>> +        Changed |= addENDBR(MBB, std::next(I));
>> +        EHPadIBTNeeded = false;
>>          }
>>        }
>>      }
>>
>> diff  --git a/llvm/lib/Target/X86/X86TargetMachine.cpp
>> b/llvm/lib/Target/X86/X86TargetMachine.cpp
>> index d80c82c584bd..90a47c30dfc5 100644
>> --- a/llvm/lib/Target/X86/X86TargetMachine.cpp
>> +++ b/llvm/lib/Target/X86/X86TargetMachine.cpp
>> @@ -214,7 +214,7 @@ X86TargetMachine::X86TargetMachine(const Target &T, const Triple &TT,
>>              getEffectiveRelocModel(TT, JIT, RM),
>>              getEffectiveX86CodeModel(CM, JIT, TT.getArch() == Triple::x86_64),
>>              OL),
>> -      TLOF(createTLOF(getTargetTriple())) {
>> +      TLOF(createTLOF(getTargetTriple())), IsJIT(JIT) {
>>      // On PS4, the "return address" of a 'noreturn' call must still be within
>>      // the calling function, and TrapUnreachable is an easy way to get that.
>>      if (TT.isPS4() || TT.isOSBinFormatMachO()) {
>>
>> diff  --git a/llvm/lib/Target/X86/X86TargetMachine.h
>> b/llvm/lib/Target/X86/X86TargetMachine.h
>> index ec3db7b1e9e8..757ce8bc5c72 100644
>> --- a/llvm/lib/Target/X86/X86TargetMachine.h
>> +++ b/llvm/lib/Target/X86/X86TargetMachine.h
>> @@ -30,6 +30,8 @@ class TargetTransformInfo;
>>    class X86TargetMachine final : public LLVMTargetMachine {
>>      std::unique_ptr<TargetLoweringObjectFile> TLOF;
>>      mutable StringMap<std::unique_ptr<X86Subtarget>> SubtargetMap;
>> +  // True if this is used in JIT.
>> +  bool IsJIT;
>>    
>>    public:
>>      X86TargetMachine(const Target &T, const Triple &TT, StringRef CPU,
>> @@ -52,6 +54,8 @@ class X86TargetMachine final : public LLVMTargetMachine {
>>      TargetLoweringObjectFile *getObjFileLowering() const override {
>>        return TLOF.get();
>>      }
>> +
>> +  bool isJIT() const { return IsJIT; }
>>    };
>>    
>>    } // end namespace llvm
>>
>> diff  --git
>> a/llvm/test/CodeGen/X86/indirect-branch-tracking-cm-lager.ll
>> b/llvm/test/CodeGen/X86/indirect-branch-tracking-cm-lager.ll
>> new file mode 100644
>> index 000000000000..f515c1a34c73
>> --- /dev/null
>> +++ b/llvm/test/CodeGen/X86/indirect-branch-tracking-cm-lager.ll
>> @@ -0,0 +1,36 @@
>> +; RUN: llc -mtriple=x86_64-unknown-unknown -code-model=large < %s |
>> +FileCheck %s
>> +
>> +; In large code model indirect branches are needed when branching to
>> +addresses ; whose offset from the current instruction pointer is unknown.
>> +;CHECK-COUNT-3: endbr
>> +
>> + at a = dso_local local_unnamed_addr global i32 1, align 4
>> +
>> +; Function Attrs: nofree noinline norecurse nounwind uwtable
>> +writeonly define dso_local void @ext() local_unnamed_addr #0 {
>> +entry:
>> +  store i32 0, i32* @a, align 4
>> +  ret void
>> +}
>> +
>> +; Function Attrs: nofree norecurse nounwind uwtable define dso_local
>> +i32 @main() local_unnamed_addr #1 {
>> +entry:
>> +  tail call fastcc void @foo()
>> +  %0 = load i32, i32* @a, align 4
>> +  ret i32 %0
>> +}
>> +
>> +; Function Attrs: nofree noinline norecurse nounwind uwtable
>> +writeonly define internal fastcc void @foo() unnamed_addr #0 {
>> +entry:
>> +  tail call void @ext()
>> +  ret void
>> +}
>> +
>> +!llvm.module.flags = !{!0, !1, !2, !3}
>> +
>> +!0 = !{i32 1, !"wchar_size", i32 4}
>> +!1 = !{i32 4, !"cf-protection-return", i32 1}
>> +!2 = !{i32 4, !"cf-protection-branch", i32 1}
>> +!3 = !{i32 1, !"Code Model", i32 4}
>>
>> diff  --git a/llvm/test/ExecutionEngine/MCJIT/cet-code-model-lager.ll
>> b/llvm/test/ExecutionEngine/MCJIT/cet-code-model-lager.ll
>> new file mode 100644
>> index 000000000000..782a2b84a844
>> --- /dev/null
>> +++ b/llvm/test/ExecutionEngine/MCJIT/cet-code-model-lager.ll
>> @@ -0,0 +1,33 @@
>> +; RUN: %lli -code-model=large %s > /dev/null target triple =
>> +"x86_64-unknown-linux-gnu"
>> +
>> + at a = dso_local local_unnamed_addr global i32 1, align 4
>> +
>> +; Function Attrs: nofree noinline norecurse nounwind uwtable
>> +writeonly define dso_local void @ext() local_unnamed_addr #0 {
>> +entry:
>> +  store i32 0, i32* @a, align 4
>> +  ret void
>> +}
>> +
>> +; Function Attrs: nofree norecurse nounwind uwtable define dso_local
>> +i32 @main() local_unnamed_addr #1 {
>> +entry:
>> +  tail call fastcc void @foo()
>> +  %0 = load i32, i32* @a, align 4
>> +  ret i32 %0
>> +}
>> +
>> +; Function Attrs: nofree noinline norecurse nounwind uwtable
>> +writeonly define internal fastcc void @foo() unnamed_addr #0 {
>> +entry:
>> +  tail call void @ext()
>> +  ret void
>> +}
>> +
>> +!llvm.module.flags = !{!0, !1, !2, !3}
>> +
>> +!0 = !{i32 1, !"wchar_size", i32 4}
>> +!1 = !{i32 4, !"cf-protection-return", i32 1}
>> +!2 = !{i32 4, !"cf-protection-branch", i32 1}
>> +!3 = !{i32 1, !"Code Model", i32 4}
>>
>>
>>           
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list