diff --git a/core/java/android/os/ParcelFileDescriptor.java b/core/java/android/os/ParcelFileDescriptor.java index 68460a481a7d1..d348f0b37ba3c 100644 --- a/core/java/android/os/ParcelFileDescriptor.java +++ b/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; } diff --git a/core/java/com/android/internal/gmscompat/dynamite/GmsDynamiteClientHooks.java b/core/java/com/android/internal/gmscompat/dynamite/GmsDynamiteClientHooks.java index 47a6173ec5c97..f87224c34e818 100644 --- a/core/java/com/android/internal/gmscompat/dynamite/GmsDynamiteClientHooks.java +++ b/core/java/com/android/internal/gmscompat/dynamite/GmsDynamiteClientHooks.java @@ -44,10 +44,10 @@ 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); } @@ -55,7 +55,7 @@ public static ApkAssets loadAssetsFromPath(String path, int flags, AssetsProvide 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); } diff --git a/core/java/com/android/internal/gmscompat/fileservice/GmsCoreFileServerClientHooks.java b/core/java/com/android/internal/gmscompat/fileservice/GmsCoreFileServerClientHooks.java index c16f3fd97e2c0..44ee34a96dd7b 100644 --- a/core/java/com/android/internal/gmscompat/fileservice/GmsCoreFileServerClientHooks.java +++ b/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 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;