From 048155c399a97c31b1b1bf1f99ed12f30dd9c441 Mon Sep 17 00:00:00 2001 From: Ivan Krasin Date: Thu, 2 Jun 2016 18:36:12 +0000 Subject: [PATCH] UBSan: crash less often on corrupted Vtables. Summary: This CL adds a weak check for a Vtable prefix: for a well-formed Vtable, we require the prefix to be within [-1<<20; 1<<20]. Practically, this solves most of the known cases when UBSan segfaults without providing any useful diagnostics. Reviewers: pcc Subscribers: kubabrecka Differential Revision: http://reviews.llvm.org/D19750 llvm-svn: 271560 --- compiler-rt/lib/ubsan/ubsan_handlers_cxx.cc | 16 +++++--- compiler-rt/lib/ubsan/ubsan_type_hash.h | 4 ++ .../lib/ubsan/ubsan_type_hash_itanium.cc | 6 +++ .../vptr-corrupted-vtable-itanium.cpp | 41 +++++++++++++++++++ 4 files changed, 62 insertions(+), 5 deletions(-) create mode 100644 compiler-rt/test/ubsan/TestCases/TypeCheck/vptr-corrupted-vtable-itanium.cpp diff --git a/compiler-rt/lib/ubsan/ubsan_handlers_cxx.cc b/compiler-rt/lib/ubsan/ubsan_handlers_cxx.cc index 593b15e12e16..d97ec4813ccd 100644 --- a/compiler-rt/lib/ubsan/ubsan_handlers_cxx.cc +++ b/compiler-rt/lib/ubsan/ubsan_handlers_cxx.cc @@ -55,11 +55,17 @@ static bool HandleDynamicTypeCacheMiss( << TypeCheckKinds[Data->TypeCheckKind] << (void*)Pointer << Data->Type; // If possible, say what type it actually points to. - if (!DTI.isValid()) - Diag(Pointer, DL_Note, "object has invalid vptr") - << TypeName(DTI.getMostDerivedTypeName()) - << Range(Pointer, Pointer + sizeof(uptr), "invalid vptr"); - else if (!DTI.getOffset()) + if (!DTI.isValid()) { + if (DTI.getOffset() < -VptrMaxOffsetToTop || DTI.getOffset() > VptrMaxOffsetToTop) { + Diag(Pointer, DL_Note, "object has a possibly invalid vptr: abs(offset to top) too big") + << TypeName(DTI.getMostDerivedTypeName()) + << Range(Pointer, Pointer + sizeof(uptr), "possibly invalid vptr"); + } else { + Diag(Pointer, DL_Note, "object has invalid vptr") + << TypeName(DTI.getMostDerivedTypeName()) + << Range(Pointer, Pointer + sizeof(uptr), "invalid vptr"); + } + } else if (!DTI.getOffset()) Diag(Pointer, DL_Note, "object is of type %0") << TypeName(DTI.getMostDerivedTypeName()) << Range(Pointer, Pointer + sizeof(uptr), "vptr for %0"); diff --git a/compiler-rt/lib/ubsan/ubsan_type_hash.h b/compiler-rt/lib/ubsan/ubsan_type_hash.h index 695fed905a73..aa638713f089 100644 --- a/compiler-rt/lib/ubsan/ubsan_type_hash.h +++ b/compiler-rt/lib/ubsan/ubsan_type_hash.h @@ -53,6 +53,10 @@ bool checkDynamicType(void *Object, void *Type, HashValue Hash); const unsigned VptrTypeCacheSize = 128; +/// A sanity check for Vtable. Offsets to top must be reasonably small +/// numbers (by absolute value). It's a weak check for Vtable corruption. +const int VptrMaxOffsetToTop = 1<<20; + /// \brief A cache of the results of checkDynamicType. \c checkDynamicType would /// return \c true (modulo hash collisions) if /// \code diff --git a/compiler-rt/lib/ubsan/ubsan_type_hash_itanium.cc b/compiler-rt/lib/ubsan/ubsan_type_hash_itanium.cc index c152eb885848..26272e3360b2 100644 --- a/compiler-rt/lib/ubsan/ubsan_type_hash_itanium.cc +++ b/compiler-rt/lib/ubsan/ubsan_type_hash_itanium.cc @@ -221,6 +221,10 @@ bool __ubsan::checkDynamicType(void *Object, void *Type, HashValue Hash) { VtablePrefix *Vtable = getVtablePrefix(VtablePtr); if (!Vtable) return false; + if (Vtable->Offset < -VptrMaxOffsetToTop || Vtable->Offset > VptrMaxOffsetToTop) { + // Too large or too small offset are signs of Vtable corruption. + return false; + } // Check that this is actually a type_info object for a class type. abi::__class_type_info *Derived = @@ -243,6 +247,8 @@ __ubsan::getDynamicTypeInfoFromVtable(void *VtablePtr) { VtablePrefix *Vtable = getVtablePrefix(VtablePtr); if (!Vtable) return DynamicTypeInfo(0, 0, 0); + if (Vtable->Offset < -VptrMaxOffsetToTop || Vtable->Offset > VptrMaxOffsetToTop) + return DynamicTypeInfo(0, Vtable->Offset, 0); const abi::__class_type_info *ObjectType = findBaseAtOffset( static_cast(Vtable->TypeInfo), -Vtable->Offset); diff --git a/compiler-rt/test/ubsan/TestCases/TypeCheck/vptr-corrupted-vtable-itanium.cpp b/compiler-rt/test/ubsan/TestCases/TypeCheck/vptr-corrupted-vtable-itanium.cpp new file mode 100644 index 000000000000..37ffe5b705f0 --- /dev/null +++ b/compiler-rt/test/ubsan/TestCases/TypeCheck/vptr-corrupted-vtable-itanium.cpp @@ -0,0 +1,41 @@ +// RUN: %clangxx -frtti -fsanitize=vptr -fno-sanitize-recover=vptr -g %s -O3 -o %t +// RUN: not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-CORRUPTED-VTABLE --strict-whitespace + +// UNSUPPORTED: win32 +// REQUIRES: stable-runtime, cxxabi +#include + +#include + +struct S { + S() {} + ~S() {} + virtual int v() { return 0; } +}; + +// See the proper definition in ubsan_type_hash_itanium.cc +struct VtablePrefix { + signed long Offset; + std::type_info *TypeInfo; +}; + +int main(int argc, char **argv) { + // Test that we don't crash on corrupted vtable when + // offset is too large or too small. + S Obj; + void *Ptr = &Obj; + VtablePrefix* RealPrefix = reinterpret_cast( + *reinterpret_cast(Ptr)) - 1; + + VtablePrefix Prefix[2]; + Prefix[0].Offset = 1<<21; // Greater than VptrMaxOffset + Prefix[0].TypeInfo = RealPrefix->TypeInfo; + + // Hack Vtable ptr for Obj. + *reinterpret_cast(Ptr) = static_cast(&Prefix[1]); + + // CHECK-CORRUPTED-VTABLE: vptr-corrupted-vtable-itanium.cpp:[[@LINE+3]]:16: runtime error: member call on address [[PTR:0x[0-9a-f]*]] which does not point to an object of type 'S' + // CHECK-CORRUPTED-VTABLE-NEXT: [[PTR]]: note: object has a possibly invalid vptr: abs(offset to top) too big + S* Ptr2 = reinterpret_cast(Ptr); + return Ptr2->v(); +}