[DFSan] Add force_zero_label abilist option to DFSan. This can be used as a work-around for overtainting.

[morehouse]

What's the motivation for this feature?

I think discard already returns 0 labels. When do we also need to store 0 labels?

compiler-rt/test/dfsan/Inputs/force_zero_abilist.txt

1 ↗

(On Diff #372780)

Why a separate abilist.txt?

compiler-rt/test/dfsan/force_zero.c

26

Wouldn't this also pass if we used uninstrumented or discard in the ABI list?

[browneee]

The motivation for this change is to remove taint in functions which write out their return data...

e.g.

void GenerateData(char* out_buf, int out_buf_len) { ... }

This feature allows us to untaint the data produced by this function.

compiler-rt/test/dfsan/Inputs/force_zero_abilist.txt

1 ↗

(On Diff #372780)

For separate test. Happy to combine them if you prefer.

compiler-rt/test/dfsan/force_zero.c

26

For the return value yes, but not for the shadow of the contents of the the out pointer.

[morehouse]

Please also update the documentation: https://clang.llvm.org/docs/DataFlowSanitizer.html#abi-list

compiler-rt/test/dfsan/Inputs/force_zero_abilist.txt

1 ↗

(On Diff #372780)

Yes, let's combine them for simplicity.

llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp

149

Nit: Could we make the distinction between uninstrumented and instrumented functions a little clearer?

I think we only use functional and discard with uninstrumented, while we only use force_zero_labels with instrumented.

1218

For consistency.

llvm/test/Instrumentation/DataFlowSanitizer/Inputs/force_zero_abilist.txt

1 ↗

(On Diff #372780)

Let's also reuse the existing ABI list for llvm tests.

llvm/test/Instrumentation/DataFlowSanitizer/force_zero.ll

10

Let's check that 0 is being stored specifically to SHADOW_PTR.

[browneee]

Address review comments.

[morehouse]

Please also update the documentation: https://clang.llvm.org/docs/DataFlowSanitizer.html#abi-list

llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp

152

155

[browneee]

Docs

[morehouse]

clang/docs/DataFlowSanitizer.rst

141

[browneee]

Rebase

[browneee]

Update comments.

clang/docs/DataFlowSanitizer.rst

.. code-block:: none
   fun:tolower=uninstrumented
   fun:tolower=functional
 
   # memcpy needs to copy the shadow from the source to the destination region.
   # This is done in a custom function.
   fun:memcpy=uninstrumented
   fun:memcpy=custom
 
+For instrumented functions, the ABI list supports a ``force_zero_labels``
+category, which will make all stores and return values set zero labels.
+Functions should never be labelled with both ``force_zero_labels``
+and ``uninstrumented`` or any of the unistrumented wrapper kinds.
+
+For example:
+
+.. code-block:: none
+
+  # e.g. void writes_data(char* out_buf, int out_buf_len) {...}
+  # Applying force_zero_labels will force out_buf shadow to zero.
+  fun:writes_data=force_zero_labels
+
+
 Compilation Flags
 -----------------
 
 * ``-dfsan-abilist`` -- The additional ABI list files that control how shadow
   parameters are passed. File names are separated by comma.
 * ``-dfsan-combine-pointer-labels-on-load`` -- Controls whether to include or
   ignore the labels of pointers in load instructions. Its default value is true.
   For example:

compiler-rt/test/dfsan/Inputs/flags_abilist.txt

 fun:f=uninstrumented
 
+fun:function_to_force_zero=force_zero_labels
+
 fun:main=uninstrumented
 fun:main=discard
 
-fun:dfsan_create_label=uninstrumented
-fun:dfsan_create_label=discard
-
 fun:dfsan_set_label=uninstrumented
 fun:dfsan_set_label=discard

compiler-rt/test/dfsan/force_zero.c

+// RUN: %clang_dfsan %s -fsanitize-ignorelist=%S/Inputs/flags_abilist.txt -DFORCE_ZERO_LABELS -o %t && %run %t
+// RUN: %clang_dfsan %s -o %t && %run %t
+//
+// REQUIRES: x86_64-target-arch
+
+#include <sanitizer/dfsan_interface.h>
+
+#include <assert.h>
+
+int function_to_force_zero(int i, int* out) {
+  *out = i;
+  return i;
+}
+
+int main(void) {
+  int i = 1;
+  dfsan_label i_label = 2;
+  dfsan_set_label(i_label, &i, sizeof(i));
+
+  int out = 0;
+  int ret = function_to_force_zero(i, &out);
+
+#ifdef FORCE_ZERO_LABELS
+  assert(dfsan_get_label(out) == 0);
+  assert(dfsan_get_label(ret) == 0);
+#else
+  assert(dfsan_get_label(out) == i_label);
+  assert(dfsan_get_label(ret) == i_label);
+#endif
+
+  return 0;
+}

llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp

static cl::opt<bool> ClPreserveAlignment(
     cl::desc("respect alignment requirements provided by input IR"), cl::Hidden,
     cl::init(false));
 
 // The ABI list files control how shadow parameters are passed. The pass treats
 // every function labelled "uninstrumented" in the ABI list file as conforming
 // to the "native" (i.e. unsanitized) ABI.  Unless the ABI list contains
 // additional annotations for those functions, a call to one of those functions
 // will produce a warning message, as the labelling behaviour of the function is
-// unknown.  The other supported annotations are "functional" and "discard",
-// which are described below under DataFlowSanitizer::WrapperKind.
+// unknown. The other supported annotations for uninstrumented functions are
+// "functional" and "discard", which are described below under
+// DataFlowSanitizer::WrapperKind.
+// Functions will often be labelled with both "uninstrumented" and one of
+// "functional" or "discard". This will leave the function unchanged by this
+// pass, and create a wrapper function that will call the original.
+//
+// Instrumented functions can also be annotated as "force_zero_labels", which
+// will make all shadow and return values set zero labels.
+// Functions should never be labelled with both "force_zero_labels" and
+// "uninstrumented" or any of the unistrumented wrapper kinds.
 static cl::list<std::string> ClABIListFiles(
     "dfsan-abilist",
     cl::desc("File listing native ABI functions and how the pass treats them"),
     cl::Hidden);
 
 // Controls whether the pass uses IA_Args or IA_TLS as the ABI for instrumented
 // functions (see DataFlowSanitizer::InstrumentedABI below).
 static cl::opt<bool>
class DataFlowSanitizer {
 
   Value *getShadowOffset(Value *Addr, IRBuilder<> &IRB);
   Value *getShadowAddress(Value *Addr, Instruction *Pos);
   Value *getShadowAddress(Value *Addr, Instruction *Pos, Value *ShadowOffset);
   std::pair<Value *, Value *>
   getShadowOriginAddress(Value *Addr, Align InstAlignment, Instruction *Pos);
   bool isInstrumented(const Function *F);
   bool isInstrumented(const GlobalAlias *GA);
+  bool isForceZeroLabels(const Function *F);
   FunctionType *getArgsFunctionType(FunctionType *T);
   FunctionType *getTrampolineFunctionType(FunctionType *T);
   TransformedFunction getCustomFunctionType(FunctionType *T);
   InstrumentedABI getInstrumentedABI();
   WrapperKind getWrapperKind(Function *F);
   void addGlobalNameSuffix(GlobalValue *GV);
   Function *buildWrapperFunction(Function *F, StringRef NewFName,
                                  GlobalValue::LinkageTypes NewFLink,

 };
 
 struct DFSanFunction {
   DataFlowSanitizer &DFS;
   Function *F;
   DominatorTree DT;
   DataFlowSanitizer::InstrumentedABI IA;
   bool IsNativeABI;
+  bool IsForceZeroLabels;
   AllocaInst *LabelReturnAlloca = nullptr;
   AllocaInst *OriginReturnAlloca = nullptr;
   DenseMap<Value *, Value *> ValShadowMap;
   DenseMap<Value *, Value *> ValOriginMap;
   DenseMap<AllocaInst *, AllocaInst *> AllocaShadowMap;
   DenseMap<AllocaInst *, AllocaInst *> AllocaOriginMap;
 
   struct PHIFixupElement {
struct DFSanFunction {
   DenseMap<std::pair<Value *, Value *>, CachedShadow> CachedShadows;
   /// Maps a value to its latest collapsed shadow value it was converted to in
   /// terms of domination tree. When ClDebugNonzeroLabels is on, this cache is
   /// used at a post process where CFG blocks are split. So it does not cache
   /// BasicBlock like CachedShadows, but uses domination between values.
   DenseMap<Value *, Value *> CachedCollapsedShadows;
   DenseMap<Value *, std::set<Value *>> ShadowElements;
 
-  DFSanFunction(DataFlowSanitizer &DFS, Function *F, bool IsNativeABI)
-      : DFS(DFS), F(F), IA(DFS.getInstrumentedABI()), IsNativeABI(IsNativeABI) {
+  DFSanFunction(DataFlowSanitizer &DFS, Function *F, bool IsNativeABI,
+                bool IsForceZeroLabels)
+      : DFS(DFS), F(F), IA(DFS.getInstrumentedABI()), IsNativeABI(IsNativeABI),
+        IsForceZeroLabels(IsForceZeroLabels) {
     DT.recalculate(*F);
   }
 
   /// Computes the shadow address for a given function argument.
   ///
   /// Shadow = ArgTLS+ArgOffset.
   Value *getArgTLS(Type *T, unsigned ArgOffset, IRBuilder<> &IRB);
 

 bool DataFlowSanitizer::isInstrumented(const Function *F) {
   return !ABIList.isIn(*F, "uninstrumented");
 }
 
 bool DataFlowSanitizer::isInstrumented(const GlobalAlias *GA) {
   return !ABIList.isIn(*GA, "uninstrumented");
 }
 
+bool DataFlowSanitizer::isForceZeroLabels(const Function *F) {
+  return ABIList.isIn(*F, "force_zero_labels");
+}
+
 DataFlowSanitizer::InstrumentedABI DataFlowSanitizer::getInstrumentedABI() {
   return ClArgsABI ? IA_Args : IA_TLS;
 }
 
 DataFlowSanitizer::WrapperKind DataFlowSanitizer::getWrapperKind(Function *F) {
   if (ABIList.isIn(*F, "functional"))
     return WK_Functional;
   if (ABIList.isIn(*F, "discard"))
for (unsigned N = FT->getNumParams(); N != 0; ++AI, --N)
       Args.push_back(&*AI);
     CallInst *CI = CallInst::Create(FT, &*F->arg_begin(), Args, "", BB);
     Type *RetType = FT->getReturnType();
     ReturnInst *RI = RetType->isVoidTy() ? ReturnInst::Create(*Ctx, BB)
                                          : ReturnInst::Create(*Ctx, CI, BB);
 
     // F is called by a wrapped custom function with primitive shadows. So
     // its arguments and return value need conversion.
-    DFSanFunction DFSF(*this, F, /*IsNativeABI=*/true);
+    DFSanFunction DFSF(*this, F, /*IsNativeABI=*/true,
+                       /*ForceZeroLabels=*/false);
     Function::arg_iterator ValAI = F->arg_begin(), ShadowAI = AI;
     ++ValAI;
     for (unsigned N = FT->getNumParams(); N != 0; ++ValAI, ++ShadowAI, --N) {
       Value *Shadow =
           DFSF.expandFromPrimitiveShadow(ValAI->getType(), &*ShadowAI, CI);
       DFSF.ValShadowMap[&*ValAI] = Shadow;
     }
     Function::arg_iterator RetShadowAI = ShadowAI;
bool DataFlowSanitizer::runImpl(Module &M) {
 
   injectMetadataGlobals(M);
 
   initializeCallbackFunctions(M);
   initializeRuntimeFunctions(M);
 
   std::vector<Function *> FnsToInstrument;
   SmallPtrSet<Function *, 2> FnsWithNativeABI;
+  SmallPtrSet<Function *, 2> FnsWithForceZeroLabel;
   for (Function &F : M)
     if (!F.isIntrinsic() && !DFSanRuntimeFunctions.contains(&F))
       FnsToInstrument.push_back(&F);
 
   // Give function aliases prefixes when necessary, and build wrappers where the
   // instrumentedness is inconsistent.
   for (Module::alias_iterator AI = M.alias_begin(), AE = M.alias_end();
        AI != AE;) {
for (std::vector<Function *>::iterator FI = FnsToInstrument.begin(),
        FI != FE; ++FI) {
     Function &F = **FI;
     FunctionType *FT = F.getFunctionType();
 
     bool IsZeroArgsVoidRet = (FT->getNumParams() == 0 && !FT->isVarArg() &&
                               FT->getReturnType()->isVoidTy());
 
     if (isInstrumented(&F)) {
+      if (isForceZeroLabels(&F))
+        FnsWithForceZeroLabel.insert(&F);
+
       // Instrumented functions get a '.dfsan' suffix.  This allows us to more
       // easily identify cases of mismatching ABIs. This naming scheme is
       // mangling-compatible (see Itanium ABI), using a vendor-specific suffix.
       if (getInstrumentedABI() == IA_Args && !IsZeroArgsVoidRet) {
         FunctionType *NewFT = getArgsFunctionType(FT);
         Function *NewF = Function::Create(NewFT, F.getLinkage(),
                                           F.getAddressSpace(), "", &M);
         NewF->copyAttributesFrom(&F);
bool DataFlowSanitizer::runImpl(Module &M) {
   }
 
   for (Function *F : FnsToInstrument) {
     if (!F || F->isDeclaration())
       continue;
 
     removeUnreachableBlocks(*F);
 
-    DFSanFunction DFSF(*this, F, FnsWithNativeABI.count(F));
+    DFSanFunction DFSF(*this, F, FnsWithNativeABI.count(F),
+                       FnsWithForceZeroLabel.count(F));
 
     // DFSanVisitor may create new basic blocks, which confuses df_iterator.
     // Build a copy of the list before iterating over it.
     SmallVector<BasicBlock *, 4> BBList(depth_first(&F->getEntryBlock()));
 
     for (BasicBlock *BB : BBList) {
       Instruction *Inst = &BB->front();
       while (true) {
Value *DFSanFunction::getShadowForTLSArgument(Argument *A) {
   }
 
   return DFS.getZeroShadow(A);
 }
 
 Value *DFSanFunction::getShadow(Value *V) {
   if (!isa<Argument>(V) && !isa<Instruction>(V))
     return DFS.getZeroShadow(V);
+  if (IsForceZeroLabels)
+    return DFS.getZeroShadow(V);
   Value *&Shadow = ValShadowMap[V];
   if (!Shadow) {
     if (Argument *A = dyn_cast<Argument>(V)) {
       if (IsNativeABI)
         return DFS.getZeroShadow(V);
       switch (IA) {
       case DataFlowSanitizer::IA_TLS: {
         Shadow = getShadowForTLSArgument(A);

llvm/test/Instrumentation/DataFlowSanitizer/Inputs/abilist.txt

 fun:discard*=uninstrumented
 fun:discard*=discard
 
 fun:functional=uninstrumented
 fun:functional=functional
 
 fun:custom*=uninstrumented
 fun:custom*=custom
 
 fun:uninstrumented*=uninstrumented
+
+fun:function_to_force_zero=force_zero_labels

llvm/test/Instrumentation/DataFlowSanitizer/force_zero.ll

+; RUN: opt < %s -dfsan -dfsan-abilist=%S/Inputs/abilist.txt -S | FileCheck %s -DSHADOW_XOR_MASK=87960930222080
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-linux-gnu"
+
+define i32 @function_to_force_zero(i32 %0, i32* %1) {
+  ; CHECK-LABEL: define i32 @function_to_force_zero.dfsan
+  ; CHECK: %[[#SHADOW_XOR:]] = xor i64 {{.*}}, [[SHADOW_XOR_MASK]]
+  ; CHECK: %[[#SHADOW_PTR:]] = inttoptr i64 %[[#SHADOW_XOR]] to i8*
+  ; CHECK: %[[#SHADOW_BITCAST:]] = bitcast i8* %[[#SHADOW_PTR]] to i32*
+  ; CHECK: store i32 0, i32* %[[#SHADOW_BITCAST]]
+  ; CHECK: store i32 %{{.*}}
+  store i32 %0, i32* %1, align 4
+  ; CHECK: store i8 0, {{.*}}@__dfsan_retval_tls
+  ; CHECK: ret i32
+  ret i32 %0
+}