Index: lib/cfi/CMakeLists.txt =================================================================== --- lib/cfi/CMakeLists.txt +++ lib/cfi/CMakeLists.txt @@ -30,7 +30,6 @@ RTSanitizerCommon RTSanitizerCommonLibc RTUbsan - RTUbsan_cxx CFLAGS ${CFI_CFLAGS} ${CFI_DIAG_CFLAGS} PARENT_TARGET cfi) endforeach() Index: lib/cfi/cfi.cc =================================================================== --- lib/cfi/cfi.cc +++ lib/cfi/cfi.cc @@ -42,7 +42,7 @@ return (uint16_t *)(__cfi_shadow + ((x >> kShadowGranularity) << 1)); } -typedef int (*CFICheckFn)(u64, void *); +typedef int (*CFICheckFn)(u64, void *, void *); class ShadowValue { uptr addr; @@ -188,14 +188,20 @@ dl_iterate_phdr(dl_iterate_phdr_cb, nullptr); } -extern "C" SANITIZER_INTERFACE_ATTRIBUTE -void __cfi_slowpath(u64 CallSiteTypeId, void *Ptr) { +static ALWAYS_INLINE void CfiSlowPathCommon(u64 CallSiteTypeId, void *Ptr, + void *DiagData) { uptr Addr = (uptr)Ptr; VReport(3, "__cfi_slowpath: %llx, %p\n", CallSiteTypeId, Ptr); ShadowValue sv = ShadowValue::load(Addr); if (sv.is_invalid()) { - VReport(2, "CFI: invalid memory region for a function pointer (shadow==0): %p\n", Ptr); - Die(); + // FIXME: call the ubsan handler if DiagData != nullptr? + Report( + "CFI: invalid memory region for a function pointer (shadow==0): %p\n", + Ptr); + if (DiagData) + return; + else + Die(); } if (sv.is_unchecked()) { VReport(2, "CFI: unchecked call (shadow=FFFF): %p\n", Ptr); @@ -203,7 +209,17 @@ } CFICheckFn cfi_check = sv.get_cfi_check(); VReport(2, "__cfi_check at %p\n", cfi_check); - cfi_check(CallSiteTypeId, Ptr); + cfi_check(CallSiteTypeId, Ptr, DiagData); +} + +extern "C" SANITIZER_INTERFACE_ATTRIBUTE void +__cfi_slowpath(u64 CallSiteTypeId, void *Ptr) { + CfiSlowPathCommon(CallSiteTypeId, Ptr, nullptr); +} + +extern "C" SANITIZER_INTERFACE_ATTRIBUTE void +__cfi_slowpath_diag(u64 CallSiteTypeId, void *Ptr, void *DiagData) { + CfiSlowPathCommon(CallSiteTypeId, Ptr, DiagData); } static void InitializeFlags() { Index: lib/ubsan/ubsan_handlers.h =================================================================== --- lib/ubsan/ubsan_handlers.h +++ lib/ubsan/ubsan_handlers.h @@ -148,13 +148,24 @@ /// \brief Handle passing null pointer to function with nonnull attribute. RECOVERABLE(nonnull_arg, NonNullArgData *Data) -struct CFIBadIcallData { +/// \brief Known CFI check kinds. +/// Keep in sync with the enum of the same name in CodeGenFunction.h +enum CFITypeCheckKind : unsigned char { + CFITCK_VCall, + CFITCK_NVCall, + CFITCK_DerivedCast, + CFITCK_UnrelatedCast, + CFITCK_ICall, +}; + +struct CFICheckFailData { + CFITypeCheckKind CheckKind; SourceLocation Loc; const TypeDescriptor &Type; }; -/// \brief Handle control flow integrity failure for indirect function calls. -RECOVERABLE(cfi_bad_icall, CFIBadIcallData *Data, ValueHandle Function) +/// \brief Handle control flow integrity failures. +RECOVERABLE(cfi_check_fail, CFICheckFailData *Data, ValueHandle Function) } Index: lib/ubsan/ubsan_handlers.cc =================================================================== --- lib/ubsan/ubsan_handlers.cc +++ lib/ubsan/ubsan_handlers.cc @@ -523,8 +523,11 @@ Die(); } -static void handleCFIBadIcall(CFIBadIcallData *Data, ValueHandle Function, +static void handleCFIBadIcall(CFICheckFailData *Data, ValueHandle Function, ReportOptions Opts) { + if (Data->CheckKind != CFITCK_ICall) + Die(); + SourceLocation Loc = Data->Loc.acquire(); ErrorType ET = ErrorType::CFIBadType; @@ -544,16 +547,28 @@ Diag(FLoc, DL_Note, "%0 defined here") << FName; } -void __ubsan::__ubsan_handle_cfi_bad_icall(CFIBadIcallData *Data, - ValueHandle Function) { +namespace __ubsan { +SANITIZER_WEAK_ATTRIBUTE +void HandleCFIBadType(CFICheckFailData *Data, ValueHandle Vtable, + ReportOptions Opts); +} // namespace __ubsan + +void __ubsan::__ubsan_handle_cfi_check_fail(CFICheckFailData *Data, + ValueHandle Value) { GET_REPORT_OPTIONS(false); - handleCFIBadIcall(Data, Function, Opts); + if (Data->CheckKind == CFITCK_ICall) + handleCFIBadIcall(Data, Value, Opts); + else + HandleCFIBadType(Data, Value, Opts); } -void __ubsan::__ubsan_handle_cfi_bad_icall_abort(CFIBadIcallData *Data, - ValueHandle Function) { +void __ubsan::__ubsan_handle_cfi_check_fail_abort(CFICheckFailData *Data, + ValueHandle Value) { GET_REPORT_OPTIONS(true); - handleCFIBadIcall(Data, Function, Opts); + if (Data->CheckKind == CFITCK_ICall) + handleCFIBadIcall(Data, Value, Opts); + else + HandleCFIBadType(Data, Value, Opts); Die(); } Index: lib/ubsan/ubsan_handlers_cxx.h =================================================================== --- lib/ubsan/ubsan_handlers_cxx.h +++ lib/ubsan/ubsan_handlers_cxx.h @@ -25,12 +25,6 @@ unsigned char TypeCheckKind; }; -struct CFIBadTypeData { - SourceLocation Loc; - const TypeDescriptor &Type; - unsigned char TypeCheckKind; -}; - /// \brief Handle a runtime type check failure, caused by an incorrect vptr. /// When this handler is called, all we know is that the type was not in the /// cache; this does not necessarily imply the existence of a bug. @@ -40,14 +34,6 @@ extern "C" SANITIZER_INTERFACE_ATTRIBUTE void __ubsan_handle_dynamic_type_cache_miss_abort( DynamicTypeCacheMissData *Data, ValueHandle Pointer, ValueHandle Hash); - -/// \brief Handle a control flow integrity check failure by printing a -/// diagnostic. -extern "C" SANITIZER_INTERFACE_ATTRIBUTE void -__ubsan_handle_cfi_bad_type(CFIBadTypeData *Data, ValueHandle Vtable); -extern "C" SANITIZER_INTERFACE_ATTRIBUTE void -__ubsan_handle_cfi_bad_type_abort(CFIBadTypeData *Data, ValueHandle Vtable); - } #endif // UBSAN_HANDLERS_H Index: lib/ubsan/ubsan_handlers_cxx.cc =================================================================== --- lib/ubsan/ubsan_handlers_cxx.cc +++ lib/ubsan/ubsan_handlers_cxx.cc @@ -15,6 +15,7 @@ #include "ubsan_platform.h" #if CAN_SANITIZE_UB +#include "ubsan_handlers.h" #include "ubsan_handlers_cxx.h" #include "ubsan_diag.h" #include "ubsan_type_hash.h" @@ -87,8 +88,9 @@ Die(); } -static void HandleCFIBadType(CFIBadTypeData *Data, ValueHandle Vtable, - ReportOptions Opts) { +namespace __ubsan { +void HandleCFIBadType(CFICheckFailData *Data, ValueHandle Vtable, + ReportOptions Opts) { SourceLocation Loc = Data->Loc.acquire(); ErrorType ET = ErrorType::CFIBadType; @@ -96,18 +98,29 @@ return; ScopedReport R(Opts, Loc, ET); - DynamicTypeInfo DTI = getDynamicTypeInfoFromVtable((void*)Vtable); - - static const char *TypeCheckKinds[] = { - "virtual call", - "non-virtual call", - "base-to-derived cast", - "cast to unrelated type", - }; + DynamicTypeInfo DTI = getDynamicTypeInfoFromVtable((void *)Vtable); + + const char *CheckKindStr; + switch (Data->CheckKind) { + case CFITCK_VCall: + CheckKindStr = "virtual call"; + break; + case CFITCK_NVCall: + CheckKindStr = "non-virtual call"; + break; + case CFITCK_DerivedCast: + CheckKindStr = "base-to-derived cast"; + break; + case CFITCK_UnrelatedCast: + CheckKindStr = "cast to unrelated type"; + break; + case CFITCK_ICall: + Die(); + } Diag(Loc, DL_Error, "control flow integrity check for type %0 failed during " "%1 (vtable address %2)") - << Data->Type << TypeCheckKinds[Data->TypeCheckKind] << (void *)Vtable; + << Data->Type << CheckKindStr << (void *)Vtable; // If possible, say what type it actually points to. if (!DTI.isValid()) @@ -116,18 +129,6 @@ Diag(Vtable, DL_Note, "vtable is of type %0") << TypeName(DTI.getMostDerivedTypeName()); } +} // namespace __ubsan -void __ubsan::__ubsan_handle_cfi_bad_type(CFIBadTypeData *Data, - ValueHandle Vtable) { - GET_REPORT_OPTIONS(false); - HandleCFIBadType(Data, Vtable, Opts); -} - -void __ubsan::__ubsan_handle_cfi_bad_type_abort(CFIBadTypeData *Data, - ValueHandle Vtable) { - GET_REPORT_OPTIONS(true); - HandleCFIBadType(Data, Vtable, Opts); - Die(); -} - -#endif // CAN_SANITIZE_UB +#endif // CAN_SANITIZE_UB Index: test/cfi/cross-dso/icall/diag.cpp =================================================================== --- /dev/null +++ test/cfi/cross-dso/icall/diag.cpp @@ -0,0 +1,157 @@ +// Cross-DSO diagnostics. +// The rules are: +// * If the library needs diagnostics, the main executable must request at +// least some diagnostics as well (to link the diagnostic runtime). +// * -fsanitize-trap on the caller side overrides everything. +// * otherwise, the callee decides between trap/recover/norecover. + +// Full-recover. +// RUN: %clangxx_cfi_dso_diag -g -DSHARED_LIB %s -fPIC -shared -o %t-so.so +// RUN: %clangxx_cfi_dso_diag -g %s -o %t %t-so.so + +// RUN: %t icv 2>&1 | FileCheck %s --check-prefix=ICALL-DIAG --check-prefix=CAST-DIAG \ +// RUN: --check-prefix=VCALL-DIAG --check-prefix=ALL-RECOVER + +// RUN: %t i_v 2>&1 | FileCheck %s --check-prefix=ICALL-DIAG --check-prefix=CAST-NODIAG \ +// RUN: --check-prefix=VCALL-DIAG --check-prefix=ALL-RECOVER + +// RUN: %t _cv 2>&1 | FileCheck %s --check-prefix=ICALL-NODIAG --check-prefix=CAST-DIAG \ +// RUN: --check-prefix=VCALL-DIAG --check-prefix=ALL-RECOVER + +// RUN: %t ic_ 2>&1 | FileCheck %s --check-prefix=ICALL-DIAG --check-prefix=CAST-DIAG \ +// RUN: --check-prefix=VCALL-NODIAG --check-prefix=ALL-RECOVER + +// Trap on icall, no-recover on cast. +// RUN: %clangxx_cfi_dso_diag -fsanitize-trap=cfi-icall -fno-sanitize-recover=cfi-unrelated-cast \ +// RUN: -g -DSHARED_LIB %s -fPIC -shared -o %t-so.so +// RUN: %clangxx_cfi_dso_diag -fsanitize-trap=cfi-icall -fno-sanitize-recover=cfi-unrelated-cast \ +// RUN: -g %s -o %t %t-so.so + +// RUN: %expect_crash %t icv 2>&1 | FileCheck %s --check-prefix=ICALL-NODIAG --check-prefix=CAST-NODIAG \ +// RUN: --check-prefix=VCALL-NODIAG --check-prefix=ICALL-FATAL + +// RUN: not %t _cv 2>&1 | FileCheck %s --check-prefix=ICALL-NODIAG --check-prefix=CAST-DIAG \ +// RUN: --check-prefix=VCALL-NODIAG --check-prefix=CAST-FATAL + +// RUN: %t __v 2>&1 | FileCheck %s --check-prefix=ICALL-NODIAG --check-prefix=CAST-NODIAG \ +// RUN: --check-prefix=VCALL-DIAG + +// Callee: trap on icall, no-recover on cast. +// Caller: recover on everything. +// The same as in the previous case, behaviour is decided by the callee. +// RUN: %clangxx_cfi_dso_diag -fsanitize-trap=cfi-icall -fno-sanitize-recover=cfi-unrelated-cast \ +// RUN: -g -DSHARED_LIB %s -fPIC -shared -o %t-so.so +// RUN: %clangxx_cfi_dso_diag \ +// RUN: -g %s -o %t %t-so.so + +// RUN: %expect_crash %t icv 2>&1 | FileCheck %s --check-prefix=ICALL-NODIAG --check-prefix=CAST-NODIAG \ +// RUN: --check-prefix=VCALL-NODIAG --check-prefix=ICALL-FATAL + +// RUN: not %t _cv 2>&1 | FileCheck %s --check-prefix=ICALL-NODIAG --check-prefix=CAST-DIAG \ +// RUN: --check-prefix=VCALL-NODIAG --check-prefix=CAST-FATAL + +// RUN: %t __v 2>&1 | FileCheck %s --check-prefix=ICALL-NODIAG --check-prefix=CAST-NODIAG \ +// RUN: --check-prefix=VCALL-DIAG + +// Caller in trapping mode, callee with full diagnostic+recover. +// Caller wins. +// cfi-nvcall is non-trapping in the main executable to link the diagnostic runtime library. +// RUN: %clangxx_cfi_dso_diag \ +// RUN: -g -DSHARED_LIB %s -fPIC -shared -o %t-so.so +// RUN: %clangxx_cfi_dso -fno-sanitize-trap=cfi-nvcall \ +// RUN: -g %s -o %t %t-so.so + +// RUN: %expect_crash %t icv 2>&1 | FileCheck %s --check-prefix=ICALL-NODIAG --check-prefix=CAST-NODIAG \ +// RUN: --check-prefix=VCALL-NODIAG --check-prefix=ICALL-FATAL + +// RUN: %expect_crash %t _cv 2>&1 | FileCheck %s --check-prefix=ICALL-NODIAG --check-prefix=CAST-NODIAG \ +// RUN: --check-prefix=VCALL-NODIAG --check-prefix=CAST-FATAL + +// RUN: %expect_crash %t __v 2>&1 | FileCheck %s --check-prefix=ICALL-NODIAG --check-prefix=CAST-NODIAG \ +// RUN: --check-prefix=VCALL-NODIAG --check-prefix=VCALL-FATAL + +#include +#include +#include + +struct A { + virtual void f(); +}; + +void *create_B(); + +#ifdef SHARED_LIB + +#include "../../utils.h" +struct B { + virtual void f(); +}; +void B::f() {} + +void *create_B() { + create_derivers(); + return (void *)(new B()); +} + +#else + +void A::f() {} + +int main(int argc, char *argv[]) { + assert(argc == 2); + assert(strlen(argv[1]) == 3); + + // ICALL-FATAL: =0= + // CAST-FATAL: =0= + // VCALL-FATAL: =0= + // ALL-RECOVER: =0= + fprintf(stderr, "=0=\n"); + + void *p; + if (argv[1][0] == 'i') { + // ICALL-DIAG: runtime error: control flow integrity check for type 'void *(int)' failed during indirect function call + // ICALL-DIAG-NEXT: note: create_B() defined here + // ICALL-NODIAG-NOT: runtime error: control flow integrity check {{.*}} during indirect function call + p = ((void *(*)(int))create_B)(42); + } else { + p = create_B(); + } + + // ICALL-FATAL-NOT: =1= + // CAST-FATAL: =1= + // VCALL-FATAL: =1= + // ALL-RECOVER: =1= + fprintf(stderr, "=1=\n"); + + A *a; + if (argv[1][1] == 'c') { + // CAST-DIAG: runtime error: control flow integrity check for type 'A' failed during cast to unrelated type + // CAST-DIAG-NEXT: note: vtable is of type '{{(struct )?}}B' + // CAST-NODIAG-NOT: runtime error: control flow integrity check {{.*}} during cast to unrelated type + a = (A*)p; + } else { + // Invisible to CFI. + memcpy(&a, &p, sizeof(a)); + } + + // ICALL-FATAL-NOT: =2= + // CAST-FATAL-NOT: =2= + // VCALL-FATAL: =2= + // ALL-RECOVER: =2= + fprintf(stderr, "=2=\n"); + + // VCALL-DIAG: runtime error: control flow integrity check for type 'A' failed during virtual call + // VCALL-DIAG-NEXT: note: vtable is of type '{{(struct )?}}B' + // VCALL-NODIAG-NOT: runtime error: control flow integrity check {{.*}} during virtual call + if (argv[1][2] == 'v') { + a->f(); // UB here + } + + // ICALL-FATAL-NOT: =3= + // CAST-FATAL-NOT: =3= + // VCALL-FATAL-NOT: =3= + // ALL-RECOVER: =3= + fprintf(stderr, "=3=\n"); + +} +#endif Index: test/cfi/cross-dso/icall/icall-from-dso.cpp =================================================================== --- test/cfi/cross-dso/icall/icall-from-dso.cpp +++ test/cfi/cross-dso/icall/icall-from-dso.cpp @@ -1,17 +1,25 @@ // RUN: %clangxx_cfi_dso -DSHARED_LIB %s -fPIC -shared -o %t-so.so // RUN: %clangxx_cfi_dso %s -o %t %t-so.so && %expect_crash %t 2>&1 | FileCheck %s +// RUN: %clangxx_cfi_dso_diag -g -DSHARED_LIB %s -fPIC -shared -o %t2-so.so +// RUN: %clangxx_cfi_dso_diag -g %s -o %t2 %t2-so.so && %t2 2>&1 | FileCheck %s --check-prefix=CFI-DIAG + #include #ifdef SHARED_LIB void g(); void f() { + // CHECK-DIAG: =1= // CHECK: =1= fprintf(stderr, "=1=\n"); ((void (*)(void))g)(); + // CHECK-DIAG: =2= // CHECK: =2= fprintf(stderr, "=2=\n"); + // CFI-DIAG: runtime error: control flow integrity check for type 'void (int)' failed during indirect function call + // CFI-DIAG-NEXT: note: g() defined here ((void (*)(int))g)(42); // UB here + // CHECK-DIAG: =3= // CHECK-NOT: =3= fprintf(stderr, "=3=\n"); } Index: test/cfi/cross-dso/icall/icall.cpp =================================================================== --- test/cfi/cross-dso/icall/icall.cpp +++ test/cfi/cross-dso/icall/icall.cpp @@ -1,6 +1,9 @@ // RUN: %clangxx_cfi_dso -DSHARED_LIB %s -fPIC -shared -o %t-so.so // RUN: %clangxx_cfi_dso %s -o %t %t-so.so && %expect_crash %t 2>&1 | FileCheck %s +// RUN: %clangxx_cfi_dso_diag -g -DSHARED_LIB %s -fPIC -shared -o %t2-so.so +// RUN: %clangxx_cfi_dso_diag -g %s -o %t2 %t2-so.so && %t2 2>&1 | FileCheck %s --check-prefix=CFI-DIAG + #include #ifdef SHARED_LIB @@ -9,12 +12,17 @@ #else void f(); int main() { + // CHECK-DIAG: =1= // CHECK: =1= fprintf(stderr, "=1=\n"); ((void (*)(void))f)(); + // CHECK-DIAG: =2= // CHECK: =2= fprintf(stderr, "=2=\n"); + // CFI-DIAG: runtime error: control flow integrity check for type 'void (int)' failed during indirect function call + // CFI-DIAG-NEXT: note: f() defined here ((void (*)(int))f)(42); // UB here + // CHECK-DIAG: =3= // CHECK-NOT: =3= fprintf(stderr, "=3=\n"); } Index: test/cfi/cross-dso/simple-fail.cpp =================================================================== --- test/cfi/cross-dso/simple-fail.cpp +++ test/cfi/cross-dso/simple-fail.cpp @@ -28,6 +28,11 @@ // RUN: %t6 2>&1 | FileCheck --check-prefix=NCFI %s // RUN: %t6 x 2>&1 | FileCheck --check-prefix=NCFI %s +// RUN: %clangxx_cfi_dso_diag -DSHARED_LIB %s -fPIC -shared -o %t7-so.so +// RUN: %clangxx_cfi_dso_diag %s -o %t7 %t7-so.so +// RUN: %t7 2>&1 | FileCheck --check-prefix=CFI-DIAG-CALL %s +// RUN: %t7 x 2>&1 | FileCheck --check-prefix=CFI-DIAG-CALL --check-prefix=CFI-DIAG-CAST %s + // Tests that the CFI mechanism crashes the program when making a virtual call // to an object of the wrong class but with a compatible vtable, by casting a // pointer to such an object and attempting to make a call through it. @@ -71,6 +76,8 @@ if (argc > 1 && argv[1][0] == 'x') { // Test cast. BOOM. + // CFI-DIAG-CAST: runtime error: control flow integrity check for type 'A' failed during cast to unrelated type + // CFI-DIAG-CAST-NEXT: note: vtable is of type '{{(struct )?}}B' a = (A*)p; } else { // Invisible to CFI. Test virtual call later. @@ -82,6 +89,8 @@ // NCFI: =1= fprintf(stderr, "=1=\n"); + // CFI-DIAG-CALL: runtime error: control flow integrity check for type 'A' failed during virtual call + // CFI-DIAG-CALL-NEXT: note: vtable is of type '{{(struct )?}}B' a->f(); // UB here // CFI-NOT: =2=