Fix a use-after-free bug on Win32 when calling lookupSymbol() (#62484)

Author: grynspan

stdlib/public/core/KeyPath.swift

@@ -3893,8 +3893,15 @@ internal func _instantiateKeyPathBuffer(
 
 #if SWIFT_ENABLE_REFLECTION
 
-@_silgen_name("swift_keyPath_dladdr")
-fileprivate func keypath_dladdr(_: UnsafeRawPointer) -> UnsafePointer<CChar>?
+@_silgen_name("swift_keyPath_copySymbolName")
+fileprivate func keyPath_copySymbolName(
+  _: UnsafeRawPointer
+) -> UnsafePointer<CChar>?
+
+@_silgen_name("swift_keyPath_freeSymbolName")
+fileprivate func keyPath_freeSymbolName(
+  _: UnsafePointer<CChar>?
+) -> Void
 
 @_silgen_name("swift_keyPathSourceString")
 fileprivate func demangle(
@@ -3908,7 +3915,10 @@ fileprivate func dynamicLibraryAddress<Base, Leaf>(
 ) -> String {
   let getter: ComputedAccessorsPtr.Getter<Base, Leaf> = pointer.getter()
   let pointer = unsafeBitCast(getter, to: UnsafeRawPointer.self)
-  if let cString = keypath_dladdr(UnsafeRawPointer(pointer)) {
+  if let cString = keyPath_copySymbolName(UnsafeRawPointer(pointer)) {
+    defer {
+      keyPath_freeSymbolName(cString)
+    }
     if let demangled = demangle(name: cString)
       .map({ pointer in
         defer {

stdlib/public/runtime/CMakeLists.txt

@@ -54,6 +54,7 @@ set(swift_runtime_sources
     ImageInspectionCOFF.cpp
     ImageInspectionStatic.cpp
     ImageInspectionWasm.cpp
+    SymbolInfo.cpp
     KeyPaths.cpp
     KnownMetadata.cpp
     Metadata.cpp

stdlib/public/runtime/Errors.cpp

@@ -83,14 +83,14 @@ static bool getSymbolNameAddr(llvm::StringRef libraryName,
                               std::string &symbolName, uintptr_t &addrOut) {
   // If we failed to find a symbol and thus dlinfo->dli_sname is nullptr, we
   // need to use the hex address.
-  bool hasUnavailableAddress = syminfo.symbolName == nullptr;
+  bool hasUnavailableAddress = syminfo.getSymbolName() == nullptr;
 
   if (hasUnavailableAddress) {
     return false;
   }
 
   // Ok, now we know that we have some sort of "real" name. Set the outAddr.
-  addrOut = uintptr_t(syminfo.symbolAddress);
+  addrOut = uintptr_t(syminfo.getSymbolAddress());
 
   // First lets try to demangle using cxxabi. If this fails, we will try to
   // demangle with swift. We are taking advantage of __cxa_demangle actually
@@ -109,7 +109,7 @@ static bool getSymbolNameAddr(llvm::StringRef libraryName,
     dwFlags |= UNDNAME_32_BIT_DECODE;
 #endif
 
-    return UnDecorateSymbolName(syminfo.symbolName.get(), szUndName,
+    return UnDecorateSymbolName(syminfo.getSymbolName(), szUndName,
                                 sizeof(szUndName), dwFlags);
   });
 
@@ -120,7 +120,7 @@ static bool getSymbolNameAddr(llvm::StringRef libraryName,
 #else
   int status;
   char *demangled =
-      abi::__cxa_demangle(syminfo.symbolName.get(), 0, 0, &status);
+      abi::__cxa_demangle(syminfo.getSymbolName(), 0, 0, &status);
   if (status == 0) {
     assert(demangled != nullptr &&
            "If __cxa_demangle succeeds, demangled should never be nullptr");
@@ -135,7 +135,7 @@ static bool getSymbolNameAddr(llvm::StringRef libraryName,
   // Otherwise, try to demangle with swift. If swift fails to demangle, it will
   // just pass through the original output.
   symbolName = demangleSymbolAsString(
-      syminfo.symbolName.get(), strlen(syminfo.symbolName.get()),
+      syminfo.getSymbolName(), strlen(syminfo.getSymbolName()),
       Demangle::DemangleOptions::SimplifiedUIDemangleOptions());
   return true;
 }
@@ -155,7 +155,7 @@ void swift::dumpStackTraceEntry(unsigned index, void *framePC,
   // library name here. Avoid using StringRef::rsplit because its definition
   // is not provided in the header so that it requires linking with
   // libSupport.a.
-  llvm::StringRef libraryName{syminfo.fileName};
+  llvm::StringRef libraryName{syminfo.getFilename()};
   libraryName = libraryName.substr(libraryName.rfind('/')).substr(1);
 
   // Next we get the symbol name that we are going to use in our backtrace.
@@ -170,7 +170,8 @@ void swift::dumpStackTraceEntry(unsigned index, void *framePC,
   if (foundSymbol) {
     offset = ptrdiff_t(uintptr_t(framePC) - symbolAddr);
   } else {
-    offset = ptrdiff_t(uintptr_t(framePC) - uintptr_t(syminfo.baseAddress));
+    auto baseAddress = syminfo.getBaseAddress();
+    offset = ptrdiff_t(uintptr_t(framePC) - uintptr_t(baseAddress));
     symbolAddr = uintptr_t(framePC);
     symbolName = "<unavailable>";
   }

stdlib/public/runtime/ImageInspection.h

@@ -35,38 +35,9 @@
 #include <Windows.h>
 #endif
 
-namespace swift {
+#include "SymbolInfo.h"
 
-/// This is a platform independent version of Dl_info from dlfcn.h
-#if defined(__cplusplus)
-
-template <typename T>
-struct null_deleter {
-  void operator()(T *) const {}
-  void operator()(typename std::remove_cv<T>::type *value) const {}
-};
-
-template <typename T>
-struct free_deleter {
-  void operator()(T *value) const {
-    free(const_cast<typename std::remove_cv<T>::type *>(value));
-  }
-  void operator()(typename std::remove_cv<T>::type *value) const {
-    free(value);
-  }
-};
-
-struct SymbolInfo {
-  const char *fileName;
-  void *baseAddress;
-#if defined(_WIN32)
-  std::unique_ptr<const char, free_deleter<const char>> symbolName;
-#else
-  std::unique_ptr<const char, null_deleter<const char>> symbolName;
-#endif
-  void *symbolAddress;
-};
-#endif
+namespace swift {
 
 /// Load the metadata from the image necessary to find protocols by name.
 void initializeProtocolLookup();
@@ -112,8 +83,6 @@ void addImageAccessibleFunctionsBlockCallbackUnsafe(const void *baseAddress,
                                                     const void *start,
                                                     uintptr_t size);
 
-int lookupSymbol(const void *address, SymbolInfo *info);
-
 #if defined(_WIN32)
 /// Configure the environment to allow calling into the Debug Help library.
 ///

stdlib/public/runtime/ImageInspectionCOFF.cpp

@@ -27,46 +27,6 @@
 
 using namespace swift;
 
-int swift::lookupSymbol(const void *address, SymbolInfo *info) {
-#if defined(__CYGWIN__)
-  Dl_info dlinfo;
-  if (dladdr(address, &dlinfo) == 0) {
-    return 0;
-  }
-
-  info->fileName = dlinfo.dli_fname;
-  info->baseAddress = dlinfo.dli_fbase;
-  info->symbolName = dli_info.dli_sname;
-  info->symbolAddress = dli_saddr;
-  return 1;
-#elif defined(_WIN32)
-  return _swift_win32_withDbgHelpLibrary([&] (HANDLE hProcess) {
-    static const constexpr size_t kSymbolMaxNameLen = 1024;
-
-    if (!hProcess) {
-      return 0;
-    }
-
-    SYMBOL_INFO_PACKAGE symbol = {};
-    symbol.si.SizeOfStruct = sizeof(SYMBOL_INFO);
-    symbol.si.MaxNameLen = MAX_SYM_NAME;
-    if (SymFromAddr(hProcess, reinterpret_cast<const DWORD64>(address),
-                    nullptr, &symbol.si) == FALSE) {
-      return 0;
-    }
-
-    info->fileName = NULL;
-    info->baseAddress = reinterpret_cast<void *>(symbol.si.ModBase);
-    info->symbolName.reset(_strdup(symbol.si.Name));
-    info->symbolAddress = reinterpret_cast<void *>(symbol.si.Address);
-
-    return 1;
-  });
-#else
-  return 0;
-#endif // defined(__CYGWIN__) || defined(_WIN32)
-}
-
 #if defined(_WIN32)
 static LazyMutex mutex;
 static HANDLE dbgHelpHandle = nullptr;

stdlib/public/runtime/ImageInspectionCommon.cpp

@@ -65,8 +65,8 @@ static void fixupMetadataSectionBaseAddress(swift::MetadataSections *sections) {
     // We need to fix up the base address. We'll need a known-good address in
     // the same image: `sections` itself will work nicely.
     swift::SymbolInfo symbolInfo;
-    if (lookupSymbol(sections, &symbolInfo) && symbolInfo.baseAddress) {
-        sections->baseAddress.store(symbolInfo.baseAddress,
+    if (lookupSymbol(sections, &symbolInfo) && symbolInfo.getBaseAddress()) {
+        sections->baseAddress.store(symbolInfo.getBaseAddress(),
                                     std::memory_order_relaxed);
     }
   }
@@ -192,8 +192,8 @@ const char *
 swift_getMetadataSectionName(const swift::MetadataSections *section) {
   swift::SymbolInfo info;
   if (lookupSymbol(section, &info)) {
-    if (info.fileName) {
-      return info.fileName;
+    if (info.getFilename()) {
+      return info.getFilename();
     }
   }
   return "";
@@ -205,7 +205,7 @@ void swift_getMetadataSectionBaseAddress(const swift::MetadataSections *section,
                                          void const **out_expected) {
   swift::SymbolInfo info;
   if (lookupSymbol(section, &info)) {
-    *out_actual = info.baseAddress;
+    *out_actual = info.getBaseAddress();
   } else {
     *out_actual = nullptr;
   }

stdlib/public/runtime/ImageInspectionELF.cpp

@@ -20,23 +20,6 @@
 
 #if defined(__ELF__)
 
-#include "swift/shims/MetadataSections.h"
 #include "ImageInspection.h"
-#include <dlfcn.h>
-
-using namespace swift;
-
-int swift::lookupSymbol(const void *address, SymbolInfo *info) {
-  Dl_info dlinfo;
-  if (dladdr(address, &dlinfo) == 0) {
-    return 0;
-  }
-
-  info->fileName = dlinfo.dli_fname;
-  info->baseAddress = dlinfo.dli_fbase;
-  info->symbolName.reset(dlinfo.dli_sname);
-  info->symbolAddress = dlinfo.dli_saddr;
-  return 1;
-}
 
 #endif // defined(__ELF__)

stdlib/public/runtime/ImageInspectionMachO.cpp

@@ -171,20 +171,5 @@ void swift::initializeAccessibleFunctionsLookup() {
                        addImageAccessibleFunctionsBlockCallbackUnsafe>);
 }
 
-#if SWIFT_STDLIB_HAS_DLADDR
-int swift::lookupSymbol(const void *address, SymbolInfo *info) {
-  Dl_info dlinfo;
-  if (dladdr(address, &dlinfo) == 0) {
-    return 0;
-  }
-
-  info->fileName = dlinfo.dli_fname;
-  info->baseAddress = dlinfo.dli_fbase;
-  info->symbolName.reset(dlinfo.dli_sname);
-  info->symbolAddress = dlinfo.dli_saddr;
-  return 1;
-}
-#endif
-
 #endif // defined(__APPLE__) && defined(__MACH__) &&
        // !defined(SWIFT_RUNTIME_STATIC_IMAGE_INSPECTION)

stdlib/public/runtime/ImageInspectionWasm.cpp

@@ -23,13 +23,4 @@
 
 using namespace swift;
 
-int swift::lookupSymbol(const void *address, SymbolInfo *info) {
-  // Currently, Wasm doesn't have a standard stable ABI for exporting address <->
-  // symbol table, it's work in progress. Also, there is no API to access such
-  // information from Wasm binary side. It's accessible only from host VM. 
-  // See https://github.com/WebAssembly/tool-conventions/blob/main/DynamicLinking.md
-  // Seems reasonable to use a stub for now.
-  return 0;
-}
-
 #endif // defined(__wasm__)

stdlib/public/runtime/MetadataLookup.cpp

@@ -1105,13 +1105,11 @@ _gatherGenericParameters(const ContextDescriptor *context,
 
       str += "_gatherGenericParameters: context: ";
 
-#if SWIFT_STDLIB_HAS_DLADDR
       SymbolInfo contextInfo;
       if (lookupSymbol(context, &contextInfo)) {
-        str += contextInfo.symbolName.get();
+        str += contextInfo.getSymbolName();
         str += " ";
       }
-#endif
 
       char *contextStr;
       swift_asprintf(&contextStr, "%p", context);

stdlib/public/runtime/ProtocolConformance.cpp

@@ -124,12 +124,12 @@ static const char *class_getName(const ClassMetadata* type) {
 }
 
 template<> void ProtocolConformanceDescriptor::dump() const {
+  SymbolInfo info;
   auto symbolName = [&](const void *addr) -> const char * {
-    SymbolInfo info;
     int ok = lookupSymbol(addr, &info);
-    if (!ok)
+    if (!ok || !info.getSymbolName())
       return "<unknown addr>";
-    return info.symbolName.get();
+    return info.getSymbolName();
   };
 
   switch (auto kind = getTypeKind()) {

stdlib/public/runtime/ReflectionMirror.cpp

@@ -1114,14 +1114,18 @@ id swift_reflectionMirror_quickLookObject(OpaqueValue *value, const Metadata *T)
 }
 #endif
 
-SWIFT_CC(swift)
-SWIFT_RUNTIME_STDLIB_INTERNAL const char *swift_keyPath_dladdr(void *address) {
+SWIFT_CC(swift) SWIFT_RUNTIME_STDLIB_INTERNAL
+const char *swift_keyPath_copySymbolName(void *address) {
   SymbolInfo info;
-  if (lookupSymbol(address, &info) == 0) {
-    return 0;
-  } else {
-    return info.symbolName.get();
+  if (lookupSymbol(address, &info) && info.getSymbolName()) {
+    return strdup(info.getSymbolName());
   }
+  return 0;
+}
+
+SWIFT_CC(swift) SWIFT_RUNTIME_STDLIB_INTERNAL
+void swift_keyPath_freeSymbolName(const char *symbolName) {
+  free(const_cast<char *>(symbolName));
 }
 
 SWIFT_CC(swift)