Medium firefox Logic Error 🔧 Commit mapped

Overview

Medium
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
Impactmoderate
DescriptionSite isolation issue in the Graphics component
ComponentCore
Bug ClassLogic Error
Tracker2048345
Fix commit8d21c36c0b90 (firefox) +91/-35
CISA KEVNot listed
CreditedSteven Julian
Disclosed2026-07-21

Changed Functions

FunctionChangeNotes
if
mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/RemoteSurfaceAllocator.java
modified

Files Changed

  • mobile/android/geckoview/src/main/aidl/org/mozilla/gecko/process/IChildProcess.aidl
  • mobile/android/geckoview/src/main/aidl/org/mozilla/gecko/process/IProcessManager.aidl
  • mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/RemoteSurfaceAllocator.java
  • mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/SurfaceAllocator.java
  • mobile/android/geckoview/src/main/java/org/mozilla/gecko/process/GeckoProcessManager.java
  • mobile/android/geckoview/src/main/java/org/mozilla/gecko/process/GeckoServiceChildProcess.java
  • mobile/android/geckoview/src/main/java/org/mozilla/gecko/process/GeckoServiceGpuProcess.java
diff --git a/mobile/android/geckoview/src/main/aidl/org/mozilla/gecko/process/IChildProcess.aidl b/mobile/android/geckoview/src/main/aidl/org/mozilla/gecko/process/IChildProcess.aidl
index 2e76c9eac77..9060795fecf 100644
--- a/mobile/android/geckoview/src/main/aidl/org/mozilla/gecko/process/IChildProcess.aidl
+++ b/mobile/android/geckoview/src/main/aidl/org/mozilla/gecko/process/IChildProcess.aidl
@@ -9,6 +9,7 @@ import org.mozilla.gecko.gfx.ISurfaceAllocator;
 import org.mozilla.gecko.process.IProcessManager;
 
 import android.os.Bundle;
+import android.os.IBinder;
 import android.os.ParcelFileDescriptor;
 
 interface IChildProcess {
@@ -39,6 +40,9 @@ interface IChildProcess {
      * consumed by the GPU process. Must only be called for a GPU child process type.
      * @param allocatorId A unique ID used to identify the GPU process instance the allocator
      *     belongs to.
+     * @param client An IBinder identifying the process connecting to the surface allocator. For
+     *     content processes this will have been passed from the content process through the parent,
+     *     and finally to the GPU process here.
      */
-    ISurfaceAllocator getSurfaceAllocator(int allocatorId);
+    ISurfaceAllocator getSurfaceAllocator(int allocatorId, in IBinder client);
 }
diff --git a/mobile/android/geckoview/src/main/aidl/org/mozilla/gecko/process/IProcessManager.aidl b/mobile/android/geckoview/src/main/aidl/org/mozilla/gecko/process/IProcessManager.aidl
index b75f3171242..224df64fc82 100644
--- a/mobile/android/geckoview/src/main/aidl/org/mozilla/gecko/process/IProcessManager.aidl
+++ b/mobile/android/geckoview/src/main/aidl/org/mozilla/gecko/process/IProcessManager.aidl
@@ -7,8 +7,10 @@ package org.mozilla.gecko.process;
 import org.mozilla.gecko.IGeckoEditableChild;
 import org.mozilla.gecko.gfx.ISurfaceAllocator;
 
+import android.os.IBinder;
+
 interface IProcessManager {
     void getEditableParent(in IGeckoEditableChild child, long contentId, long tabId);
     // Returns the interface that child processes should use to allocate Surfaces.
-    ISurfaceAllocator getSurfaceAllocator();
+    ISurfaceAllocator getSurfaceAllocator(in IBinder client);
 }
diff --git a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/RemoteSurfaceAllocator.java b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/RemoteSurfaceAllocator.java
index 31b5999100c..2d9b08e3b4f 100644
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/RemoteSurfaceAllocator.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/RemoteSurfaceAllocator.java
@@ -4,39 +4,60 @@
 
 package org.mozilla.gecko.gfx;
 
+import android.os.IBinder;
+import android.os.RemoteException;
+import android.util.LongSparseArray;
 import java.util.concurrent.atomic.AtomicInteger;
 import org.mozilla.gecko.GeckoThread;
 
-public final class RemoteSurfaceAllocator extends ISurfaceAllocator.Stub {
+public final class RemoteSurfaceAllocator extends ISurfaceAllocator.Stub
+    implements IBinder.DeathRecipient {
   private static final String LOGTAG = "RemoteSurfaceAllocator";
 
-  private static RemoteSurfaceAllocator mInstance;
-
+  /// Unique ID identifying the process this instance belongs to, which must be 0 for the parent
+  /// process. This is used so that following a GPU process shutdown, surface handles allocated by
+  /// the new compositor process (either a new GPU process, or the parent process) do not clash with
+  /// surface handles allocated by a previous GPU process.
   private final int mAllocatorId;
   /// Monotonically increasing counter used to generate unique handles
   /// for each SurfaceTexture by combining with mAllocatorId.
   private static AtomicInteger sNextHandle = new AtomicInteger(1);
 
+  ///  List of Surface handles owned by this instance.
+  private final LongSparseArray<Boolean> mOwnedHandles = new LongSparseArray<Boolean>();
+  ///  Whether the client is still connected to this allocator.
+  private boolean mClientConnected = true;
+
   /**
-   * Retrieves the singleton allocator instance for this process.
+   * Retrieves the allocator instance for the provided client.
    *
    * @param allocatorId A unique ID identifying the process this instance belongs to, which must be
-   *     0 for the parent process instance.
+   *     0 for a parent process instance.
+   * @param client An IBinder identifying the process for which we will be allocating surfaces.
    */
-  public static synchronized RemoteSurfaceAllocator getInstance(final int allocatorId) {
-    if (mInstance == null && GeckoThread.isStateAtLeast(GeckoThread.State.JNI_READY)) {
-      mInstance = new RemoteSurfaceAllocator(allocatorId);
+  public static RemoteSurfaceAllocator create(final int allocatorId, final IBinder client) {
+    if (GeckoThread.isStateAtLeast(GeckoThread.State.JNI_READY)) {
+      try {
+        return new RemoteSurfaceAllocator(allocatorId, client);
+      } catch (final RemoteException ignored) {
+      }
     }
-    return mInstance;
+    return null;
   }
 
-  private RemoteSurfaceAllocator(final int allocatorId) {
+  private RemoteSurfaceAllocator(final int allocatorId, final IBinder client)
+      throws RemoteException {
     mAllocatorId = allocatorId;
+    client.linkToDeath(this, 0);
   }
 
   @Override
-  public GeckoSurface acquireSurface(
+  public synchronized GeckoSurface acquireSurface(
       final int width, final int height, final boolean singleBufferMode) {
+    if (!mClientConnected) {
+      return null;
+    }
+
     final long handle = ((long) mAllocatorId << 32) | sNextHandle.getAndIncrement();
     final GeckoSurfaceTexture gst = GeckoSurfaceTexture.acquire(singleBufferMode, handle);
 
@@ -48,11 +69,15 @@ public final class RemoteSurfaceAllocator extends ISurfaceAllocator.Stub {
       gst.setDefaultBufferSize(width, height);
     }
 
+    mOwnedHandles.put(handle, true);
     return new GeckoSurface(gst);
   }
 
   @Override
-  public void releaseSurface(final long handle) {
+  public synchronized void releaseSurface(final long handle) {
+    ensureOwned(handle);
+    mOwnedHandles.remove(handle);
+
     final GeckoSurfaceTexture gst = GeckoSurfaceTexture.lookup(handle);
     if (gst != null) {
       gst.decrementUse();
@@ -60,7 +85,9 @@ public final class RemoteSurfaceAllocator extends ISurfaceAllocator.Stub {
   }
 
   @Override
-  public void configureSync(final SyncConfig config) {
+  public synchronized void configureSync(final SyncConfig config) {
+    ensureOwned(config.sourceTextureHandle);
+
     final GeckoSurfaceTexture gst = GeckoSurfaceTexture.lookup(config.sourceTextureHandle);
     if (gst != null) {
       gst.configureSnapshot(config.targetSurface, config.width, config.height);
@@ -68,10 +95,30 @@ public final class RemoteSurfaceAllocator extends ISurfaceAllocator.Stub {
   }
 
   @Override
-  public void sync(final long handle) {
+  public synchronized void sync(final long handle) {
+    ensureOwned(handle);
+
     final GeckoSurfaceTexture gst = GeckoSurfaceTexture.lookup(handle);
     if (gst != null) {
       gst.takeSnapshot();
     }
   }
+
+  @Override
+  public synchronized void binderDied() {
+    mClientConnected = false;
+    for (int i = 0; i < mOwnedHandles.size(); i++) {
+      final GeckoSurfaceTexture gst = GeckoSurfaceTexture.lookup(mOwnedHandles.keyAt(i));
+      if (gst != null) {
+        gst.decrementUse();
+      }
+    }
+    mOwnedHandles.clear();
+  }
+
+  private void ensureOwned(final long handle) {
+    if (mOwnedHandles.indexOfKey(handle) < 0) {
+      throw new SecurityException("Surface handle is not owned by this allocator session");
+    }
+  }
 }
diff --git a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/SurfaceAllocator.java b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/SurfaceAllocator.java
index d4da49a0ad5..63d5f91cb03 100644
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/SurfaceAllocator.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/SurfaceAllocator.java
@@ -4,6 +4,7 @@
 
 package org.mozilla.gecko.gfx;
 
+import android.os.Binder;
 import android.os.IBinder;
 import android.os.RemoteException;
 import android.util.Log;
@@ -17,6 +18,7 @@ import org.mozilla.gecko.process.GeckoServiceChildProcess;
   private static final String LOGTAG = "SurfaceAllocator";
 
   private static ISurfaceAllocator sAllocator;
+  private static final IBinder sClient = new Binder();
 
   // Keep a reference to all allocated Surfaces, so that we can release them if we lose the
   // connection to the allocator service.
@@ -30,9 +32,9 @@ import org.mozilla.gecko.process.GeckoServiceChildProcess;
 
     try {
       if (GeckoAppShell.isParentProcess()) {
-        sAllocator = GeckoProcessManager.getInstance().getSurfaceAllocator();
Loading diff…