ubsan: Implement memory permission validation for vtables.

[pcc]

Summary

If the pointer passed to the getVtablePrefix function was read from a freed
object, we may end up following pointers into objects on the heap and
printing bogus dynamic type names in diagnostics. However, we know that
vtable pointers will generally only point into memory mapped from object
files, not objects on the heap.

This change causes us to only follow pointers in a vtable if the vtable
and one of the virtual functions it points to appear to have appropriate
permissions (i.e. non-writable, and maybe executable), which will generally
exclude heap pointers.

Only enabled for Linux; this hasn't been tested on FreeBSD, and vtables are
writable on Mac (PR24782) so this won't work there.

[kcc]

lib/ubsan/ubsan_type_hash_itanium.cc

199 ↗

(On Diff #34520)

can this be if (SANITIZER_LINUX)?

test/ubsan/TestCases/Linux/TypeCheck/vptr-bad-perms.cpp

2 ↗

(On Diff #34520)

add an explanation comment to the test

test/ubsan/TestCases/Linux/lit.local.cfg

1 ↗

(On Diff #34520)

I wonder if we can use something like

REQUIRES: linux

instead of putting this test into a separate dir.
We don't have this exact requirement now, but quite a few others.

[pcc]

• Address comments

lib/ubsan/ubsan_type_hash_itanium.cc

199 ↗

(On Diff #34520)

I don't think it can. The code may create references to undefined symbols on (e.g.) Windows.

test/ubsan/TestCases/Linux/TypeCheck/vptr-bad-perms.cpp

2 ↗

(On Diff #34520)

Done

test/ubsan/TestCases/Linux/lit.local.cfg

1 ↗

(On Diff #34520)

Introduced REQUIRES: vptr-validation.

[kcc]

lib/ubsan/ubsan_type_hash_itanium.cc

199 ↗

(On Diff #34523)

ifdefs inside the function body is a great evil.

Alternatively, or maybe even much better, I'd ask you to move this functionality into a separate function
somewhere in sanitizer_common.

[pcc]

• Move vptr validation to a separate function

lib/ubsan/ubsan_type_hash_itanium.cc

199 ↗

(On Diff #34523)

This code isn't really general purpose enough to live in sanitizer_common. I've moved it into a separate function in this file.

[kcc]

LGTM

[samsonov]

LGTM. Thanks!

lib/ubsan/ubsan_type_hash_itanium.cc

197 ↗

(On Diff #34594)

These two functions should be static

[pcc]

lib/ubsan/ubsan_type_hash_itanium.cc

197 ↗

(On Diff #34594)

They are both in an anonymous namespace, so they don't need to be.

[sberg]

If this gets re-enabled after r248157 "Revert 'ubsan: Implement memory permission validation for vtables,'" please consider modifying isValidVptr as

   MemoryMappingLayout Layout(/*cache_enabled=*/true);
   uptr Start, End, Prot;
   while (Layout.Next(&Start, &End, 0, 0, 0, &Prot)) {
+    Prot &= ~MemoryMappingLayout::kProtectionShared;
     if (Start <= ((uptr)Vtable) && ((uptr)Vtable) <= End &&
         (Prot == MemoryMappingLayout::kProtectionRead ||
          Prot == (MemoryMappingLayout::kProtectionRead |

for code like LibreOffice that synthesizes vtables at runtime, and can do so via the double-mmap technique described at http://www.akkadia.org/drepper/selinux-mem.html (to appease SELinux),

compiler-rt/trunk/lib/ubsan/ubsan_type_hash_itanium.cc

 //===----------------------------------------------------------------------===//
 
 #include "sanitizer_common/sanitizer_platform.h"
 #include "ubsan_platform.h"
 #if CAN_SANITIZE_UB && !SANITIZER_WINDOWS
 #include "ubsan_type_hash.h"
 
 #include "sanitizer_common/sanitizer_common.h"
+#include "sanitizer_common/sanitizer_procmaps.h"
 
 // The following are intended to be binary compatible with the definitions
 // given in the Itanium ABI. We make no attempt to be ODR-compatible with
 // those definitions, since existing ABI implementations aren't.
 
 namespace std {
   class type_info {
   public:

 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;
 };
+
+#if SANITIZER_LINUX
+bool isValidVptr(void *Vtable) {
+  // Validate the memory permissions of the vtable pointer and the first
+  // function pointer in the vtable. They should be r-- or r-x and r-x
+  // respectively. Only enabled for Linux; this hasn't been tested on FreeBSD,
+  // and vtables are writable on Mac (PR24782) so this won't work there.
+  uptr FirstFunctionPtr = *reinterpret_cast<uptr *>(Vtable);
+  bool ValidVtable = false, ValidFirstFunctionPtr = false;
+  MemoryMappingLayout Layout(/*cache_enabled=*/true);
+  uptr Start, End, Prot;
+  while (Layout.Next(&Start, &End, 0, 0, 0, &Prot)) {
+    if (Start <= ((uptr)Vtable) && ((uptr)Vtable) <= End &&
+        (Prot == MemoryMappingLayout::kProtectionRead ||
+         Prot == (MemoryMappingLayout::kProtectionRead |
+                  MemoryMappingLayout::kProtectionExecute)))
+      ValidVtable = true;
+    if (Start <= FirstFunctionPtr && FirstFunctionPtr <= End &&
+        Prot == (MemoryMappingLayout::kProtectionRead |
+                 MemoryMappingLayout::kProtectionExecute))
+      ValidFirstFunctionPtr = true;
+    if (ValidVtable && ValidFirstFunctionPtr)
+      return true;
+  }
+  return false;
+}
+#else  // !SANITIZER_LINUX
+bool isValidVptr(void *Vtable) {
+  return true;
+}
+#endif
+
 VtablePrefix *getVtablePrefix(void *Vtable) {
+  if (!IsAccessibleMemoryRange((uptr)Vtable, sizeof(void *)))
+    return 0;
+
+  if (!isValidVptr(Vtable))
+    return 0;
+
   VtablePrefix *Vptr = reinterpret_cast<VtablePrefix*>(Vtable);
   if (!Vptr)
     return 0;
   VtablePrefix *Prefix = Vptr - 1;
   if (!Prefix->TypeInfo)
     // This can't possibly be a valid vtable.
     return 0;
   return Prefix;

compiler-rt/trunk/test/ubsan/TestCases/TypeCheck/vptr-bad-perms.cpp

+// RUN: %clangxx -frtti -fsanitize=vptr -fno-sanitize-recover=vptr -g %s -O3 -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s
+
+// Tests that we consider vtable pointers in writable memory to be invalid.
+
+// REQUIRES: vptr-validation
+
+#include <string.h>
+
+struct A {
+  virtual void f();
+};
+
+void A::f() {}
+
+struct B {
+  virtual void f();
+};
+
+void B::f() {}
+
+int main() {
+  // Create a fake vtable for A in writable memory and copy A's vtable into it.
+  void *fake_vtable[3];
+  A a;
+  void ***vtp = (void ***)&a;
+  memcpy(fake_vtable, *vtp - 2, sizeof(void *) * 3);
+  *vtp = fake_vtable + 2;
+
+  // A's vtable is invalid because it lives in writable memory.
+  // CHECK: invalid vptr
+  reinterpret_cast<B*>(&a)->f();
+}

compiler-rt/trunk/test/ubsan/lit.common.cfg

if config.host_os == 'Windows':
   # We do not currently support enough of the Microsoft ABI for UBSan to work on
   # Windows.
   config.available_features.remove('cxxabi')
 
 # Allow tests to use REQUIRES=stable-runtime.  For use when you cannot use XFAIL
 # because the test hangs or fails on one configuration and not the other.
 if config.target_arch.startswith('arm') == False:
   config.available_features.add('stable-runtime')
+
+if config.host_os == 'Linux':
+  config.available_features.add('vptr-validation')