[tbb-commits] [tor-browser] 04/14: Bug 1791975 - Don't sweep realms that were allocated during incremental GC r=jandem, a=dmeehan

[gitolite role]

This is an automated email from the git hooks/post-receive script.

richard pushed a commit to branch tor-browser-91.13.0esr-11.5-1
in repository tor-browser.

commit 7a592af7a3bea8c876a51a5313f934e8b6594cc8
Author: Jon Coppeard <jcoppeard at mozilla.com>
AuthorDate: Mon Oct 17 17:09:07 2022 +0000

    Bug 1791975 - Don't sweep realms that were allocated during incremental GC r=jandem, a=dmeehan
    
    When marking a BaseShape we mark its global, and we read the pointer to that
    global from the realm. If a realm doesn't have a live global we can sweep the
    realm but there may still be pointers to it in base shapes and these are left
    dangling.
    
    This happens when we hit OOM while creating a global during an incremental GC.
    The BaseShape survives because it was allocated after the start of the GC. The
    global itself is never successfully created and so the realm doesn't have a
    live global and is swept. In this case, we trigger UAF when we try to compact
    the heap and trace the base shape.
    
    The patch adds an extra case for keeping a realm alive if it was created during
    an incremental GC. This matches the way that GC things are not collected if
    they are allocated after the start of a GC.
    
    Differential Revision: https://phabricator.services.mozilla.com/D158022
---
 js/src/gc/GC.cpp      |  3 +++
 js/src/vm/Realm-inl.h | 11 ++++++++---
 js/src/vm/Realm.cpp   |  2 ++
 js/src/vm/Realm.h     |  2 ++
 4 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/js/src/gc/GC.cpp b/js/src/gc/GC.cpp
index 14c4951c5b87..ad82b84c0652 100644
--- a/js/src/gc/GC.cpp
+++ b/js/src/gc/GC.cpp
@@ -6767,6 +6767,9 @@ void GCRuntime::finishCollection() {
   for (GCZonesIter zone(this); !zone.done(); zone.next()) {
     zone->changeGCState(Zone::Finished, Zone::NoGC);
     zone->notifyObservingDebuggers();
+    for (RealmsInZoneIter realm(zone); !realm.done(); realm.next()) {
+      realm->clearAllocatedDuringGC();
+    }
   }
 
 #ifdef JS_GC_ZEAL
diff --git a/js/src/vm/Realm-inl.h b/js/src/vm/Realm-inl.h
index 2663cac04a67..e0b902d9b9d8 100644
--- a/js/src/vm/Realm-inl.h
+++ b/js/src/vm/Realm-inl.h
@@ -46,9 +46,14 @@ inline bool JS::Realm::hasLiveGlobal() const {
 }
 
 inline bool JS::Realm::marked() const {
-  // Preserve this Realm if it has a live global or if it has been entered (to
-  // ensure we don't destroy the Realm while we're allocating its global).
-  return hasLiveGlobal() || hasBeenEnteredIgnoringJit();
+  // The Realm survives in the following cases:
+  //  - its global is live
+  //  - it has been entered (to ensure we don't destroy the Realm while we're
+  //    allocating its global)
+  //  - it was allocated after the start of an incremental GC (as there may be
+  //    pointers to it from other GC things)
+  return hasLiveGlobal() || hasBeenEnteredIgnoringJit() ||
+         allocatedDuringIncrementalGC_;
 }
 
 /* static */ inline js::ObjectRealm& js::ObjectRealm::get(const JSObject* obj) {
diff --git a/js/src/vm/Realm.cpp b/js/src/vm/Realm.cpp
index 53b7670cda33..abe25ec31835 100644
--- a/js/src/vm/Realm.cpp
+++ b/js/src/vm/Realm.cpp
@@ -62,6 +62,8 @@ Realm::Realm(Compartment* comp, const JS::RealmOptions& options)
       varNames_(zone_),
       randomKeyGenerator_(runtime_->forkRandomKeyGenerator()),
       debuggers_(zone_),
+      allocatedDuringIncrementalGC_(zone_->isGCMarkingOrSweeping() ||
+                                    zone_->isGCFinished()),
       wasm(runtime_) {
   MOZ_ASSERT_IF(creationOptions_.mergeable(),
                 creationOptions_.invisibleToDebugger());
diff --git a/js/src/vm/Realm.h b/js/src/vm/Realm.h
index 1f8852befc72..5f7763c7ed52 100644
--- a/js/src/vm/Realm.h
+++ b/js/src/vm/Realm.h
@@ -411,6 +411,7 @@ class JS::Realm : public JS::shadow::Realm {
 
   bool isSelfHostingRealm_ = false;
   bool isSystem_ = false;
+  bool allocatedDuringIncrementalGC_;
 
   js::UniquePtr<js::coverage::LCovRealm> lcovRealm_ = nullptr;
 
@@ -636,6 +637,7 @@ class JS::Realm : public JS::shadow::Realm {
   }
 
   inline bool marked() const;
+  void clearAllocatedDuringGC() { allocatedDuringIncrementalGC_ = false; }
 
   /*
    * The principals associated with this realm. Note that the same several

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.