[compiler-rt] 5595249 - [scudo][standalone] Add chunk ownership function

[Kostya Kortchinsky via llvm-commits]

Author: Kostya Kortchinsky
Date: 2019-12-03T08:32:26-08:00
New Revision: 5595249e48ef83bae5f2e61c0190332534902051

URL: https://github.com/llvm/llvm-project/commit/5595249e48ef83bae5f2e61c0190332534902051
DIFF: https://github.com/llvm/llvm-project/commit/5595249e48ef83bae5f2e61c0190332534902051.diff

LOG: [scudo][standalone] Add chunk ownership function

Summary:
In order to be compliant with tcmalloc's extension ownership
determination function, we have to expose a function that will
say if a chunk was allocated by us.

As to whether or not this has security consequences: someone
able to call this function repeatedly could use it to determine
secrets (cookie) or craft a valid header. So this should not be
exposed directly to untrusted user input.

Add related tests.

Additionally clang-format caught a few things to change.

Reviewers: hctim, pcc, cferris, eugenis, vitalybuka

Subscribers: JDevlieghere, jfb, #sanitizers, llvm-commits

Tags: #sanitizers, #llvm

Differential Revision: https://reviews.llvm.org/D70908

Added: 
    

Modified: 
    compiler-rt/lib/scudo/standalone/chunk.h
    compiler-rt/lib/scudo/standalone/combined.h
    compiler-rt/lib/scudo/standalone/tests/combined_test.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/scudo/standalone/chunk.h b/compiler-rt/lib/scudo/standalone/chunk.h
index dff13db8a6c8..f4d68b3ac6c4 100644
--- a/compiler-rt/lib/scudo/standalone/chunk.h
+++ b/compiler-rt/lib/scudo/standalone/chunk.h
@@ -91,8 +91,7 @@ inline AtomicPackedHeader *getAtomicHeader(void *Ptr) {
                                                 getHeaderSize());
 }
 
-inline
-const AtomicPackedHeader *getConstAtomicHeader(const void *Ptr) {
+inline const AtomicPackedHeader *getConstAtomicHeader(const void *Ptr) {
   return reinterpret_cast<const AtomicPackedHeader *>(
       reinterpret_cast<uptr>(Ptr) - getHeaderSize());
 }
@@ -118,9 +117,8 @@ inline void storeHeader(u32 Cookie, void *Ptr,
   atomic_store_relaxed(getAtomicHeader(Ptr), NewPackedHeader);
 }
 
-inline
-void loadHeader(u32 Cookie, const void *Ptr,
-                UnpackedHeader *NewUnpackedHeader) {
+inline void loadHeader(u32 Cookie, const void *Ptr,
+                       UnpackedHeader *NewUnpackedHeader) {
   PackedHeader NewPackedHeader = atomic_load_relaxed(getConstAtomicHeader(Ptr));
   *NewUnpackedHeader = bit_cast<UnpackedHeader>(NewPackedHeader);
   if (UNLIKELY(NewUnpackedHeader->Checksum !=
@@ -141,8 +139,8 @@ inline void compareExchangeHeader(u32 Cookie, void *Ptr,
     reportHeaderRace(Ptr);
 }
 
-inline
-bool isValid(u32 Cookie, const void *Ptr, UnpackedHeader *NewUnpackedHeader) {
+inline bool isValid(u32 Cookie, const void *Ptr,
+                    UnpackedHeader *NewUnpackedHeader) {
   PackedHeader NewPackedHeader = atomic_load_relaxed(getConstAtomicHeader(Ptr));
   *NewUnpackedHeader = bit_cast<UnpackedHeader>(NewPackedHeader);
   return NewUnpackedHeader->Checksum ==

diff  --git a/compiler-rt/lib/scudo/standalone/combined.h b/compiler-rt/lib/scudo/standalone/combined.h
index f33c9150148f..b355a4746fae 100644
--- a/compiler-rt/lib/scudo/standalone/combined.h
+++ b/compiler-rt/lib/scudo/standalone/combined.h
@@ -457,6 +457,18 @@ template <class Params> class Allocator {
     Stats.get(S);
   }
 
+  // Returns true if the pointer provided was allocated by the current
+  // allocator instance, which is compliant with tcmalloc's ownership concept.
+  // A corrupted chunk will not be reported as owned, which is WAI.
+  bool isOwned(const void *Ptr) {
+    initThreadMaybe();
+    if (!Ptr || !isAligned(reinterpret_cast<uptr>(Ptr), MinAlignment))
+      return false;
+    Chunk::UnpackedHeader Header;
+    return Chunk::isValid(Cookie, Ptr, &Header) &&
+           Header.State == Chunk::State::Allocated;
+  }
+
 private:
   using SecondaryT = typename Params::Secondary;
   typedef typename PrimaryT::SizeClassMap SizeClassMap;
@@ -468,6 +480,9 @@ template <class Params> class Allocator {
   static const uptr MaxAllowedMallocSize =
       FIRST_32_SECOND_64(1UL << 31, 1ULL << 40);
 
+  static_assert(MinAlignment >= sizeof(Chunk::PackedHeader),
+                "Minimal alignment must at least cover a chunk header.");
+
   // Constants used by the chunk iteration mechanism.
   static const u32 BlockMarker = 0x44554353U;
   static const uptr InvalidChunk = ~static_cast<uptr>(0);

diff  --git a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
index f38e9826863b..fec5f864aeb7 100644
--- a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
@@ -32,6 +32,12 @@ template <class Config> static void testAllocator() {
                                                            Deleter);
   Allocator->reset();
 
+  EXPECT_FALSE(Allocator->isOwned(&Mutex));
+  EXPECT_FALSE(Allocator->isOwned(&Allocator));
+  scudo::u64 StackVariable = 0x42424242U;
+  EXPECT_FALSE(Allocator->isOwned(&StackVariable));
+  EXPECT_EQ(StackVariable, 0x42424242U);
+
   constexpr scudo::uptr MinAlignLog = FIRST_32_SECOND_64(3U, 4U);
 
   // This allocates and deallocates a bunch of chunks, with a wide range of
@@ -46,6 +52,7 @@ template <class Config> static void testAllocator() {
         const scudo::uptr Size = (1U << SizeLog) + Delta;
         void *P = Allocator->allocate(Size, Origin, Align);
         EXPECT_NE(P, nullptr);
+        EXPECT_TRUE(Allocator->isOwned(P));
         EXPECT_TRUE(scudo::isAligned(reinterpret_cast<scudo::uptr>(P), Align));
         EXPECT_LE(Size, Allocator->getUsableSize(P));
         memset(P, 0xaa, Size);