Moderate firefox Logic Error 🔧 Commit mapped

Overview

Moderate
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Impactmoderate
DescriptionUse-after-free in the Graphics component
ComponentGraphics
Bug ClassLogic Error
Tracker2058618
Fix commit9606294ac903 (firefox) +77/-47
CISA KEVNot listed
CreditedMozilla
Disclosed2026-09-15

Changed Functions

FunctionChangeNotes
if
gfx/thebes/CoreTextFontList.cpp
modified
if
gfx/thebes/gfxDWriteFontList.cpp
modified
if
gfx/thebes/gfxFT2FontList.cpp
modified
if
gfx/thebes/gfxFcPlatformFontList.cpp
modified
if
gfx/thebes/gfxFontEntry.cpp
modified
if
gfx/thebes/gfxFontEntry.h
modified
if
gfx/thebes/gfxUserFontSet.h
modified

Files Changed

  • gfx/thebes/CoreTextFontList.cpp
  • gfx/thebes/gfxDWriteFontList.cpp
  • gfx/thebes/gfxFT2FontList.cpp
  • gfx/thebes/gfxFcPlatformFontList.cpp
  • gfx/thebes/gfxFontEntry.cpp
  • gfx/thebes/gfxFontEntry.h
  • gfx/thebes/gfxUserFontSet.cpp
  • gfx/thebes/gfxUserFontSet.h
diff --git a/gfx/thebes/CoreTextFontList.cpp b/gfx/thebes/CoreTextFontList.cpp
index cbadfbe1f52..f9a1ef3360c 100644
--- a/gfx/thebes/CoreTextFontList.cpp
+++ b/gfx/thebes/CoreTextFontList.cpp
@@ -240,7 +240,7 @@ static CFStringRef CreateCFStringForString(const nsACString& aSrc) {
 
 nsresult CTFontEntry::ReadCMAP(FontInfoData* aFontInfoData) {
   // attempt this once, if errors occur leave a blank cmap
-  if (mCharacterMap || mShmemCharacterMap) {
+  if (HasCharacterMap()) {
     return NS_OK;
   }
 
@@ -354,6 +354,7 @@ nsresult CTFontEntry::ReadCMAP(FontInfoData* aFontInfoData) {
   if (setCharMap) {
     // Temporarily retain charmap, until the shared version is
     // ready for use.
+    AutoWriteLock lock(mLock);
     if (mCharacterMap.compareExchange(nullptr, charmap.get())) {
       charmap.get()->AddRef();
     }
@@ -361,7 +362,7 @@ nsresult CTFontEntry::ReadCMAP(FontInfoData* aFontInfoData) {
 
   LOG_FONTLIST(("(fontlist-cmap) name: %s, size: %zu hash: %8.8x%s\n",
                 mName.get(), charmap->SizeOfIncludingThis(moz_malloc_size_of),
-                charmap->mHash, mCharacterMap == charmap ? " new" : ""));
+                charmap->mHash, GetCharacterMap() == charmap ? " new" : ""));
   if (LOG_CMAPDATA_ENABLED()) {
     char prefix[256];
     SprintfLiteral(prefix, "(cmapdata) name: %.220s", mName.get());
diff --git a/gfx/thebes/gfxDWriteFontList.cpp b/gfx/thebes/gfxDWriteFontList.cpp
index 7357d6bbc96..327c596d125 100644
--- a/gfx/thebes/gfxDWriteFontList.cpp
+++ b/gfx/thebes/gfxDWriteFontList.cpp
@@ -532,7 +532,7 @@ nsresult gfxDWriteFontEntry::ReadCMAP(FontInfoData* aFontInfoData) {
   AUTO_PROFILER_LABEL("gfxDWriteFontEntry::ReadCMAP", GRAPHICS);
 
   // attempt this once, if errors occur leave a blank cmap
-  if (mCharacterMap || mShmemCharacterMap) {
+  if (HasCharacterMap()) {
     return NS_OK;
   }
 
@@ -586,6 +586,7 @@ nsresult gfxDWriteFontEntry::ReadCMAP(FontInfoData* aFontInfoData) {
   if (setCharMap) {
     // Temporarily retain charmap, until the shared version is
     // ready for use.
+    AutoWriteLock lock(mLock);
     if (mCharacterMap.compareExchange(nullptr, charmap.get())) {
       charmap.get()->AddRef();
     }
@@ -593,7 +594,7 @@ nsresult gfxDWriteFontEntry::ReadCMAP(FontInfoData* aFontInfoData) {
 
   LOG_FONTLIST(("(fontlist-cmap) name: %s, size: %zu hash: %8.8x%s\n",
                 mName.get(), charmap->SizeOfIncludingThis(moz_malloc_size_of),
-                charmap->mHash, mCharacterMap == charmap ? " new" : ""));
+                charmap->mHash, GetCharacterMap() == charmap ? " new" : ""));
   if (LOG_CMAPDATA_ENABLED()) {
     char prefix[256];
     SprintfLiteral(prefix, "(cmapdata) name: %.220s", mName.get());
diff --git a/gfx/thebes/gfxFT2FontList.cpp b/gfx/thebes/gfxFT2FontList.cpp
index 5675f5fca44..95502b7a736 100644
--- a/gfx/thebes/gfxFT2FontList.cpp
+++ b/gfx/thebes/gfxFT2FontList.cpp
@@ -380,7 +380,7 @@ FT2FontEntry* gfxFT2Font::GetFontEntry() {
 // properly.
 
 nsresult FT2FontEntry::ReadCMAP(FontInfoData* aFontInfoData) {
-  if (mCharacterMap || mShmemCharacterMap) {
+  if (HasCharacterMap()) {
     return NS_OK;
   }
 
@@ -459,6 +459,7 @@ nsresult FT2FontEntry::ReadCMAP(FontInfoData* aFontInfoData) {
     charmap = new gfxCharacterMap(0);
   }
   if (setCharMap) {
+    AutoWriteLock lock(mLock);
     if (mCharacterMap.compareExchange(nullptr, charmap.get())) {
       // We forget rather than addref because we don't use the charmap below.
       charmap.forget().leak();
diff --git a/gfx/thebes/gfxFcPlatformFontList.cpp b/gfx/thebes/gfxFcPlatformFontList.cpp
index 97ed1b72f69..2d3e76e0e7d 100644
--- a/gfx/thebes/gfxFcPlatformFontList.cpp
+++ b/gfx/thebes/gfxFcPlatformFontList.cpp
@@ -556,7 +556,7 @@ gfxFontconfigFontEntry::AutoHBFace gfxFontconfigFontEntry::GetHBFace() {
 
 nsresult gfxFontconfigFontEntry::ReadCMAP(FontInfoData* aFontInfoData) {
   // attempt this once, if errors occur leave a blank cmap
-  if (mCharacterMap) {
+  if (HasCharacterMap()) {
     return NS_OK;
   }
 
@@ -600,6 +600,7 @@ nsresult gfxFontconfigFontEntry::ReadCMAP(FontInfoData* aFontInfoData) {
     charmap = new gfxCharacterMap(0);
   }
   if (setCharMap) {
+    AutoWriteLock lock(mLock);
     if (mCharacterMap.compareExchange(nullptr, charmap.get())) {
       charmap.get()->AddRef();
     }
@@ -607,7 +608,7 @@ nsresult gfxFontconfigFontEntry::ReadCMAP(FontInfoData* aFontInfoData) {
 
   LOG_FONTLIST(("(fontlist-cmap) name: %s, size: %zu hash: %8.8x%s\n",
                 mName.get(), charmap->SizeOfIncludingThis(moz_malloc_size_of),
-                charmap->mHash, mCharacterMap == charmap ? " new" : ""));
+                charmap->mHash, GetCharacterMap() == charmap ? " new" : ""));
   if (LOG_CMAPDATA_ENABLED()) {
     char prefix[256];
     SprintfLiteral(prefix, "(cmapdata) name: %.220s", mName.get());
diff --git a/gfx/thebes/gfxFontEntry.cpp b/gfx/thebes/gfxFontEntry.cpp
index 77b8f1f0dd2..13801ce1052 100644
--- a/gfx/thebes/gfxFontEntry.cpp
+++ b/gfx/thebes/gfxFontEntry.cpp
@@ -5,6 +5,10 @@
 #include "gfxFontEntry.h"
 
 #include <algorithm>
+#include <chrono>
+#include <thread>
+
+#include "nsXULAppAPI.h"
 
 #include "COLRFonts.h"
 #include "ThebesRLBox.h"
@@ -140,22 +144,24 @@ bool gfxFontEntry::TrySetShmemCharacterMap() {
 }
 
 bool gfxFontEntry::TestCharacterMap(uint32_t aCh) {
-  if (!mCharacterMap && !mShmemCharacterMap) {
+  if (!HasCharacterMap()) {
     ReadCMAP();
-    MOZ_ASSERT(mCharacterMap || mShmemCharacterMap,
-               "failed to initialize character map");
+    MOZ_ASSERT(HasCharacterMap(), "failed to initialize character map");
+  }
+  if (const auto* map = GetShmemCharacterMap()) {
+    return map->test(aCh);
   }
-  return mShmemCharacterMap ? GetShmemCharacterMap()->test(aCh)
-                            : GetCharacterMap()->test(aCh);
+  AutoReadLock lock(mLock);
+  gfxCharacterMap* map = mCharacterMap;
+  return map ? map->test(aCh) : 0;
 }
 
 void gfxFontEntry::EnsureUVSMapInitialized() {
   // mUVSOffset will not be initialized
   // until cmap is initialized.
-  if (!mCharacterMap && !mShmemCharacterMap) {
+  if (!HasCharacterMap()) {
     ReadCMAP();
-    NS_ASSERTION(mCharacterMap || mShmemCharacterMap,
-                 "failed to initialize character map");
+    MOZ_ASSERT(HasCharacterMap(), "failed to initialize character map");
   }
 
   if (!mUVSOffset) {
@@ -209,6 +215,7 @@ bool gfxFontEntry::SupportsScriptInGSUB(const hb_tag_t* aScriptTags,
 
 nsresult gfxFontEntry::ReadCMAP(FontInfoData* aFontInfoData) {
   MOZ_ASSERT(false, "using default no-op implementation of ReadCMAP");
+  AutoWriteLock lock(mLock);
   RefPtr<gfxCharacterMap> cmap = new gfxCharacterMap(0);
   if (mCharacterMap.compareExchange(nullptr, cmap.get())) {
     cmap.forget().leak();  // mCharacterMap now owns the reference
@@ -1265,9 +1272,10 @@ void gfxFontEntry::AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf,
   aSizes->mFontListSize += mName.SizeOfExcludingThisIfUnshared(aMallocSizeOf);
 
   // cmaps are shared so only non-shared cmaps are included here
-  if (mCharacterMap && GetCharacterMap()->mBuildOnTheFly) {
-    aSizes->mCharMapsSize +=
-        GetCharacterMap()->SizeOfIncludingThis(aMallocSizeOf);
+  if (gfxCharacterMap* map = GetCharacterMap()) {
+    if (map->mBuildOnTheFly) {
+      aSizes->mCharMapsSize += map->SizeOfIncludingThis(aMallocSizeOf);
+    }
   }
 
   {
diff --git a/gfx/thebes/gfxFontEntry.h b/gfx/thebes/gfxFontEntry.h
index 68a3b03e648..725b37ad96c 100644
--- a/gfx/thebes/gfxFontEntry.h
+++ b/gfx/thebes/gfxFontEntry.h
@@ -301,19 +301,22 @@ class gfxFontEntry {
     return flag == LazyFlag::Yes;
   }
 
-  inline bool HasCharacter(uint32_t ch) {
-    if (mShmemCharacterMap) {
-      return GetShmemCharacterMap()->test(ch);
+  inline bool HasCharacter(uint32_t ch) MOZ_EXCLUDES(mLock) {
+    if (const auto* map = GetShmemCharacterMap()) {
+      return map->test(ch);
     }
-    if (mCharacterMap) {
-      if (mShmemFace && TrySetShmemCharacterMap()) {
-        // Forget our temporary local copy, now we can use the shared cmap
-        auto* oldCmap = mCharacterMap.exchange(nullptr);
-        NS_IF_RELEASE(oldCmap);
-        return GetShmemCharacterMap()->test(ch);
-      }
-      if (GetCharacterMap()->test(ch)) {
-        return true;
+    {
+      mozilla::AutoReadLock lock(mLock);
+      if (gfxCharacterMap* map = GetCharacterMap()) {
+        if (mShmemFace && TrySetShmemCharacterMap()) {
+          // Forget our temporary local copy, now we can use the shared cmap
+          auto* oldCmap = mCharacterMap.exchange(nullptr);
+          NS_IF_RELEASE(oldCmap);
+          return GetShmemCharacterMap()->test(ch);
+        }
+        if (map->test(ch)) {
+          return true;
+        }
       }
     }
     return TestCharacterMap(ch);
@@ -573,8 +576,19 @@ class gfxFontEntry {
   mutable mozilla::RWLock mLock;
   mutable mozilla::Mutex mFeatureInfoLock;
 
-  mozilla::Atomic<gfxCharacterMap*> mCharacterMap;  // strong ref
-  gfxCharacterMap* GetCharacterMap() const { return mCharacterMap; }
+  mozilla::Atomic<gfxCharacterMap*> mCharacterMap MOZ_GUARDED_BY(mLock);  // strong ref
+  gfxCharacterMap* GetCharacterMap() const {
+    mozilla::AutoReadLock lock(mLock);
+    return mCharacterMap;
+  }
+
+  // Check for presence of either shmem or local charmap.
+  bool HasCharacterMap() const MOZ_NO_THREAD_SAFETY_ANALYSIS {
+    // Although mCharacterMap is MOZ_GUARDED_BY(mLock), we don't lock here
+    // as it is an atomic var, and we're not holding on to dereferencing it,
+    // just checking whether it's non-null.
+    return mShmemCharacterMap || mCharacterMap;
+  }
 
   mozilla::fontlist::Face* mShmemFace = nullptr;
   const mozilla::fontlist::Family* mShmemFamily = nullptr;
@@ -737,7 +751,7 @@ class gfxFontEntry {
       FontInfoData* aFontInfoData, uint32_t& aUVSOffset);
 
   // helper for HasCharacter(), which is what client code should call
-  virtual bool TestCharacterMap(uint32_t aCh);
+  virtual bool TestCharacterMap(uint32_t aCh) MOZ_EXCLUDES(mLock);
 
   // Try to set mShmemCharacterMap, based on the char map in mShmemFace;
   // return true if successful, false if it remains null (maybe the parent
diff --git a/gfx/thebes/gfxUserFontSet.cpp b/gfx/thebes/gfxUserFontSet.cpp
index a9750c6bbe1..827b05416fe 100644
--- a/gfx/thebes/gfxUserFontSet.cpp
+++ b/gfx/thebes/gfxUserFontSet.cpp
@@ -102,20 +102,23 @@ gfxUserFontEntry::~gfxUserFontEntry() {
 
 bool gfxUserFontEntry::Matches(const nsTArray<gfxFontFaceSrc>& aFontFaceSrcList,
                                const gfxUserFontAttributes& aAttr) {
-  return mWeightRange == aAttr.mWeight && mWidthRange == aAttr.mWidth &&
-         mStyleRange == aAttr.mStyle &&
-         mFeatureSettings == aAttr.mFeatureSettings &&
-         mVariationSettings == aAttr.mVariationSettings &&
-         mLanguageOverride == aAttr.mLanguageOverride &&
-         mSrcList == aFontFaceSrcList && mFontDisplay == aAttr.mFontDisplay &&
-         mRangeFlags == aAttr.mRangeFlags &&
-         mAscentOverride == aAttr.mAscentOverride &&
-         mDescentOverride == aAttr.mDescentOverride &&
-         mLineGapOverride == aAttr.mLineGapOverride &&
-         mSizeAdjust == aAttr.mSizeAdjust &&
-         ((!aAttr.mUnicodeRanges && !mCharacterMap) ||
-          (aAttr.mUnicodeRanges && mCharacterMap &&
-           GetCharacterMap()->Equals(aAttr.mUnicodeRanges)));
+  if (!(mWeightRange == aAttr.mWeight && mWidthRange == aAttr.mWidth &&
+        mStyleRange == aAttr.mStyle &&
+        mFeatureSettings == aAttr.mFeatureSettings &&
+        mVariationSettings == aAttr.mVariationSettings &&
+        mLanguageOverride == aAttr.mLanguageOverride &&
+        mSrcList == aFontFaceSrcList && mFontDisplay == aAttr.mFontDisplay &&
+        mRangeFlags == aAttr.mRangeFlags &&
+        mAscentOverride == aAttr.mAscentOverride &&
+        mDescentOverride == aAttr.mDescentOverride &&
+        mLineGapOverride == aAttr.mLineGapOverride &&
+        mSizeAdjust == aAttr.mSizeAdjust)) {
+    return false;
+  }
+  RefPtr map = GetCharacterMap();
+  return (!aAttr.mUnicodeRanges && !map) ||
+      (aAttr.mUnicodeRanges && map &&
+       map->Equals(aAttr.mUnicodeRanges));
 }
 
 gfxFont* gfxUserFontEntry::CreateFontInstance(const gfxFontStyle* aFontStyle) {
diff --git a/gfx/thebes/gfxUserFontSet.h b/gfx/thebes/gfxUserFontSet.h
index f4a81d058ea..6be7d898790 100644
--- a/gfx/thebes/gfxUserFontSet.h
+++ b/gfx/thebes/gfxUserFontSet.h
@@ -643,6 +643,7 @@ class gfxUserFontEntry : public gfxFontEntry {
     auto* oldCmap = GetUnicodeRangeMap();
     if (oldCmap != aCharMap) {
       auto* newCmap = aCharMap.forget().take();
+      mozilla::AutoWriteLock lock(mLock);
       if (mCharacterMap.compareExchange(oldCmap, newCmap)) {
         NS_IF_RELEASE(oldCmap);
       } else {
Loading diff…