[clang] fd5e262 - [ModuleUtils][KCFI] Set patchable-function-prefix for synthesized functions

[Sami Tolvanen via cfe-commits]

Author: Sami Tolvanen
Date: 2023-01-11T23:45:49Z
New Revision: fd5e2627066075f3d15ef774ef368e08735a9ac9

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

LOG: [ModuleUtils][KCFI] Set patchable-function-prefix for synthesized functions

When -fpatchable-function-entry is used to emit prefix nops
before functions, KCFI assumes all indirectly called functions
have the same number of prefix nops, because the nops are emitted
between the KCFI type hash and the function entry. However, as
patchable-function-prefix is a function attribute set by Clang,
functions later synthesized by LLVM don't inherit this attribute
and end up not having prefix nops. One of these functions
is asan.module_ctor, which the Linux kernel ends up calling
indirectly when KASAN is enabled.

In order to avoid tripping KCFI, save the expected prefix offset
to a module flag, and use it when we're setting KCFI type for the
relevant synthesized functions.

Link: https://github.com/ClangBuiltLinux/linux/issues/1742

Reviewed By: MaskRay

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

Added: 
    llvm/test/Instrumentation/AddressSanitizer/kcfi-offset.ll

Modified: 
    clang/lib/CodeGen/CodeGenModule.cpp
    clang/test/CodeGen/kcfi.c
    llvm/lib/Transforms/Utils/ModuleUtils.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index c8e27b2f975ca..b85d8926941a5 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -758,8 +758,14 @@ void CodeGenModule::Release() {
                               CodeGenOpts.SanitizeCfiCanonicalJumpTables);
   }
 
-  if (LangOpts.Sanitize.has(SanitizerKind::KCFI))
+  if (LangOpts.Sanitize.has(SanitizerKind::KCFI)) {
     getModule().addModuleFlag(llvm::Module::Override, "kcfi", 1);
+    // KCFI assumes patchable-function-prefix is the same for all indirectly
+    // called functions. Store the expected offset for code generation.
+    if (CodeGenOpts.PatchableFunctionEntryOffset)
+      getModule().addModuleFlag(llvm::Module::Override, "kcfi-offset",
+                                CodeGenOpts.PatchableFunctionEntryOffset);
+  }
 
   if (CodeGenOpts.CFProtectionReturn &&
       Target.checkCFProtectionReturnSupported(getDiags())) {

diff  --git a/clang/test/CodeGen/kcfi.c b/clang/test/CodeGen/kcfi.c
index b0368d59fc744..1adf6df92a654 100644
--- a/clang/test/CodeGen/kcfi.c
+++ b/clang/test/CodeGen/kcfi.c
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -o - %s | FileCheck %s
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -x c++ -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -fpatchable-function-entry-offset=3 -o - %s | FileCheck %s --check-prefixes=CHECK,OFFSET
 #if !__has_feature(kcfi)
 #error Missing kcfi?
 #endif
@@ -54,5 +55,6 @@ int test(void) {
 }
 
 // CHECK-DAG: ![[#]] = !{i32 4, !"kcfi", i32 1}
+// OFFSET-DAG: ![[#]] = !{i32 4, !"kcfi-offset", i32 3}
 // CHECK-DAG: ![[#TYPE]] = !{i32 [[#HASH]]}
 // CHECK-DAG: ![[#TYPE2]] = !{i32 [[#%d,HASH2:]]}

diff  --git a/llvm/lib/Transforms/Utils/ModuleUtils.cpp b/llvm/lib/Transforms/Utils/ModuleUtils.cpp
index ee5e65fbaa505..2c2e8db34f278 100644
--- a/llvm/lib/Transforms/Utils/ModuleUtils.cpp
+++ b/llvm/lib/Transforms/Utils/ModuleUtils.cpp
@@ -161,6 +161,13 @@ void llvm::setKCFIType(Module &M, Function &F, StringRef MangledType) {
       MDNode::get(Ctx, MDB.createConstant(ConstantInt::get(
                            Type::getInt32Ty(Ctx),
                            static_cast<uint32_t>(xxHash64(MangledType))))));
+  // If the module was compiled with -fpatchable-function-entry, ensure
+  // we use the same patchable-function-prefix.
+  if (auto *MD = mdconst::extract_or_null<ConstantInt>(
+          M.getModuleFlag("kcfi-offset"))) {
+    if (unsigned Offset = MD->getZExtValue())
+      F.addFnAttr("patchable-function-prefix", std::to_string(Offset));
+  }
 }
 
 FunctionCallee

diff  --git a/llvm/test/Instrumentation/AddressSanitizer/kcfi-offset.ll b/llvm/test/Instrumentation/AddressSanitizer/kcfi-offset.ll
new file mode 100644
index 0000000000000..b5d103c073998
--- /dev/null
+++ b/llvm/test/Instrumentation/AddressSanitizer/kcfi-offset.ll
@@ -0,0 +1,15 @@
+;; Test that we set patchable-function-prefix for asan.module_ctor when kcfi-offset is defined.
+
+; RUN: opt < %s -passes=asan -S | FileCheck %s
+
+; CHECK: @llvm.global_ctors = {{.*}}{ i32 1, ptr @asan.module_ctor, ptr @asan.module_ctor }
+
+; CHECK: define internal void @asan.module_ctor()
+; CHECK-SAME: #[[#ATTR:]]
+; CHECK-SAME: !kcfi_type
+
+; CHECK: attributes #[[#ATTR]] = { {{.*}} "patchable-function-prefix"="3" }
+
+!llvm.module.flags = !{!0, !1}
+!0 = !{i32 4, !"kcfi", i32 1}
+!1 = !{i32 4, !"kcfi-offset", i32 3}