Skip to content

Commit

Permalink
Revert "KCFI sanitizer"
Browse files Browse the repository at this point in the history
This reverts commit 67504c9 as using
PointerEmbeddedInt to store 32 bits breaks 32-bit arm builds.
  • Loading branch information
samitolvanen committed Aug 24, 2022
1 parent dda3878 commit a79060e
Show file tree
Hide file tree
Showing 81 changed files with 53 additions and 1,607 deletions.
13 changes: 0 additions & 13 deletions clang/docs/ControlFlowIntegrity.rst
Expand Up @@ -306,19 +306,6 @@ the identity of function pointers is maintained, and calls across shared
library boundaries are no different from calls within a single program or
shared library.

.. _kcfi:

``-fsanitize=kcfi``
-------------------

This is an alternative indirect call control-flow integrity scheme designed
for low-level system software, such as operating system kernels. Unlike
``-fsanitize=cfi-icall``, it doesn't require ``-flto``, won't result in
function pointers being replaced with jump table references, and never breaks
cross-DSO function address equality. These properties make KCFI easier to
adopt in low-level software. KCFI is limited to checking only function
pointers, and isn't compatible with executable-only memory.

Member Function Pointer Call Checking
=====================================

Expand Down
2 changes: 0 additions & 2 deletions clang/docs/UsersManual.rst
Expand Up @@ -1720,8 +1720,6 @@ are listed below.
flow analysis.
- ``-fsanitize=cfi``: :doc:`control flow integrity <ControlFlowIntegrity>`
checks. Requires ``-flto``.
- ``-fsanitize=kcfi``: kernel indirect call forward-edge control flow
integrity.
- ``-fsanitize=safe-stack``: :doc:`safe stack <SafeStack>`
protection against stack-based memory corruption errors.

Expand Down
1 change: 0 additions & 1 deletion clang/include/clang/Basic/Features.def
Expand Up @@ -228,7 +228,6 @@ FEATURE(is_trivially_assignable, LangOpts.CPlusPlus)
FEATURE(is_trivially_constructible, LangOpts.CPlusPlus)
FEATURE(is_trivially_copyable, LangOpts.CPlusPlus)
FEATURE(is_union, LangOpts.CPlusPlus)
FEATURE(kcfi, LangOpts.Sanitize.has(SanitizerKind::KCFI))
FEATURE(modules, LangOpts.Modules)
FEATURE(safe_stack, LangOpts.Sanitize.has(SanitizerKind::SafeStack))
FEATURE(shadow_call_stack,
Expand Down
3 changes: 0 additions & 3 deletions clang/include/clang/Basic/Sanitizers.def
Expand Up @@ -127,9 +127,6 @@ SANITIZER_GROUP("cfi", CFI,
CFIDerivedCast | CFIICall | CFIMFCall | CFIUnrelatedCast |
CFINVCall | CFIVCall)

// Kernel Control Flow Integrity
SANITIZER("kcfi", KCFI)

// Safe Stack
SANITIZER("safe-stack", SafeStack)

Expand Down
4 changes: 0 additions & 4 deletions clang/lib/CodeGen/CGCall.cpp
Expand Up @@ -5368,10 +5368,6 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
SmallVector<llvm::OperandBundleDef, 1> BundleList =
getBundlesForFunclet(CalleePtr);

if (SanOpts.has(SanitizerKind::KCFI) &&
!isa_and_nonnull<FunctionDecl>(TargetDecl))
EmitKCFIOperandBundle(ConcreteCallee, BundleList);

if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(CurFuncDecl))
if (FD->hasAttr<StrictFPAttr>())
// All calls within a strictfp function are marked strictfp
Expand Down
8 changes: 0 additions & 8 deletions clang/lib/CodeGen/CodeGenFunction.cpp
Expand Up @@ -2606,14 +2606,6 @@ void CodeGenFunction::EmitSanitizerStatReport(llvm::SanitizerStatKind SSK) {
CGM.getSanStats().create(IRB, SSK);
}

void CodeGenFunction::EmitKCFIOperandBundle(
const CGCallee &Callee, SmallVectorImpl<llvm::OperandBundleDef> &Bundles) {
const FunctionProtoType *FP =
Callee.getAbstractInfo().getCalleeFunctionProtoType();
if (FP)
Bundles.emplace_back("kcfi", CGM.CreateKCFITypeId(FP->desugar()));
}

llvm::Value *
CodeGenFunction::FormResolverCondition(const MultiVersionResolverOption &RO) {
llvm::Value *Condition = nullptr;
Expand Down
3 changes: 0 additions & 3 deletions clang/lib/CodeGen/CodeGenFunction.h
Expand Up @@ -4612,9 +4612,6 @@ class CodeGenFunction : public CodeGenTypeCache {
/// passing to a runtime sanitizer handler.
llvm::Constant *EmitCheckSourceLocation(SourceLocation Loc);

void EmitKCFIOperandBundle(const CGCallee &Callee,
SmallVectorImpl<llvm::OperandBundleDef> &Bundles);

/// Create a basic block that will either trap or call a handler function in
/// the UBSan runtime with the provided arguments, and create a conditional
/// branch to it.
Expand Down
75 changes: 0 additions & 75 deletions clang/lib/CodeGen/CodeGenModule.cpp
Expand Up @@ -48,7 +48,6 @@
#include "clang/CodeGen/ConstantInitBuilder.h"
#include "clang/Frontend/FrontendDiagnostic.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringSwitch.h"
#include "llvm/ADT/Triple.h"
#include "llvm/Analysis/TargetLibraryInfo.h"
Expand All @@ -68,7 +67,6 @@
#include "llvm/Support/MD5.h"
#include "llvm/Support/TimeProfiler.h"
#include "llvm/Support/X86TargetParser.h"
#include "llvm/Support/xxhash.h"

using namespace clang;
using namespace CodeGen;
Expand Down Expand Up @@ -579,8 +577,6 @@ void CodeGenModule::Release() {
CodeGenFunction(*this).EmitCfiCheckFail();
CodeGenFunction(*this).EmitCfiCheckStub();
}
if (LangOpts.Sanitize.has(SanitizerKind::KCFI))
finalizeKCFITypes();
emitAtAvailableLinkGuard();
if (Context.getTargetInfo().getTriple().isWasm())
EmitMainVoidAlias();
Expand Down Expand Up @@ -763,9 +759,6 @@ void CodeGenModule::Release() {
CodeGenOpts.SanitizeCfiCanonicalJumpTables);
}

if (LangOpts.Sanitize.has(SanitizerKind::KCFI))
getModule().addModuleFlag(llvm::Module::Override, "kcfi", 1);

if (CodeGenOpts.CFProtectionReturn &&
Target.checkCFProtectionReturnSupported(getDiags())) {
// Indicate that we want to instrument return control flow protection.
Expand Down Expand Up @@ -1676,20 +1669,6 @@ llvm::ConstantInt *CodeGenModule::CreateCrossDsoCfiTypeId(llvm::Metadata *MD) {
return llvm::ConstantInt::get(Int64Ty, llvm::MD5Hash(MDS->getString()));
}

llvm::ConstantInt *CodeGenModule::CreateKCFITypeId(QualType T) {
if (auto *FnType = T->getAs<FunctionProtoType>())
T = getContext().getFunctionType(
FnType->getReturnType(), FnType->getParamTypes(),
FnType->getExtProtoInfo().withExceptionSpec(EST_None));

std::string OutName;
llvm::raw_string_ostream Out(OutName);
getCXXABI().getMangleContext().mangleTypeName(T, Out);

return llvm::ConstantInt::get(Int32Ty,
static_cast<uint32_t>(llvm::xxHash64(OutName)));
}

void CodeGenModule::SetLLVMFunctionAttributes(GlobalDecl GD,
const CGFunctionInfo &Info,
llvm::Function *F, bool IsThunk) {
Expand Down Expand Up @@ -2308,57 +2287,6 @@ void CodeGenModule::CreateFunctionTypeMetadataForIcall(const FunctionDecl *FD,
F->addTypeMetadata(0, llvm::ConstantAsMetadata::get(CrossDsoTypeId));
}

void CodeGenModule::setKCFIType(const FunctionDecl *FD, llvm::Function *F) {
if (isa<CXXMethodDecl>(FD) && !cast<CXXMethodDecl>(FD)->isStatic())
return;

llvm::LLVMContext &Ctx = F->getContext();
llvm::MDBuilder MDB(Ctx);
F->setMetadata(llvm::LLVMContext::MD_kcfi_type,
llvm::MDNode::get(
Ctx, MDB.createConstant(CreateKCFITypeId(FD->getType()))));
}

static bool allowKCFIIdentifier(StringRef Name) {
// KCFI type identifier constants are only necessary for external assembly
// functions, which means it's safe to skip unusual names. Subset of
// MCAsmInfo::isAcceptableChar() and MCAsmInfoXCOFF::isAcceptableChar().
return llvm::all_of(Name, [](const char &C) {
return llvm::isAlnum(C) || C == '_' || C == '.';
});
}

void CodeGenModule::finalizeKCFITypes() {
llvm::Module &M = getModule();
for (auto &F : M.functions()) {
// Remove KCFI type metadata from non-address-taken local functions.
bool AddressTaken = F.hasAddressTaken();
if (!AddressTaken && F.hasLocalLinkage())
F.eraseMetadata(llvm::LLVMContext::MD_kcfi_type);

// Generate a constant with the expected KCFI type identifier for all
// address-taken function declarations to support annotating indirectly
// called assembly functions.
if (!AddressTaken || !F.isDeclaration())
continue;

const llvm::ConstantInt *Type;
if (const llvm::MDNode *MD = F.getMetadata(llvm::LLVMContext::MD_kcfi_type))
Type = llvm::mdconst::extract<llvm::ConstantInt>(MD->getOperand(0));
else
continue;

StringRef Name = F.getName();
if (!allowKCFIIdentifier(Name))
continue;

std::string Asm = (".weak __kcfi_typeid_" + Name + "\n.set __kcfi_typeid_" +
Name + ", " + Twine(Type->getZExtValue()) + "\n")
.str();
M.appendModuleInlineAsm(Asm);
}
}

void CodeGenModule::SetFunctionAttributes(GlobalDecl GD, llvm::Function *F,
bool IsIncompleteFunction,
bool IsThunk) {
Expand Down Expand Up @@ -2441,9 +2369,6 @@ void CodeGenModule::SetFunctionAttributes(GlobalDecl GD, llvm::Function *F,
!CodeGenOpts.SanitizeCfiCanonicalJumpTables)
CreateFunctionTypeMetadataForIcall(FD, F);

if (LangOpts.Sanitize.has(SanitizerKind::KCFI))
setKCFIType(FD, F);

if (getLangOpts().OpenMP && FD->hasAttr<OMPDeclareSimdDeclAttr>())
getOpenMPRuntime().emitDeclareSimdFunction(FD, F);

Expand Down
9 changes: 0 additions & 9 deletions clang/lib/CodeGen/CodeGenModule.h
Expand Up @@ -1440,9 +1440,6 @@ class CodeGenModule : public CodeGenTypeCache {
/// Generate a cross-DSO type identifier for MD.
llvm::ConstantInt *CreateCrossDsoCfiTypeId(llvm::Metadata *MD);

/// Generate a KCFI type identifier for T.
llvm::ConstantInt *CreateKCFITypeId(QualType T);

/// Create a metadata identifier for the given type. This may either be an
/// MDString (for external identifiers) or a distinct unnamed MDNode (for
/// internal identifiers).
Expand All @@ -1461,12 +1458,6 @@ class CodeGenModule : public CodeGenTypeCache {
void CreateFunctionTypeMetadataForIcall(const FunctionDecl *FD,
llvm::Function *F);

/// Set type metadata to the given function.
void setKCFIType(const FunctionDecl *FD, llvm::Function *F);

/// Emit KCFI type identifier constants and remove unused identifiers.
void finalizeKCFITypes();

/// Whether this function's return type has no side effects, and thus may
/// be trivially discarded if it is unused.
bool MayDropFunctionReturn(const ASTContext &Context, QualType ReturnType);
Expand Down
15 changes: 3 additions & 12 deletions clang/lib/Driver/SanitizerArgs.cpp
Expand Up @@ -37,8 +37,7 @@ static const SanitizerMask NotAllowedWithTrap = SanitizerKind::Vptr;
static const SanitizerMask NotAllowedWithMinimalRuntime =
SanitizerKind::Function | SanitizerKind::Vptr;
static const SanitizerMask RequiresPIE =
SanitizerKind::DataFlow | SanitizerKind::HWAddress | SanitizerKind::Scudo |
SanitizerKind::KCFI;
SanitizerKind::DataFlow | SanitizerKind::HWAddress | SanitizerKind::Scudo;
static const SanitizerMask NeedsUnwindTables =
SanitizerKind::Address | SanitizerKind::HWAddress | SanitizerKind::Thread |
SanitizerKind::Memory | SanitizerKind::DataFlow;
Expand All @@ -60,9 +59,8 @@ static const SanitizerMask RecoverableByDefault =
SanitizerKind::FloatDivideByZero | SanitizerKind::ObjCCast;
static const SanitizerMask Unrecoverable =
SanitizerKind::Unreachable | SanitizerKind::Return;
static const SanitizerMask AlwaysRecoverable = SanitizerKind::KernelAddress |
SanitizerKind::KernelHWAddress |
SanitizerKind::KCFI;
static const SanitizerMask AlwaysRecoverable =
SanitizerKind::KernelAddress | SanitizerKind::KernelHWAddress;
static const SanitizerMask NeedsLTO = SanitizerKind::CFI;
static const SanitizerMask TrappingSupported =
(SanitizerKind::Undefined & ~SanitizerKind::Vptr) | SanitizerKind::Integer |
Expand Down Expand Up @@ -714,13 +712,6 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
options::OPT_fno_sanitize_cfi_canonical_jump_tables, true);
}

if (AllAddedKinds & SanitizerKind::KCFI && DiagnoseErrors) {
if (AllAddedKinds & SanitizerKind::CFI)
D.Diag(diag::err_drv_argument_not_allowed_with)
<< "-fsanitize=kcfi"
<< lastArgumentForMask(D, Args, SanitizerKind::CFI);
}

Stats = Args.hasFlag(options::OPT_fsanitize_stats,
options::OPT_fno_sanitize_stats, false);

Expand Down
3 changes: 0 additions & 3 deletions clang/lib/Driver/ToolChain.cpp
Expand Up @@ -1089,9 +1089,6 @@ SanitizerMask ToolChain::getSupportedSanitizers() const {
getTriple().getArch() == llvm::Triple::arm || getTriple().isWasm() ||
getTriple().isAArch64() || getTriple().isRISCV())
Res |= SanitizerKind::CFIICall;
if (getTriple().getArch() == llvm::Triple::x86_64 ||
getTriple().isAArch64(64))
Res |= SanitizerKind::KCFI;
if (getTriple().getArch() == llvm::Triple::x86_64 ||
getTriple().isAArch64(64) || getTriple().isRISCV())
Res |= SanitizerKind::ShadowCallStack;
Expand Down
58 changes: 0 additions & 58 deletions clang/test/CodeGen/kcfi.c

This file was deleted.

12 changes: 0 additions & 12 deletions clang/test/Driver/fsanitize.c
Expand Up @@ -649,18 +649,6 @@
// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi -fsanitize-stats -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-STATS
// CHECK-CFI-STATS: -fsanitize-stats

// RUN: %clang -target x86_64-linux-gnu -fsanitize=kcfi -fsanitize=cfi -flto -fvisibility=hidden %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-KCFI-NOCFI
// CHECK-KCFI-NOCFI: error: invalid argument '-fsanitize=kcfi' not allowed with '-fsanitize=cfi'

// RUN: %clang -target x86_64-linux-gnu -fsanitize=kcfi -fsanitize-trap=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-KCFI-NOTRAP
// CHECK-KCFI-NOTRAP: error: unsupported argument 'kcfi' to option '-fsanitize-trap='

// RUN: %clang -target x86_64-linux-gnu -fsanitize=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-KCFI
// CHECK-KCFI: "-fsanitize=kcfi"

// RUN: %clang -target x86_64-linux-gnu -fsanitize=kcfi -fno-sanitize-recover=kcfi %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-KCFI-RECOVER
// CHECK-KCFI-RECOVER: error: unsupported argument 'kcfi' to option '-fno-sanitize-recover='

// RUN: %clang_cl -fsanitize=address -c -MDd -### -- %s 2>&1 | FileCheck %s -check-prefix=CHECK-ASAN-DEBUGRTL
// RUN: %clang_cl -fsanitize=address -c -MTd -### -- %s 2>&1 | FileCheck %s -check-prefix=CHECK-ASAN-DEBUGRTL
// RUN: %clang_cl -fsanitize=address -c -LDd -### -- %s 2>&1 | FileCheck %s -check-prefix=CHECK-ASAN-DEBUGRTL
Expand Down

0 comments on commit a79060e

Please sign in to comment.