[cfi] Safe handling of unaddressable vtable pointers.

[eugenis]

Summary

This change attempts to safely check the cast validity, even when
vtable itself, or its typeinfo object, are unaddressable. The check
is quite slow (requires a system call), so we only do this when an
error is already detected to avoid slowing down the vptr sanitizer.

[pcc]

lib/ubsan/ubsan_type_hash_itanium.cc

212

I don't think we should add a parameter to make the checking optional unless we know that it is necessary. Have you measured the perf impact on UBSan? (Note that UBSan is already calling getDynamicTypeInfoFromVtable on every cache miss).

[eugenis]

lib/ubsan/ubsan_type_hash_itanium.cc

212

Not on every cache miss. On every check failure, right before consulting the blacklist and printing a report.

[eugenis]

Abandoned. See
http://reviews.llvm.org/D16823
http://reviews.llvm.org/D16824

lib/ubsan/ubsan_type_hash_itanium.cc

 
 /// \brief Find the derived-most dynamic base class of \p Derived at offset
 /// \p Offset.
 static const abi::__class_type_info *findBaseAtOffset(
     const abi::__class_type_info *Derived, sptr Offset) {
   if (!Offset)
     return Derived;
 
+  if (!IsAccessibleMemoryRange((uptr)Derived, sizeof(*Derived)))
+    return 0;
+
+  uptr *DerivedVtable = *(uptr **)Derived;
+  if (!IsAccessibleMemoryRange((uptr)(DerivedVtable - 2), 2 * sizeof(uptr)))
+    return 0;
+
   if (const abi::__si_class_type_info *SI =
-        dynamic_cast<const abi::__si_class_type_info*>(Derived))
+          dynamic_cast<const abi::__si_class_type_info *>(Derived)) {
+    if (!IsAccessibleMemoryRange((uptr)&SI->__base_type,
+                                 sizeof(SI->__base_type)))
+      return 0;
     return findBaseAtOffset(SI->__base_type, Offset);
+  }
 
   const abi::__vmi_class_type_info *VTI =
     dynamic_cast<const abi::__vmi_class_type_info*>(Derived);
   if (!VTI)
     // No base class subobjects.
     return 0;
 
+  if (!IsAccessibleMemoryRange((uptr)VTI, sizeof(*VTI)))
+    return 0;
+
+  if (!IsAccessibleMemoryRange((uptr)VTI->base_info,
+                               sizeof(VTI->base_info[0]) * VTI->base_count))
+    return 0;
+
   for (unsigned int base = 0; base != VTI->base_count; ++base) {
     sptr OffsetHere = VTI->base_info[base].__offset_flags >>
                       abi::__base_class_type_info::__offset_shift;
     if (VTI->base_info[base].__offset_flags &
           abi::__base_class_type_info::__virtual_mask)
       // FIXME: Can't handle virtual bases yet.
       continue;
     if (const abi::__class_type_info *Base =

 struct VtablePrefix {
   /// The offset from the vptr to the start of the most-derived object.
   /// This will only be greater than zero in some virtual base class vtables
   /// used during object con-/destruction, and will usually be exactly zero.
   sptr Offset;
   /// The type_info object describing the most-derived class type.
   std::type_info *TypeInfo;
 };
-VtablePrefix *getVtablePrefix(void *Vtable) {
+VtablePrefix *getVtablePrefix(void *Vtable, bool Safe) {
   VtablePrefix *Vptr = reinterpret_cast<VtablePrefix*>(Vtable);
   if (!Vptr)
     return 0;
   VtablePrefix *Prefix = Vptr - 1;
+  if (Safe && !IsAccessibleMemoryRange((uptr)Prefix, sizeof(VtablePrefix)))
+    return 0;
   if (!Prefix->TypeInfo)
     // This can't possibly be a valid vtable.
     return 0;
   return Prefix;
 }
 
 }
 
 bool __ubsan::checkDynamicType(void *Object, void *Type, HashValue Hash) {
   // A crash anywhere within this function probably means the vptr is corrupted.
   // FIXME: Perform these checks more cautiously.
 
   // Check whether this is something we've evicted from the cache.
   HashValue *Bucket = getTypeCacheHashTableBucket(Hash);
   if (*Bucket == Hash) {
     __ubsan_vptr_type_cache[Hash % VptrTypeCacheSize] = Hash;
     return true;
   }
 
   void *VtablePtr = *reinterpret_cast<void **>(Object);
-  VtablePrefix *Vtable = getVtablePrefix(VtablePtr);
+  VtablePrefix *Vtable = getVtablePrefix(VtablePtr, false /* Safe */);
   if (!Vtable)
     return false;
 
   // Check that this is actually a type_info object for a class type.
   abi::__class_type_info *Derived =
     dynamic_cast<abi::__class_type_info*>(Vtable->TypeInfo);
   if (!Derived)
     return false;
 
   abi::__class_type_info *Base = (abi::__class_type_info*)Type;
   if (!isDerivedFromAtOffset(Derived, Base, -Vtable->Offset))
     return false;
 
   // Success. Cache this result.
   __ubsan_vptr_type_cache[Hash % VptrTypeCacheSize] = Hash;
   *Bucket = Hash;
   return true;
 }
 
 __ubsan::DynamicTypeInfo
 __ubsan::getDynamicTypeInfoFromVtable(void *VtablePtr) {
-  VtablePrefix *Vtable = getVtablePrefix(VtablePtr);
+  VtablePrefix *Vtable = getVtablePrefix(VtablePtr, true /* Safe */);
   if (!Vtable)
     return DynamicTypeInfo(0, 0, 0);
   const abi::__class_type_info *ObjectType = findBaseAtOffset(
     static_cast<const abi::__class_type_info*>(Vtable->TypeInfo),
     -Vtable->Offset);
-  return DynamicTypeInfo(Vtable->TypeInfo->__type_name, -Vtable->Offset,
+  const char *most_derived_type_name =
+      IsAccessibleMemoryRange((uptr)&Vtable->TypeInfo->__type_name,
+                              sizeof(char *))
+          ? Vtable->TypeInfo->__type_name
+          : nullptr;
+  return DynamicTypeInfo(most_derived_type_name, -Vtable->Offset,
                          ObjectType ? ObjectType->__type_name : "<unknown>");
 }
 
 #endif  // CAN_SANITIZE_UB && !SANITIZER_WINDOWS

test/cfi/cross-dso/target_out_of_bounds.cpp

 // RUN: %clangxx_cfi_dso_diag %s -o %t
-// RUN: %expect_crash %t 2>&1 | FileCheck %s
+// RUN: %t zero 2>&1 | FileCheck --check-prefix=CHECK-ZERO %s
+// RUN: %t unaddressable 2>&1 | FileCheck --check-prefix=CHECK-UNADDR %s
+// RUN: %t 2>&1 | FileCheck --check-prefix=CHECK-TYPEINFO %s
 
 // REQUIRES: cxxabi
 
 #include <stdio.h>
 #include <stdint.h>
 #include <stdlib.h>
 #include <string.h>
 
 struct A {
   virtual void f();
 };
 
 void A::f() {}
 
 int main(int argc, char *argv[]) {
+  char *volatile p = reinterpret_cast<char *>(new A());
+  if (argc > 1 && strcmp(argv[1], "unaddressable") == 0) {
+    // Create an object with a vtable in an unaddressable memory region.
+    memset(p, 0xFE, sizeof(A));
+    // CHECK-UNADDR: runtime error: control flow integrity check for type 'A' failed during cast
+    // CHECK-UNADDR: fefefefe{{.*}}: note: invalid vtable
+    // CHECK-UNADDR: <memory cannot be printed>
+    // CHECK-UNADDR: runtime error: control flow integrity check for type 'A' failed during cast
+    // CHECK-UNADDR: fefefefe{{.*}}: note: invalid vtable
+    // CHECK-UNADDR: <memory cannot be printed>
+  } else if (argc > 1 && strcmp(argv[1], "zero") == 0) {
     // Create an object with a vtable outside of any known DSO, but still in an
-  // addressable area. Current implementation of handlers in UBSan is not robust
-  // enough to handle unaddressable vtables. TODO: fix this.
+    // addressable area.
     void *empty = calloc(1, 128);
     uintptr_t v = (uintptr_t)empty + 64;
-  char *volatile p = reinterpret_cast<char *>(new A());
     for (uintptr_t *q = (uintptr_t *)p; q < (uintptr_t *)(p + sizeof(A)); ++q)
       *q = v;
+    // CHECK-ZERO: runtime error: control flow integrity check for type 'A' failed during cast
+    // CHECK-ZERO: note: invalid vtable
+    // CHECK-ZERO: 00 00 00 00 00 00 00 00
+    // CHECK-ZERO: runtime error: control flow integrity check for type 'A' failed during cast
+    // CHECK-ZERO: note: invalid vtable
+    // CHECK-ZERO: 00 00 00 00 00 00 00 00
+  } else {
+    // Create an object with a seemingly fine vtable, but with an unaddressable
+    // typeinfo pointer.
+    void *empty = calloc(1, 128);
+    memset(empty, 0xFE, 128);
+    uintptr_t v = (uintptr_t)empty + 64;
+    for (uintptr_t *q = (uintptr_t *)p; q < (uintptr_t *)(p + sizeof(A)); ++q)
+      *q = v;
+    // CHECK-TYPEINFO: runtime error: control flow integrity check for type 'A' failed during cast
+    // CHECK-TYPEINFO: note: invalid vtable
+    // CHECK-TYPEINFO: fe fe fe fe fe fe fe fe
+    // CHECK-TYPEINFO: runtime error: control flow integrity check for type 'A' failed during cast
+    // CHECK-TYPEINFO: note: invalid vtable
+    // CHECK-TYPEINFO: fe fe fe fe fe fe fe fe
+  }
 
-  // CHECK: runtime error: control flow integrity check for type 'A' failed during cast
   A *volatile pa = reinterpret_cast<A *>(p);
-
-  // CHECK: untime error: control flow integrity check for type 'A' failed during virtual call
-  pa->f();
+  pa = reinterpret_cast<A *>(p);
 }