Merge pull request #37863 from mikeash/metadata-reader-sane-sizes

[Reflection] Sanity-check metadata sizes in MetadataReader before reading.

Author: mikeash

include/swift/Remote/MetadataReader.h

@@ -170,21 +170,24 @@ class MetadataReader {
   using StoredSize = typename Runtime::StoredSize;
 
 private:
+  /// The maximum number of bytes to read when reading metadata. Anything larger
+  /// will automatically return failure. This prevents us from reading absurd
+  /// amounts of data when we encounter corrupt values for sizes/counts.
+  static const uint64_t MaxMetadataSize = 1048576; // 1MB
+
   /// A cache of built types, keyed by the address of the type.
   std::unordered_map<StoredPointer, BuiltType> TypeCache;
 
-  using MetadataRef = RemoteRef<TargetMetadata<Runtime>>;
-  using OwnedMetadataRef =
-    std::unique_ptr<const TargetMetadata<Runtime>, delete_with_free>;
+  using MetadataRef = RemoteRef<const TargetMetadata<Runtime>>;
+  using OwnedMetadataRef = MemoryReader::ReadBytesResult;
 
   /// A cache of read type metadata, keyed by the address of the metadata.
   std::unordered_map<StoredPointer, OwnedMetadataRef>
     MetadataCache;
 
-  using ContextDescriptorRef = RemoteRef<TargetContextDescriptor<Runtime>>;
-  using OwnedContextDescriptorRef =
-    std::unique_ptr<const TargetContextDescriptor<Runtime>,
-                    delete_with_free>;
+  using ContextDescriptorRef =
+      RemoteRef<const TargetContextDescriptor<Runtime>>;
+  using OwnedContextDescriptorRef = MemoryReader::ReadBytesResult;
 
   /// A reference to a context descriptor that may be in an unloaded image.
   class ParentContextDescriptorRef {
@@ -984,7 +987,9 @@ class MetadataReader {
 
     auto cached = ContextDescriptorCache.find(address);
     if (cached != ContextDescriptorCache.end())
-      return ContextDescriptorRef(address, cached->second.get());
+      return ContextDescriptorRef(
+          address, reinterpret_cast<const TargetContextDescriptor<Runtime> *>(
+                       cached->second.get()));
 
     // Read the flags to figure out how much space we should read.
     ContextDescriptorFlags flags;
@@ -993,9 +998,9 @@ class MetadataReader {
       return nullptr;
 
     TypeContextDescriptorFlags typeFlags(flags.getKindSpecificFlags());
-    unsigned baseSize = 0;
-    unsigned genericHeaderSize = sizeof(GenericContextDescriptorHeader);
-    unsigned metadataInitSize = 0;
+    uint64_t baseSize = 0;
+    uint64_t genericHeaderSize = sizeof(GenericContextDescriptorHeader);
+    uint64_t metadataInitSize = 0;
     bool hasVTable = false;
 
     auto readMetadataInitSize = [&]() -> unsigned {
@@ -1059,7 +1064,7 @@ class MetadataReader {
     // Determine the full size of the descriptor. This is reimplementing a fair
     // bit of TrailingObjects but for out-of-process; maybe there's a way to
     // factor the layout stuff out...
-    unsigned genericsSize = 0;
+    uint64_t genericsSize = 0;
     if (flags.isGeneric()) {
       GenericContextDescriptorHeader header;
       auto headerAddr = address
@@ -1077,7 +1082,7 @@ class MetadataReader {
           * sizeof(TargetGenericRequirementDescriptor<Runtime>);
     }
 
-    unsigned vtableSize = 0;
+    uint64_t vtableSize = 0;
     if (hasVTable) {
       TargetVTableDescriptorHeader<Runtime> header;
       auto headerAddr = address
@@ -1092,22 +1097,20 @@ class MetadataReader {
       vtableSize = sizeof(header)
         + header.VTableSize * sizeof(TargetMethodDescriptor<Runtime>);
     }
-    
-    unsigned size = baseSize + genericsSize + metadataInitSize + vtableSize;
-    auto buffer = (uint8_t *)malloc(size);
-    if (buffer == nullptr) {
+
+    uint64_t size = baseSize + genericsSize + metadataInitSize + vtableSize;
+    if (size > MaxMetadataSize)
       return nullptr;
-    }
-    if (!Reader->readBytes(RemoteAddress(address), buffer, size)) {
-      free(buffer);
+    auto readResult = Reader->readBytes(RemoteAddress(address), size);
+    if (!readResult)
       return nullptr;
-    }
 
-    auto descriptor
-      = reinterpret_cast<TargetContextDescriptor<Runtime> *>(buffer);
+    auto descriptor =
+        reinterpret_cast<const TargetContextDescriptor<Runtime> *>(
+            readResult.get());
 
     ContextDescriptorCache.insert(
-      std::make_pair(address, OwnedContextDescriptorRef(descriptor)));
+        std::make_pair(address, std::move(readResult)));
     return ContextDescriptorRef(address, descriptor);
   }
 
@@ -1633,7 +1636,9 @@ class MetadataReader {
   MetadataRef readMetadata(StoredPointer address) {
     auto cached = MetadataCache.find(address);
     if (cached != MetadataCache.end())
-      return MetadataRef(address, cached->second.get());
+      return MetadataRef(address,
+                         reinterpret_cast<const TargetMetadata<Runtime> *>(
+                             cached->second.get()));
 
     StoredPointer KindValue = 0;
     if (!Reader->readInteger(RemoteAddress(address), &KindValue))
@@ -1807,18 +1812,15 @@ class MetadataReader {
   }
 
   MetadataRef _readMetadata(StoredPointer address, size_t sizeAfter) {
-    auto size = sizeAfter;
-    uint8_t *buffer = (uint8_t *) malloc(size);
-    if (!buffer)
+    if (sizeAfter > MaxMetadataSize)
       return nullptr;
-
-    if (!Reader->readBytes(RemoteAddress(address), buffer, size)) {
-      free(buffer);
+    auto readResult = Reader->readBytes(RemoteAddress(address), sizeAfter);
+    if (!readResult)
       return nullptr;
-    }
 
-    auto metadata = reinterpret_cast<TargetMetadata<Runtime>*>(buffer);
-    MetadataCache.insert(std::make_pair(address, OwnedMetadataRef(metadata)));
+    auto metadata =
+        reinterpret_cast<const TargetMetadata<Runtime> *>(readResult.get());
+    MetadataCache.insert(std::make_pair(address, std::move(readResult)));
     return MetadataRef(address, metadata);
   }
 

test/stdlib/symbol-visibility-linux.test-sh

@@ -19,6 +19,9 @@
 // RUN:             -e _ZNSt6vectorIjSaIjEE13_M_insert_auxIJjEEEvN9__gnu_cxx17__normal_iteratorIPjS1_EEDpOT_ \
 // RUN:             -e _ZNSt6vectorISt10unique_ptrIKvSt8functionIFvPS1_EEESaIS6_EE19_M_emplace_back_auxIJS6_EEEvDpOT_ \
 // RUN:             -e _ZNSt6vectorISt10unique_ptrIKvSt8functionIFvPS1_EEESaIS6_EE17_M_realloc_insertIJS6_EEEvN9__gnu_cxx17__normal_iteratorIPS6_S8_EEDpOT_ \
+// RUN:             -e _ZNSt10_HashtableImSt4pairIKmSt10unique_ptrIKvSt8functionIFvPS3_EEEESaIS9_ENSt8__detail10_Select1stESt8equal_toImESt4hashImENSB_18_Mod_range_hashingENSB_20_Default_ranged_hashENSB_20_Prime_rehash_policyENSB_17_Hashtable_traitsILb0ELb0ELb1EEEE10_M_emplaceIJS0_ImS8_EEEES0_INSB_14_Node_iteratorIS9_Lb0ELb0EEEbESt17integral_constantIbLb1EEDpOT_ \
+// RUN:             -e _ZNSt10_HashtableImSt4pairIKmSt10unique_ptrIKvSt8functionIFvPS3_EEEESaIS9_ENSt8__detail10_Select1stESt8equal_toImESt4hashImENSB_18_Mod_range_hashingENSB_20_Default_ranged_hashENSB_20_Prime_rehash_policyENSB_17_Hashtable_traitsILb0ELb0ELb1EEEE13_M_rehash_auxEmSt17integral_constantIbLb1EE \
+// RUN:             -e _ZNSt10_HashtableImSt4pairIKmSt10unique_ptrIKvSt8functionIFvPS3_EEEESaIS9_ENSt8__detail10_Select1stESt8equal_toImESt4hashImENSB_18_Mod_range_hashingENSB_20_Default_ranged_hashENSB_20_Prime_rehash_policyENSB_17_Hashtable_traitsILb0ELb0ELb1EEEED2Ev \
 // RUN:             -e _ZNSt3_V28__rotateIPcEET_S2_S2_S2_St26random_access_iterator_tag \
 // RUN:             -e _ZN9__gnu_cxx12__to_xstringINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEcEET_PFiPT0_mPKS8_P13__va_list_tagEmSB_z \
 // RUN:             -e _ZN9__gnu_cxx12__to_xstringINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEcEET_PFiPT0_mPKS8_St9__va_listEmSB_z \
@@ -38,6 +41,9 @@
 // RUN:             -e _ZNSt6vectorIjSaIjEE13_M_insert_auxIJjEEEvN9__gnu_cxx17__normal_iteratorIPjS1_EEDpOT_ \
 // RUN:             -e _ZNSt6vectorISt10unique_ptrIKvSt8functionIFvPS1_EEESaIS6_EE19_M_emplace_back_auxIJS6_EEEvDpOT_ \
 // RUN:             -e _ZNSt6vectorISt10unique_ptrIKvSt8functionIFvPS1_EEESaIS6_EE17_M_realloc_insertIJS6_EEEvN9__gnu_cxx17__normal_iteratorIPS6_S8_EEDpOT_ \
+// RUN:             -e _ZNSt10_HashtableImSt4pairIKmSt10unique_ptrIKvSt8functionIFvPS3_EEEESaIS9_ENSt8__detail10_Select1stESt8equal_toImESt4hashImENSB_18_Mod_range_hashingENSB_20_Default_ranged_hashENSB_20_Prime_rehash_policyENSB_17_Hashtable_traitsILb0ELb0ELb1EEEE10_M_emplaceIJS0_ImS8_EEEES0_INSB_14_Node_iteratorIS9_Lb0ELb0EEEbESt17integral_constantIbLb1EEDpOT_ \
+// RUN:             -e _ZNSt10_HashtableImSt4pairIKmSt10unique_ptrIKvSt8functionIFvPS3_EEEESaIS9_ENSt8__detail10_Select1stESt8equal_toImESt4hashImENSB_18_Mod_range_hashingENSB_20_Default_ranged_hashENSB_20_Prime_rehash_policyENSB_17_Hashtable_traitsILb0ELb0ELb1EEEE13_M_rehash_auxEmSt17integral_constantIbLb1EE \
+// RUN:             -e _ZNSt10_HashtableImSt4pairIKmSt10unique_ptrIKvSt8functionIFvPS3_EEEESaIS9_ENSt8__detail10_Select1stESt8equal_toImESt4hashImENSB_18_Mod_range_hashingENSB_20_Default_ranged_hashENSB_20_Prime_rehash_policyENSB_17_Hashtable_traitsILb0ELb0ELb1EEEED2Ev \
 // RUN:             -e _ZNSt3_V28__rotateIPcEET_S2_S2_S2_St26random_access_iterator_tag \
 // RUN:             -e _ZN9__gnu_cxx12__to_xstringINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEcEET_PFiPT0_mPKS8_P13__va_list_tagEmSB_z \
 // RUN:             -e _ZN9__gnu_cxx12__to_xstringINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEcEET_PFiPT0_mPKS8_St9__va_listEmSB_z \