diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index db02449a3dd12..b3a35ec3a16a4 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3830,6 +3830,13 @@ def CFICanonicalJumpTable : InheritableAttr { let SimpleHandler = 1; } +def CFISalt : DeclOrTypeAttr { + let Spellings = [Clang<"cfi_salt">]; + let Args = [StringArgument<"Salt">]; + let Subjects = SubjectList<[Function, Field, Var, TypedefName], ErrorDiag>; + let Documentation = [CFISaltDocs]; +} + // C/C++ Thread safety attributes (e.g. for deadlock, data race checking) // Not all of these attributes will be given a [[]] spelling. The attributes // which require access to function parameter names cannot use the [[]] spelling diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 65d66dd398ad1..4c3a488076c54 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -3639,6 +3639,64 @@ make the function's CFI jump table canonical. See :ref:`the CFI documentation }]; } +def CFISaltDocs : Documentation { + let Category = DocCatFunction; + let Content = [{ +Use ``__attribute__((cfi_salt("")))`` on a function declaration, function +definition, or typedef to help distinguish CFI hashes between functions with +the same type signature. + +Example use: + +.. code-block:: c + + // .h file: + #define __cfi_salt __attribute__((cfi_salt("vogon"))) + + // Convenient typedefs to avoid nested declarator syntax. + typedef int (*fptr_t)(void); // Non-salted function call. + typedef int (*fptr_salted_t)(void) __cfi_salt; + + struct widget_generator { + fptr_t init; + fptr_salted_t exec; + fptr_t teardown; + }; + + // 1st .c file: + static int internal_init(void) { /* ... */ } + static int internal_salted_exec(void) __cfi_salt { /* ... */ } + static int internal_teardown(void) { /* ... */ } + + static struct widget_generator _generator = { + .init = internal_init, + .exec = internal_salted_exec, + .teardown = internal_teardown, + } + + struct widget_generator *widget_gen = _generator; + + // 2nd .c file: + int generate_a_widget(void) { + int ret; + + // Called with non-salted CFI. + ret = widget_gen.init(); + if (ret) + return ret; + + // Called with salted CFI. + ret = widget_gen.exec(); + if (ret) + return ret; + + // Called with non-salted CFI. + return widget_gen.teardown(); + } + + }]; +} + def DocCatTypeSafety : DocumentationCategory<"Type Safety Checking"> { let Content = [{ Clang supports additional attributes to enable checking type safety properties diff --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp index 4793ef38c2c46..b241892f3f836 100644 --- a/clang/lib/AST/TypePrinter.cpp +++ b/clang/lib/AST/TypePrinter.cpp @@ -2099,6 +2099,9 @@ void TypePrinter::printAttributedAfter(const AttributedType *T, case attr::ExtVectorType: OS << "ext_vector_type"; break; + case attr::CFISalt: + OS << "cfi_salt(\"" << cast(T->getAttr())->getSalt() << "\")"; + break; } OS << "))"; } diff --git a/clang/lib/CodeGen/CGCall.h b/clang/lib/CodeGen/CGCall.h index 0b4e3f9cb0365..f54dd64d182e5 100644 --- a/clang/lib/CodeGen/CGCall.h +++ b/clang/lib/CodeGen/CGCall.h @@ -96,6 +96,8 @@ class CGCallee { VirtualInfoStorage VirtualInfo; }; + QualType ExprType; + explicit CGCallee(SpecialKind kind) : KindOrFunctionPointer(kind) {} CGCallee(const FunctionDecl *builtinDecl, unsigned builtinID) @@ -221,6 +223,11 @@ class CGCallee { return VirtualInfo.FTy; } + /// Retain the full type of the callee before canonicalization. This may have + /// attributes important for later processing (e.g. KCFI). + QualType getExprType() const { return ExprType; } + void setExprType(QualType Ty) { ExprType = Ty; } + /// If this is a delayed callee computation of some sort, prepare /// a concrete callee. CGCallee prepareConcreteCallee(CodeGenFunction &CGF) const; diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index bae28b45afaa3..3811fa6a875f8 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -6260,12 +6260,13 @@ RValue CodeGenFunction::EmitCall(QualType CalleeType, !cast(TargetDecl)->isImmediateFunction()) && "trying to emit a call to an immediate function"); + CGCallee Callee = OrigCallee; + Callee.setExprType(CalleeType); + CalleeType = getContext().getCanonicalType(CalleeType); auto PointeeType = cast(CalleeType)->getPointeeType(); - CGCallee Callee = OrigCallee; - if (SanOpts.has(SanitizerKind::Function) && (!TargetDecl || !isa(TargetDecl)) && !isa(PointeeType)) { diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp index 024254b0affe4..471c1b9a58cdb 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -470,16 +470,14 @@ CodeGenFunction::EmitCXXMemberPointerCallExpr(const CXXMemberCallExpr *E, // Ask the ABI to load the callee. Note that This is modified. llvm::Value *ThisPtrForCall = nullptr; - CGCallee Callee = - CGM.getCXXABI().EmitLoadOfMemberFunctionPointer(*this, BO, This, - ThisPtrForCall, MemFnPtr, MPT); - - CallArgList Args; - - QualType ThisType = - getContext().getPointerType(getContext().getTagDeclType(RD)); + CGCallee Callee = CGM.getCXXABI().EmitLoadOfMemberFunctionPointer( + *this, BO, This, ThisPtrForCall, MemFnPtr, MPT); + Callee.setExprType(MemFnExpr->getType()); // Push the this ptr. + QualType ThisType = + getContext().getPointerType(getContext().getTagDeclType(RD)); + CallArgList Args; Args.add(RValue::get(ThisPtrForCall), ThisType); RequiredArgs required = RequiredArgs::forPrototypePlus(FPT, 1); diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index e2357563f7d56..3902b0e0b26c7 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -2887,10 +2887,37 @@ void CodeGenFunction::EmitSanitizerStatReport(llvm::SanitizerStatKind SSK) { void CodeGenFunction::EmitKCFIOperandBundle( const CGCallee &Callee, SmallVectorImpl &Bundles) { - const FunctionProtoType *FP = - Callee.getAbstractInfo().getCalleeFunctionProtoType(); - if (FP) - Bundles.emplace_back("kcfi", CGM.CreateKCFITypeId(FP->desugar())); + const CGCalleeInfo &CI = Callee.getAbstractInfo(); + const FunctionProtoType *FP = CI.getCalleeFunctionProtoType(); + if (!FP) + return; + + const AttributedType *AT = nullptr; + GlobalDecl GD = CI.getCalleeDecl(); + StringRef Salt; + + auto GetAttributedType = [](QualType Ty) { + if (const auto *ET = dyn_cast(Ty)) + if (const auto *TT = dyn_cast(ET->desugar())) + return dyn_cast(TT->desugar()); + + return dyn_cast(Ty); + }; + + if (GD) { + if (const auto *D = dyn_cast(GD.getDecl())) + AT = GetAttributedType(D->getType()); + } else { + QualType ExprTy = Callee.getExprType(); + if (!ExprTy.isNull()) + AT = GetAttributedType(ExprTy); + } + + if (AT) + if (const auto *CFISalt = dyn_cast(AT->getAttr())) + Salt = CFISalt->getSalt(); + + Bundles.emplace_back("kcfi", CGM.CreateKCFITypeId(FP->desugar(), Salt)); } llvm::Value * diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index 6d2c705338ecf..ad2e4a714deba 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -2221,7 +2221,7 @@ llvm::ConstantInt *CodeGenModule::CreateCrossDsoCfiTypeId(llvm::Metadata *MD) { return llvm::ConstantInt::get(Int64Ty, llvm::MD5Hash(MDS->getString())); } -llvm::ConstantInt *CodeGenModule::CreateKCFITypeId(QualType T) { +llvm::ConstantInt *CodeGenModule::CreateKCFITypeId(QualType T, StringRef Salt) { if (auto *FnType = T->getAs()) T = getContext().getFunctionType( FnType->getReturnType(), FnType->getParamTypes(), @@ -2232,6 +2232,9 @@ llvm::ConstantInt *CodeGenModule::CreateKCFITypeId(QualType T) { getCXXABI().getMangleContext().mangleCanonicalTypeName( T, Out, getCodeGenOpts().SanitizeCfiICallNormalizeIntegers); + if (!Salt.empty()) + Out << "." << Salt; + if (getCodeGenOpts().SanitizeCfiICallNormalizeIntegers) Out << ".normalized"; @@ -2898,9 +2901,15 @@ void CodeGenModule::createFunctionTypeMetadataForIcall(const FunctionDecl *FD, void CodeGenModule::setKCFIType(const FunctionDecl *FD, llvm::Function *F) { llvm::LLVMContext &Ctx = F->getContext(); llvm::MDBuilder MDB(Ctx); + llvm::StringRef Salt; + + if (const auto *AT = dyn_cast(FD->getType())) + if (const auto *CFISalt = dyn_cast(AT->getAttr())) + Salt = CFISalt->getSalt(); + F->setMetadata(llvm::LLVMContext::MD_kcfi_type, - llvm::MDNode::get( - Ctx, MDB.createConstant(CreateKCFITypeId(FD->getType())))); + llvm::MDNode::get(Ctx, MDB.createConstant(CreateKCFITypeId( + FD->getType(), Salt)))); } static bool allowKCFIIdentifier(StringRef Name) { diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h index 1db5c3bc4e4ef..5081e1f042546 100644 --- a/clang/lib/CodeGen/CodeGenModule.h +++ b/clang/lib/CodeGen/CodeGenModule.h @@ -1616,7 +1616,7 @@ class CodeGenModule : public CodeGenTypeCache { llvm::ConstantInt *CreateCrossDsoCfiTypeId(llvm::Metadata *MD); /// Generate a KCFI type identifier for T. - llvm::ConstantInt *CreateKCFITypeId(QualType T); + llvm::ConstantInt *CreateKCFITypeId(QualType T, StringRef Salt); /// Create a metadata identifier for the given type. This may either be an /// MDString (for external identifiers) or a distinct unnamed MDNode (for diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 54bac40982eda..b7bd1fd48cd0a 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -6671,6 +6671,29 @@ static void handleEnforceTCBAttr(Sema &S, Decl *D, const ParsedAttr &AL) { D->addAttr(AttrTy::Create(S.Context, Argument, AL)); } +static void handleCFISaltAttr(Sema &S, Decl *D, const ParsedAttr &AL) { + StringRef Argument; + if (!S.checkStringLiteralArgumentAttr(AL, 0, Argument)) + return; + + auto *Attr = CFISaltAttr::Create(S.Context, Argument, AL); + + if (auto *DD = dyn_cast(D)) { + QualType Ty = DD->getType(); + if (!Ty->getAs()) + DD->setType(S.Context.getAttributedType(Attr, Ty, Ty)); + } else if (auto *TD = dyn_cast(D)) { + TypeSourceInfo *TSI = TD->getTypeSourceInfo(); + QualType Ty = TD->getUnderlyingType(); + + if (!Ty->hasAttr(attr::CFISalt)) + TD->setModedTypeSourceInfo(TSI, + S.Context.getAttributedType(Attr, Ty, Ty)); + } else { + llvm_unreachable("Unexpected Decl argument type!"); + } +} + template static AttrTy *mergeEnforceTCBAttrImpl(Sema &S, Decl *D, const AttrTy &AL) { // Check if the new redeclaration has different leaf-ness in the same TCB. @@ -7473,6 +7496,10 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL, handleCountedByAttrField(S, D, AL); break; + case ParsedAttr::AT_CFISalt: + handleCFISaltAttr(S, D, AL); + break; + // Microsoft attributes: case ParsedAttr::AT_LayoutVersion: handleLayoutVersion(S, D, AL); diff --git a/clang/test/CodeGen/kcfi-salt.c b/clang/test/CodeGen/kcfi-salt.c new file mode 100644 index 0000000000000..7728e0525b3ee --- /dev/null +++ b/clang/test/CodeGen/kcfi-salt.c @@ -0,0 +1,190 @@ +// 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 --check-prefixes=CHECK,MEMBER +// 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 + +// Note that the interleving of functions, which normally would be in sequence, +// is due to the fact that Clang outputs them in a non-sequential order. + +#if !__has_feature(kcfi) +#error Missing kcfi? +#endif + +#define __cfi_salt __attribute__((cfi_salt("pepper"))) + +typedef int (*fn_t)(void); +typedef int __cfi_salt (*fn_salt_t)(void); + +typedef unsigned int (*ufn_t)(void); +typedef unsigned int __cfi_salt (*ufn_salt_t)(void); + +/// Must emit __kcfi_typeid symbols for address-taken function declarations +// CHECK: module asm ".weak __kcfi_typeid_[[F4:[a-zA-Z0-9_]+]]" +// CHECK: module asm ".set __kcfi_typeid_[[F4]], [[#%d,LOW_SODIUM_HASH:]]" +// CHECK: module asm ".weak __kcfi_typeid_[[F4_SALT:[a-zA-Z0-9_]+]]" +// CHECK: module asm ".set __kcfi_typeid_[[F4_SALT]], [[#%d,ASM_SALTY_HASH:]]" + +/// Must not __kcfi_typeid symbols for non-address-taken declarations +// CHECK-NOT: module asm ".weak __kcfi_typeid_{{f6|_Z2f6v}}" + +int f1(void); +int f1_salt(void) __cfi_salt; + +unsigned int f2(void); +unsigned int f2_salt(void) __cfi_salt; + +static int f3(void); +static int f3_salt(void) __cfi_salt; + +extern int f4(void); +extern int f4_salt(void) __cfi_salt; + +static int f5(void); +static int f5_salt(void) __cfi_salt; + +extern int f6(void); +extern int f6_salt(void) __cfi_salt; + +struct cfi_struct { + fn_t __cfi_salt fptr; + fn_salt_t td_fptr; +}; + +int f7_salt(struct cfi_struct *ptr); +int f7_typedef_salt(struct cfi_struct *ptr); + +// CHECK-LABEL: @{{__call|_Z6__callPFivE}} +// CHECK: call{{.*}} i32 +// CHECK-NOT: "kcfi" +// CHECK-SAME: () +int __call(fn_t f) __attribute__((__no_sanitize__("kcfi"))) { + return f(); +} + +// CHECK-LABEL: @{{call|_Z4callPFivE}} +// CHECK: call{{.*}} i32 %{{.}}(){{.*}} [ "kcfi"(i32 [[#LOW_SODIUM_HASH]]) ] +// CHECK-LABEL: @{{call_salt|_Z9call_saltPFivE}} +// CHECK: call{{.*}} i32 %{{.}}(){{.*}} [ "kcfi"(i32 [[#%d,SALTY_HASH:]]) ] +// CHECK-LABEL: @{{call_salt_ty|_Z12call_salt_tyPFivE}} +// CHECK: call{{.*}} i32 %{{.}}(){{.*}} [ "kcfi"(i32 [[#SALTY_HASH]]) ] +int call(fn_t f) { return f(); } +int call_salt(fn_t __cfi_salt f) { return f(); } +int call_salt_ty(fn_salt_t f) { return f(); } + +// CHECK-LABEL: @{{ucall|_Z5ucallPFjvE}} +// CHECK: call{{.*}} i32 %{{.}}(){{.*}} [ "kcfi"(i32 [[#%d,LOW_SODIUM_UHASH:]]) ] +// CHECK-LABEL: @{{ucall_salt|_Z10ucall_saltPFjvE}} +// CHECK: call{{.*}} i32 %{{.}}(){{.*}} [ "kcfi"(i32 [[#%d,SALTY_UHASH:]]) ] +// CHECK-LABEL: @{{ucall_salt_ty|_Z13ucall_salt_tyPFjvE}} +// CHECK: call{{.*}} i32 %{{.}}(){{.*}} [ "kcfi"(i32 [[#SALTY_UHASH]]) ] +unsigned int ucall(ufn_t f) { return f(); } +unsigned int ucall_salt(ufn_t __cfi_salt f) { return f(); } +unsigned int ucall_salt_ty(ufn_salt_t f) { return f(); } + +int test1(struct cfi_struct *ptr) { + return call(f1) + + call_salt(f1_salt) + + call_salt_ty(f1_salt) + + + __call((fn_t)f2) + + __call((fn_t)f2_salt) + + + ucall(f2) + + ucall_salt(f2_salt) + + ucall_salt_ty(f2_salt) + + + call(f3) + + call_salt(f3_salt) + + call_salt_ty(f3_salt) + + + call(f4) + + call_salt(f4_salt) + + call_salt_ty(f4_salt) + + + f5() + + f5_salt() + + + f6() + + f6_salt() + + + f7_salt(ptr) + + f7_typedef_salt(ptr); +} + +// CHECK-LABEL: define dso_local{{.*}} i32 @{{f1|_Z2f1v}}(){{.*}} !kcfi_type +// CHECK-SAME: ![[#LOW_SODIUM_TYPE:]] +// CHECK-LABEL: define dso_local{{.*}} i32 @{{f1_salt|_Z7f1_saltv}}(){{.*}} !kcfi_type +// CHECK-SAME: ![[#SALTY_TYPE:]] +int f1(void) { return 0; } +int f1_salt(void) __cfi_salt { return 0; } + +// CHECK-LABEL: define dso_local{{.*}} i32 @{{f2|_Z2f2v}}(){{.*}} !kcfi_type +// CHECK-SAME: ![[#LOW_SODIUM_UTYPE:]] +// CHECK: define dso_local{{.*}} i32 @{{f2_salt|_Z7f2_saltv}}(){{.*}} !kcfi_type +// CHECK-SAME: ![[#SALTY_UTYPE:]] +unsigned int f2(void) { return 2; } +unsigned int f2_salt(void) __cfi_salt { return 2; } + +// CHECK-LABEL: define internal{{.*}} i32 @{{f3|_ZL2f3v}}(){{.*}} !kcfi_type +// CHECK-SAME: ![[#LOW_SODIUM_TYPE]] +// CHECK-LABEL: define internal{{.*}} i32 @{{f3_salt|_ZL7f3_saltv}}(){{.*}} !kcfi_type +// CHECK-SAME: ![[#SALTY_TYPE]] +static int f3(void) { return 1; } +static int f3_salt(void) __cfi_salt { return 1; } + +// CHECK: declare !kcfi_type ![[#LOW_SODIUM_TYPE]]{{.*}} i32 @[[F4]]() +// CHECK: declare !kcfi_type ![[#SALTY_TYPE]]{{.*}} i32 @[[F4_SALT]]() + +/// Must not emit !kcfi_type for non-address-taken local functions +// CHECK-LABEL: define internal{{.*}} i32 @{{f5|_ZL2f5v}}() +// CHECK-NOT: !kcfi_type +// CHECK-SAME: { +// CHECK-LABEL: define internal{{.*}} i32 @{{f5_salt|_ZL7f5_saltv}}() +// CHECK-NOT: !kcfi_type +// CHECK-SAME: { +static int f5(void) { return 2; } +static int f5_salt(void) __cfi_salt { return 2; } + +// CHECK: declare !kcfi_type ![[#LOW_SODIUM_TYPE]]{{.*}} i32 @{{f6|_Z2f6v}}() +// CHECK: declare !kcfi_type ![[#SALTY_TYPE]]{{.*}} i32 @{{f6_salt|_Z7f6_saltv}}() + +// CHECK-LABEL: @{{f7_salt|_Z7f7_saltP10cfi_struct}} +// CHECK: call{{.*}} i32 %{{.*}}() [ "kcfi"(i32 [[#SALTY_HASH]]) ] +// CHECK-LABEL: @{{f7_typedef_salt|_Z15f7_typedef_saltP10cfi_struct}} +// CHECK: call{{.*}} i32 %{{.*}}() [ "kcfi"(i32 [[#SALTY_HASH]]) ] +int f7_salt(struct cfi_struct *ptr) { return ptr->fptr(); } +int f7_typedef_salt(struct cfi_struct *ptr) { return ptr->td_fptr(); } + +#ifdef __cplusplus +// MEMBER-LABEL: define dso_local void @_Z16test_member_callv() #0 !kcfi_type +// MEMBER: call void %[[#]](ptr{{.*}} [ "kcfi"(i32 [[#%d,MEMBER_LOW_SODIUM_HASH:]]) ] +// MEMBER: call void %[[#]](ptr{{.*}} [ "kcfi"(i32 [[#%d,MEMBER_SALT_HASH:]]) ] + +// MEMBER-LABEL: define{{.*}} void @_ZN1A1fEv(ptr{{.*}} %this){{.*}} !kcfi_type +// MEMBER-SAME: [[#MEMBER_LOW_SODIUM_TYPE:]] +// MEMBER-LABEL: define{{.*}} void @_ZN1A1gEv(ptr{{.*}} %this){{.*}} !kcfi_type +// MEMBER-SAME: [[#MEMBER_SALT_TYPE:]] +struct A { + void f() {} + void __cfi_salt g() {} +}; + +void test_member_call(void) { + void (A::* p)() = &A::f; + (A().*p)(); + + void __cfi_salt (A::* q)() = &A::g; + (A().*q)(); +} +#endif + +// CHECK: ![[#]] = !{i32 4, !"kcfi", i32 1} +// OFFSET: ![[#]] = !{i32 4, !"kcfi-offset", i32 3} +// +// CHECK: ![[#LOW_SODIUM_TYPE]] = !{i32 [[#LOW_SODIUM_HASH]]} +// CHECK: ![[#SALTY_TYPE]] = !{i32 [[#SALTY_HASH]]} +// +// CHECK: ![[#LOW_SODIUM_UTYPE]] = !{i32 [[#LOW_SODIUM_UHASH]]} +// CHECK: ![[#SALTY_UTYPE]] = !{i32 [[#SALTY_UHASH]]} +// +// MEMBER: ![[#MEMBER_LOW_SODIUM_TYPE]] = !{i32 [[#MEMBER_LOW_SODIUM_HASH]]} +// MEMBER: ![[#MEMBER_SALT_TYPE]] = !{i32 [[#MEMBER_SALT_HASH]]}