[lld] 3fe5c3b - [lld-macho] Fix use-after-free in loadDylib()

[Jez Ng via llvm-commits]

Author: Jez Ng
Date: 2021-04-23T18:05:49-04:00
New Revision: 3fe5c3b0189f51868493bbbc6768ef3afbdfb492

URL: https://github.com/llvm/llvm-project/commit/3fe5c3b0189f51868493bbbc6768ef3afbdfb492
DIFF: https://github.com/llvm/llvm-project/commit/3fe5c3b0189f51868493bbbc6768ef3afbdfb492.diff

LOG: [lld-macho] Fix use-after-free in loadDylib()

We were taking a reference to a value in `loadedDylibs`, which in turn
called `make<DylibFile>()`, which could then recursively call
`loadDylibs`, which would then potentially resize `loadedDylibs` and
invalidate that reference.

Fixes PR50101.

Reviewed By: #lld-macho, oontvoo

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

Added: 
    

Modified: 
    lld/MachO/DriverUtils.cpp
    lld/MachO/InputFiles.h

Removed: 
    


################################################################################
diff  --git a/lld/MachO/DriverUtils.cpp b/lld/MachO/DriverUtils.cpp
index bbbc339c3177..e0f6a353c03e 100644
--- a/lld/MachO/DriverUtils.cpp
+++ b/lld/MachO/DriverUtils.cpp
@@ -188,8 +188,8 @@ static DenseMap<CachedHashStringRef, DylibFile *> loadedDylibs;
 Optional<DylibFile *> macho::loadDylib(MemoryBufferRef mbref,
                                        DylibFile *umbrella,
                                        bool isBundleLoader) {
-  StringRef path = mbref.getBufferIdentifier();
-  DylibFile *&file = loadedDylibs[CachedHashStringRef(path)];
+  CachedHashStringRef path(mbref.getBufferIdentifier());
+  DylibFile *file = loadedDylibs[path];
   if (file)
     return file;
 
@@ -209,6 +209,11 @@ Optional<DylibFile *> macho::loadDylib(MemoryBufferRef mbref,
            magic == file_magic::macho_bundle);
     file = make<DylibFile>(mbref, umbrella, isBundleLoader);
   }
+  // Note that DylibFile's ctor may recursively invoke loadDylib(), which can
+  // cause loadedDylibs to get resized and its iterators invalidated. As such,
+  // we redo the key lookup here instead of caching an iterator from our earlier
+  // lookup at the start of the function.
+  loadedDylibs[path] = file;
   return file;
 }
 

diff  --git a/lld/MachO/InputFiles.h b/lld/MachO/InputFiles.h
index 4db93bec8611..066d4506644c 100644
--- a/lld/MachO/InputFiles.h
+++ b/lld/MachO/InputFiles.h
@@ -160,7 +160,7 @@ class DylibFile : public InputFile {
   bool isBundleLoader;
 
 private:
-  template <class LP> void parse(DylibFile *umbrella = nullptr);
+  template <class LP> void parse(DylibFile *umbrella);
 };
 
 // .a file