[PATCH] Add Forward-Edge Control-Flow Integrity support

Tom Roeder tmroeder at google.com
Mon Jul 14 14:34:57 PDT 2014


================
Comment at: include/llvm/CodeGen/CommandFlags.h:245
@@ +244,3 @@
+CFIFuncName("cfi-func-name", cl::desc("The name of the CFI function to call"),
+            cl::init(""));
+
----------------
JF Bastien wrote:
> Could you clarify that the above two options are linked? Or just have cfi-func-name, implying a call to abort otherwise? It kind of seems like the default cfi-func could just call abort?
> 
> Also, what's the function's expected signature?
> 
> What happens if the cfi-func itself gets CFI instrumented, and it fails the check? *That* should probably call abort directly, not recurse :)
Is there an abort function that's guaranteed to be present for any IR? Right now, my "abort" is to execute a trap instruction.

================
Comment at: lib/CodeGen/ForwardControlFlowIntegrity.cpp:151
@@ +150,3 @@
+    FunctionType *FunTy = dyn_cast<FunctionType>(EltTy);
+    if (!FunTy)
+      continue;
----------------
JF Bastien wrote:
> Same.
I think you're right here and in the previous comment: this shouldn't ever happen.

================
Comment at: lib/CodeGen/ForwardControlFlowIntegrity.cpp:198
@@ +197,3 @@
+    // really get this information from the Target in some way.
+    int64_t MaskValue = ((TableSize << 3) - 1) & -8;
+    Constant *JumpTableMaskValue = ConstantInt::get(Int64Ty, MaskValue);
----------------
JF Bastien wrote:
> Could you fix this?
I'd like to fix this, but I don't know where I can get this information in a principled manner. This requires the size of a jumptable entry, and that depends on the size of the jump instruction in bytes. In principle, this is available from MCInstrDesc::getSize(), but that returns 0 when I've tried it for JMP_4 on X86.

I can certainly encode it myself in the Targets, but that seems like dangerous duplication of functionality with the descriptions in the instruction tables.

================
Comment at: lib/CodeGen/ForwardControlFlowIntegrity.cpp:297
@@ +296,3 @@
+    Constant *RightShift = ConstantInt::get(Int64Ty, 3);
+    Constant *LeftShift = ConstantInt::get(Int64Ty, 64 - 3);
+
----------------
JF Bastien wrote:
> Shouldn't you get the pointer size from the Target here too?
It's not a question of pointer size but rather the size of an unconditional jump instruction. I haven't yet figured out where to get that information.

http://reviews.llvm.org/D4167






More information about the llvm-commits mailing list