gmscompat: use fresh file proxy fds for app opens

core/java/android/os/ParcelFileDescriptor.java

@@ -354,7 +354,7 @@ private static FileDescriptor openInternal(File file, int mode) throws FileNotFo
         final String path = file.getPath();
 
         if (GmsCoreFileServerClientHooks.isEnabled()) {
-            FileDescriptor override = GmsCoreFileServerClientHooks.openParcelFileDescriptorHook(path);
+            FileDescriptor override = GmsCoreFileServerClientHooks.openParcelFileDescriptorHook(path, flags);
             if (override != null) {
                 return override;
             }

core/java/com/android/internal/gmscompat/dynamite/GmsDynamiteClientHooks.java

@@ -44,18 +44,18 @@ public static void maybeInit(String auth) {
 
     // ApkAssets#loadFromPath(String, int, AssetsProvider)
     public static ApkAssets loadAssetsFromPath(String path, int flags, AssetsProvider assets) throws IOException {
-        if (!GmsCoreFileServerClientHooks.isInGmsCoreDeDataDir(path)) {
+        if (!GmsCoreFileServerClientHooks.isInGmsCoreDeDataDirAndShouldCacheFds(path)) {
             return null;
         }
-        FileDescriptor fd = GmsCoreFileServerClientHooks.openFile(path);
+        FileDescriptor fd = GmsCoreFileServerClientHooks.openCachedFile(path);
         if (fd == null) {
             throw new IOException("unable to open " + path);
         }
         // no need to dup the fd, ApkAssets does it itself
         return ApkAssets.loadFromFd(fd, path, flags, assets);
     }
 
-    // Replaces file paths of Dynamite modules with "/proc/self/fd" file descriptor references
+    // Replaces file paths of Dynamite modules with "/gmscompat_fd" file descriptor references
     // DelegateLastClassLoader#maybeModifyClassLoaderPath(String, Boolean)
     public static String maybeModifyClassLoaderPath(String path, Boolean nativeLibsPathB) {
         if (path == null) {
@@ -70,7 +70,7 @@ public static String maybeModifyClassLoaderPath(String path, Boolean nativeLibsP
 
         for (int i = 0; i < pathParts.length; ++i) {
             String pathPart = pathParts[i];
-            if (!GmsCoreFileServerClientHooks.isInGmsCoreDeDataDir(pathPart)) {
+            if (!GmsCoreFileServerClientHooks.isInGmsCoreDeDataDirAndShouldCacheFds(pathPart)) {
                 continue;
             }
             // defined in bionic/linker/linker_utils.cpp kZipFileSeparator
@@ -86,7 +86,7 @@ public static String maybeModifyClassLoaderPath(String path, Boolean nativeLibsP
                 filePath = pathPart;
                 nativeLibRelPath = null;
             }
-            FileDescriptor fd = GmsCoreFileServerClientHooks.openFile(filePath);
+            FileDescriptor fd = GmsCoreFileServerClientHooks.openCachedFile(filePath);
             if (fd == null) {
                 throw new IllegalStateException("unable to open " + filePath);
             }

core/java/com/android/internal/gmscompat/fileservice/GmsCoreFileServerClientHooks.java

@@ -9,8 +9,6 @@
 import android.os.IBinder;
 import android.os.ParcelFileDescriptor;
 import android.os.RemoteException;
-import android.system.ErrnoException;
-import android.system.Os;
 import android.util.ArrayMap;
 import android.util.Log;
 
@@ -22,8 +20,10 @@
 import java.util.ArrayList;
 
 import libcore.io.IoBridge;
+import libcore.io.IoUtils;
 
-import static android.system.OsConstants.F_DUPFD_CLOEXEC;
+import static android.system.OsConstants.O_ACCMODE;
+import static android.system.OsConstants.O_RDONLY;
 
 public class GmsCoreFileServerClientHooks {
     private static final String TAG = "GmsCoreFileServerClientHooks";
@@ -57,13 +57,9 @@ public static void init() {
         File.lastModifiedHook = GmsCoreFileServerClientHooks::getFileLastModified;
         IoBridge.openFdInterceptor = new IoBridge.OpenFdInterceptor() {
             @Override
-            public FileDescriptor maybeInterceptOpenFd(String path, int flags) throws ErrnoException {
-                if (path.startsWith(gmsCoreDeDataPrefix) || path.startsWith(gmsCoreCeDataPrefix)) {
-                    FileDescriptor fd = openFile(path);
-                    if (fd == null) {
-                        return null;
-                    }
-                    return Os.dup(fd);
+            public FileDescriptor maybeInterceptOpenFd(String path, int flags) {
+                if (isInGmsCoreDataDir(path) && shouldProxyOpen(flags)) {
+                    return openFreshFileDescriptor(path);
                 }
                 return null;
             }
@@ -75,50 +71,57 @@ public static long getFileLastModified(File file) {
         final String path = file.getPath();
 
         if (isInGmsCoreDataDir(path)) {
-            FileDescriptor fd = openFile(path);
-            if (fd != null) {
-                String fdPath = "/proc/self/fd/" + fd.getInt$();
-                return new File(fdPath).lastModified();
+            if (shouldCacheForLastModified(path)) {
+                FileDescriptor fd = openCachedFile(path);
+                return fd != null ? getFileLastModified(fd) : 0L;
+            }
+
+            ParcelFileDescriptor pfd = openFreshParcelFileDescriptor(path);
+            if (pfd != null) {
+                try {
+                    return getFileLastModified(pfd.getFileDescriptor());
+                } finally {
+                    IoUtils.closeQuietly(pfd);
+                }
             }
         }
         return 0L;
     }
 
-    public static boolean isInGmsCoreDeDataDir(String path) {
+    private static long getFileLastModified(FileDescriptor fd) {
+        return new File("/proc/self/fd/" + fd.getInt$()).lastModified();
+    }
+
+    public static boolean isInGmsCoreDeDataDirAndShouldCacheFds(String path) {
         return path.startsWith(gmsCoreDeDataPrefix);
     }
 
+    private static boolean shouldCacheForLastModified(String path) {
+        // Use cache for Dynamite modules which are served as .apk files
+        return isInGmsCoreDeDataDirAndShouldCacheFds(path) && isApkContainerPath(path);
+    }
+
+    private static boolean isApkContainerPath(String path) {
+        return path.endsWith(".apk");
+    }
+
     public static boolean isInGmsCoreDataDir(String path) {
         return path.startsWith(gmsCoreDeDataPrefix) || path.startsWith(gmsCoreCeDataPrefix);
     }
 
-    public static FileDescriptor openParcelFileDescriptorHook(String path) {
-        if (!isInGmsCoreDataDir(path)) {
+    public static FileDescriptor openParcelFileDescriptorHook(String path, int flags) {
+        if (!isInGmsCoreDataDir(path) || !shouldProxyOpen(flags)) {
             return null;
         }
 
-        FileDescriptor fd = openFile(path);
-        if (fd == null) {
-            return null;
-        }
-
-        int dupFd;
-        try {
-            dupFd = Os.fcntlInt(fd, F_DUPFD_CLOEXEC, 0);
-        } catch (ErrnoException e) {
-            throw new RuntimeException(e);
-        }
-
-        var dupJfd = new FileDescriptor();
-        dupJfd.setInt$(dupFd);
-        return dupJfd;
+        return openFreshFileDescriptor(path);
     }
 
     // Returned file descriptor should never be closed because it may be accessed at any time by the native code
     @Nullable
-    public static FileDescriptor openFile(String path) {
+    public static FileDescriptor openCachedFile(String path) {
         if (DEBUG) {
-            Log.d(TAG, "path " + path, new Throwable());
+            Log.d(TAG, "cached path " + path, new Throwable());
         }
         try {
             ArrayMap<String, ParcelFileDescriptor> cache = pfdCache;
@@ -142,6 +145,40 @@ public static FileDescriptor openFile(String path) {
         }
     }
 
+    private static boolean shouldProxyOpen(int flags) {
+        return (flags & O_ACCMODE) == O_RDONLY;
+    }
+
+    @Nullable
+    private static FileDescriptor openFreshFileDescriptor(String path) {
+        ParcelFileDescriptor pfd = openFreshParcelFileDescriptor(path);
+        if (pfd == null) {
+            return null;
+        }
+
+        FileDescriptor fd = new FileDescriptor();
+        fd.setInt$(pfd.detachFd());
+        return fd;
+    }
+
+    @Nullable
+    private static ParcelFileDescriptor openFreshParcelFileDescriptor(String path) {
+        if (DEBUG) {
+            Log.d(TAG, "fresh path " + path, new Throwable());
+        }
+        try {
+            IFileProxyService service;
+            synchronized (pfdCache) {
+                service = getFileProxyService();
+            }
+            return service.openFile(path);
+        } catch (RemoteException e) {
+            // FileProxyService never forwards exceptions to minimize the information leaks,
+            // this is a very rare "binder died" exception
+            throw e.rethrowAsRuntimeException();
+        }
+    }
+
     @GuardedBy("pfdCache")
     private static IFileProxyService getFileProxyService() {
         IFileProxyService cache = fileProxyService;