[llvm-branch-commits] [clang] c138c8a - [UBSan] Disable the function and kcfi sanitizers on an execute-only target.

Tobias Hieta via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Thu Aug 31 23:27:41 PDT 2023


Author: Ying Yi
Date: 2023-09-01T08:26:36+02:00
New Revision: c138c8a72e360c65da7cfe0cd4b716d78cdc428d

URL: https://github.com/llvm/llvm-project/commit/c138c8a72e360c65da7cfe0cd4b716d78cdc428d
DIFF: https://github.com/llvm/llvm-project/commit/c138c8a72e360c65da7cfe0cd4b716d78cdc428d.diff

LOG: [UBSan] Disable the function and kcfi sanitizers on an execute-only target.

An execute-only target disallows data access to code sections.
-fsanitize=function and -fsanitize=kcfi instrument indirect function
calls to load a type hash before the function label. This results in a
non-execute access to the code section and a runtime error.

To solve the issue, -fsanitize=function should not be included in any
check group (e.g. undefined) on an execute-only target. If a user passes
-fsanitize=undefined, there is no error and no warning. However, if the
user explicitly passes -fsanitize=function or -fsanitize=kcfi on an
execute-only target, an error will be emitted.

Fixes: https://github.com/llvm/llvm-project/issues/64931.

Reviewed By: MaskRay, probinson, simon_tatham

Differential Revision: https://reviews.llvm.org/D158614

Added: 
    

Modified: 
    clang/include/clang/Basic/Sanitizers.h
    clang/lib/Basic/CMakeLists.txt
    clang/lib/Basic/Sanitizers.cpp
    clang/lib/Driver/SanitizerArgs.cpp
    clang/test/CodeGenObjCXX/crash-function-type.mm
    clang/test/Driver/fsanitize.c

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/Sanitizers.h b/clang/include/clang/Basic/Sanitizers.h
index db53010645ae3b..c212f80fe03adc 100644
--- a/clang/include/clang/Basic/Sanitizers.h
+++ b/clang/include/clang/Basic/Sanitizers.h
@@ -23,7 +23,11 @@
 
 namespace llvm {
 class hash_code;
+class Triple;
+namespace opt {
+class ArgList;
 }
+} // namespace llvm
 
 namespace clang {
 
@@ -205,6 +209,11 @@ StringRef AsanDetectStackUseAfterReturnModeToString(
 llvm::AsanDetectStackUseAfterReturnMode
 AsanDetectStackUseAfterReturnModeFromString(StringRef modeStr);
 
+/// Return true if an execute-only target disallows data access to code
+/// sections.
+bool isExecuteOnlyTarget(const llvm::Triple &Triple,
+                         const llvm::opt::ArgList &Args);
+
 } // namespace clang
 
 #endif // LLVM_CLANG_BASIC_SANITIZERS_H

diff  --git a/clang/lib/Basic/CMakeLists.txt b/clang/lib/Basic/CMakeLists.txt
index caa1b6002e6f18..d6620ec204ad48 100644
--- a/clang/lib/Basic/CMakeLists.txt
+++ b/clang/lib/Basic/CMakeLists.txt
@@ -1,4 +1,5 @@
 set(LLVM_LINK_COMPONENTS
+  Option
   Support
   TargetParser
   )

diff  --git a/clang/lib/Basic/Sanitizers.cpp b/clang/lib/Basic/Sanitizers.cpp
index 62ccdf8e9bbf28..6fbc32df314896 100644
--- a/clang/lib/Basic/Sanitizers.cpp
+++ b/clang/lib/Basic/Sanitizers.cpp
@@ -11,10 +11,13 @@
 //===----------------------------------------------------------------------===//
 
 #include "clang/Basic/Sanitizers.h"
+#include "clang/Driver/Options.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Option/ArgList.h"
 #include "llvm/Support/MathExtras.h"
+#include "llvm/TargetParser/Triple.h"
 
 using namespace clang;
 
@@ -112,4 +115,14 @@ AsanDetectStackUseAfterReturnModeFromString(StringRef modeStr) {
       .Default(llvm::AsanDetectStackUseAfterReturnMode::Invalid);
 }
 
+bool isExecuteOnlyTarget(const llvm::Triple &Triple,
+                         const llvm::opt::ArgList &Args) {
+  if (Triple.isPS5())
+    return true;
+
+  // On Arm, the clang `-mexecute-only` option is used to generate the
+  // execute-only output (no data access to code sections).
+  return Args.hasFlag(clang::driver::options::OPT_mexecute_only,
+                      clang::driver::options::OPT_mno_execute_only, false);
+}
 } // namespace clang

diff  --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
index c3ce13f93464d7..a4e9475947487d 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -37,6 +37,8 @@ static const SanitizerMask NeedsUbsanCxxRt =
     SanitizerKind::Vptr | SanitizerKind::CFI;
 static const SanitizerMask NotAllowedWithTrap = SanitizerKind::Vptr;
 static const SanitizerMask NotAllowedWithMinimalRuntime = SanitizerKind::Vptr;
+static const SanitizerMask NotAllowedWithExecuteOnly =
+    SanitizerKind::Function | SanitizerKind::KCFI;
 static const SanitizerMask RequiresPIE =
     SanitizerKind::DataFlow | SanitizerKind::Scudo;
 static const SanitizerMask NeedsUnwindTables =
@@ -395,6 +397,22 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
           DiagnosedKinds |= SanitizerKind::Function;
         }
       }
+      // -fsanitize=function and -fsanitize=kcfi instrument indirect function
+      // calls to load a type hash before the function label. Therefore, an
+      // execute-only target doesn't support the function and kcfi sanitizers.
+      const llvm::Triple &Triple = TC.getTriple();
+      if (isExecuteOnlyTarget(Triple, Args)) {
+        if (SanitizerMask KindsToDiagnose =
+                Add & NotAllowedWithExecuteOnly & ~DiagnosedKinds) {
+          if (DiagnoseErrors) {
+            std::string Desc = describeSanitizeArg(Arg, KindsToDiagnose);
+            D.Diag(diag::err_drv_argument_not_allowed_with)
+                << Desc << Triple.str();
+          }
+          DiagnosedKinds |= KindsToDiagnose;
+        }
+        Add &= ~NotAllowedWithExecuteOnly;
+      }
 
       // FIXME: Make CFI on member function calls compatible with cross-DSO CFI.
       // There are currently two problems:
@@ -457,6 +475,10 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
       if (MinimalRuntime) {
         Add &= ~NotAllowedWithMinimalRuntime;
       }
+      // NotAllowedWithExecuteOnly is silently discarded on an execute-only
+      // target if implicitly enabled through group expansion.
+      if (isExecuteOnlyTarget(Triple, Args))
+        Add &= ~NotAllowedWithExecuteOnly;
       if (CfiCrossDso)
         Add &= ~SanitizerKind::CFIMFCall;
       Add &= Supported;

diff  --git a/clang/test/CodeGenObjCXX/crash-function-type.mm b/clang/test/CodeGenObjCXX/crash-function-type.mm
index 53acc58dfc44d8..280497a3258a40 100644
--- a/clang/test/CodeGenObjCXX/crash-function-type.mm
+++ b/clang/test/CodeGenObjCXX/crash-function-type.mm
@@ -1,3 +1,6 @@
+// Mark test as unsupported on PS5 due to PS5 doesn't support function sanitizer.
+// UNSUPPORTED: target=x86_64-sie-ps5
+
 // RUN: %clang_cc1 -fblocks -fsanitize=function -emit-llvm %s -o %t
 
 void g(void (^)());

diff  --git a/clang/test/Driver/fsanitize.c b/clang/test/Driver/fsanitize.c
index 182de9f486444a..9442f6b91471f6 100644
--- a/clang/test/Driver/fsanitize.c
+++ b/clang/test/Driver/fsanitize.c
@@ -971,3 +971,17 @@
 
 // RUN: %clang --target=x86_64-linux-gnu -fsanitize=undefined,function -mcmodel=large %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION-CODE-MODEL
 // CHECK-UBSAN-FUNCTION-CODE-MODEL: error: invalid argument '-fsanitize=function' only allowed with '-mcmodel=small'
+
+// RUN: %clang --target=x86_64-sie-ps5 -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: %clang --target=x86_64-sie-ps5 -fsanitize=undefined -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: %clang --target=x86_64-sie-ps5 -fsanitize=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-KCFI
+// RUN: %clang --target=x86_64-sie-ps5 -fsanitize=function -fsanitize=kcfi %s -### 2>&1 | FileCheck %s  --check-prefix=CHECK-UBSAN-KCFI --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: %clang --target=x86_64-sie-ps5 -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-UNDEFINED
+
+// RUN: %clang --target=armv6t2-eabi -mexecute-only -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: %clang --target=armv6t2-eabi -mexecute-only -fsanitize=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-KCFI
+// RUN: %clang --target=armv6t2-eabi -mexecute-only -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-UNDEFINED
+
+// CHECK-UBSAN-KCFI-DAG: error: invalid argument '-fsanitize=kcfi' not allowed with {{('x86_64-sie-ps5'|'armv6t2-unknown-unknown-eabi')}}
+// CHECK-UBSAN-FUNCTION-DAG: error: invalid argument '-fsanitize=function' not allowed with {{('x86_64-sie-ps5'|'armv6t2-unknown-unknown-eabi')}}
+// CHECK-UBSAN-UNDEFINED: "-fsanitize={{((alignment|array-bounds|bool|builtin|enum|float-cast-overflow|integer-divide-by-zero|nonnull-attribute|null|pointer-overflow|return|returns-nonnull-attribute|shift-base|shift-exponent|signed-integer-overflow|unreachable|vla-bound),?){17}"}}


        


More information about the llvm-branch-commits mailing list