Put PackageInstaller in the drivers seat.

openWrite() and openRead() are very flexible APIs, but their design
means they can't take advantage of the recent FileUtils.copy()
optimizations that leverage in-kernel copying.

So add new write() and read() methods where the untrusted caller
hands an FD into the OS, and then PackageInstaller drives the actual
copying process, allowing it to use FileUtils.copy() to speed
up the copying process.  (Local benchmarks are showing a 24% speed
improvement.)

We still create a FileBridge to protect the session while an active
copy is happening.

Test: bit FrameworksCoreTests:android.os.FileUtilsTest
Test: vogar --mode app_process --benchmark frameworks/base/core/tests/benchmarks/src/android/os/FileUtilsBenchmark.java
Bug: 71932978, 25510838
Change-Id: Icc237b4c0f80d5d24b74a30f238b7fe505b856ce

diff --git a/core/java/android/content/pm/IPackageInstallerSession.aidl b/core/java/android/content/pm/IPackageInstallerSession.aidl
index 0b16852..8fddb99 100644 (file)
--- a/core/java/android/content/pm/IPackageInstallerSession.aidl
+++ b/core/java/android/content/pm/IPackageInstallerSession.aidl
@@ -26,9 +26,12 @@ interface IPackageInstallerSession {
     void addClientProgress(float progress);
 
     String[] getNames();
+
     ParcelFileDescriptor openWrite(String name, long offsetBytes, long lengthBytes);
     ParcelFileDescriptor openRead(String name);
 
+    void write(String name, long offsetBytes, long lengthBytes, in ParcelFileDescriptor fd);
+
     void removeSplit(String splitName);
 
     void close();

diff --git a/core/java/android/content/pm/PackageInstaller.java b/core/java/android/content/pm/PackageInstaller.java
index df677d2..d0be6c8 100644 (file)
--- a/core/java/android/content/pm/PackageInstaller.java
+++ b/core/java/android/content/pm/PackageInstaller.java
@@ -829,7 +829,19 @@ public class PackageInstaller {
             } catch (RemoteException e) {
                 throw e.rethrowFromSystemServer();
             }
+        }
 
+        /** {@hide} */
+        public void write(@NonNull String name, long offsetBytes, long lengthBytes,
+                @NonNull ParcelFileDescriptor fd) throws IOException {
+            try {
+                mSession.write(name, offsetBytes, lengthBytes, fd);
+            } catch (RuntimeException e) {
+                ExceptionUtils.maybeUnwrapIOException(e);
+                throw e;
+            } catch (RemoteException e) {
+                throw e.rethrowFromSystemServer();
+            }
         }
 
         /**

diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java
index a6ff4f7..3dd5a34 100644 (file)
--- a/services/core/java/com/android/server/pm/PackageInstallerSession.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java
@@ -77,6 +77,7 @@ import android.os.RevocableFileDescriptor;
 import android.os.UserHandle;
 import android.os.storage.StorageManager;
 import android.system.ErrnoException;
+import android.system.Int64Ref;
 import android.system.Os;
 import android.system.OsConstants;
 import android.system.StructStat;
@@ -575,14 +576,24 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
     @Override
     public ParcelFileDescriptor openWrite(String name, long offsetBytes, long lengthBytes) {
         try {
-            return openWriteInternal(name, offsetBytes, lengthBytes);
+            return doWriteInternal(name, offsetBytes, lengthBytes, null);
         } catch (IOException e) {
             throw ExceptionUtils.wrap(e);
         }
     }
 
-    private ParcelFileDescriptor openWriteInternal(String name, long offsetBytes, long lengthBytes)
-            throws IOException {
+    @Override
+    public void write(String name, long offsetBytes, long lengthBytes,
+            ParcelFileDescriptor fd) {
+        try {
+            doWriteInternal(name, offsetBytes, lengthBytes, fd);
+        } catch (IOException e) {
+            throw ExceptionUtils.wrap(e);
+        }
+    }
+
+    private ParcelFileDescriptor doWriteInternal(String name, long offsetBytes, long lengthBytes,
+            ParcelFileDescriptor incomingFd) throws IOException {
         // Quick sanity check of state, and allocate a pipe for ourselves. We
         // then do heavy disk allocation outside the lock, but this open pipe
         // will block any attempted install transitions.
@@ -636,7 +647,44 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
                 Os.lseek(targetFd, offsetBytes, OsConstants.SEEK_SET);
             }
 
-            if (PackageInstaller.ENABLE_REVOCABLE_FD) {
+            if (incomingFd != null) {
+                switch (Binder.getCallingUid()) {
+                    case android.os.Process.SHELL_UID:
+                    case android.os.Process.ROOT_UID:
+                        break;
+                    default:
+                        throw new SecurityException("Reverse mode only supported from shell");
+                }
+
+                // In "reverse" mode, we're streaming data ourselves from the
+                // incoming FD, which means we never have to hand out our
+                // sensitive internal FD. We still rely on a "bridge" being
+                // inserted above to hold the session active.
+                try {
+                    final Int64Ref last = new Int64Ref(0);
+                    FileUtils.copy(incomingFd.getFileDescriptor(), targetFd, (long progress) -> {
+                        if (params.sizeBytes > 0) {
+                            final long delta = progress - last.value;
+                            last.value = progress;
+                            addClientProgress((float) delta / (float) params.sizeBytes);
+                        }
+                    }, null, lengthBytes);
+                } finally {
+                    IoUtils.closeQuietly(targetFd);
+                    IoUtils.closeQuietly(incomingFd);
+
+                    // We're done here, so remove the "bridge" that was holding
+                    // the session active.
+                    synchronized (mLock) {
+                        if (PackageInstaller.ENABLE_REVOCABLE_FD) {
+                            mFds.remove(fd);
+                        } else {
+                            mBridges.remove(bridge);
+                        }
+                    }
+                }
+                return null;
+            } else if (PackageInstaller.ENABLE_REVOCABLE_FD) {
                 fd.init(mContext, targetFd);
                 return fd.getRevocableFileDescriptor();
             } else {

diff --git a/services/core/java/com/android/server/pm/PackageManagerShellCommand.java b/services/core/java/com/android/server/pm/PackageManagerShellCommand.java
index 686c4a5..758c9d5 100644 (file)
--- a/services/core/java/com/android/server/pm/PackageManagerShellCommand.java
+++ b/services/core/java/com/android/server/pm/PackageManagerShellCommand.java
@@ -16,6 +16,12 @@
 
 package com.android.server.pm;
 
+import static android.content.pm.PackageManager.INTENT_FILTER_DOMAIN_VERIFICATION_STATUS_ALWAYS;
+import static android.content.pm.PackageManager.INTENT_FILTER_DOMAIN_VERIFICATION_STATUS_ALWAYS_ASK;
+import static android.content.pm.PackageManager.INTENT_FILTER_DOMAIN_VERIFICATION_STATUS_ASK;
+import static android.content.pm.PackageManager.INTENT_FILTER_DOMAIN_VERIFICATION_STATUS_NEVER;
+import static android.content.pm.PackageManager.INTENT_FILTER_DOMAIN_VERIFICATION_STATUS_UNDEFINED;
+
 import android.accounts.IAccountManager;
 import android.app.ActivityManager;
 import android.app.ActivityManagerInternal;
@@ -32,8 +38,10 @@ import android.content.pm.IPackageManager;
 import android.content.pm.InstrumentationInfo;
 import android.content.pm.PackageInfo;
 import android.content.pm.PackageInstaller;
+import android.content.pm.PackageInstaller.SessionParams;
 import android.content.pm.PackageItemInfo;
 import android.content.pm.PackageManager;
+import android.content.pm.PackageManager.NameNotFoundException;
 import android.content.pm.PackageParser;
 import android.content.pm.PackageParser.ApkLite;
 import android.content.pm.PackageParser.PackageLite;
@@ -41,9 +49,6 @@ import android.content.pm.PackageParser.PackageParserException;
 import android.content.pm.ParceledListSlice;
 import android.content.pm.PermissionGroupInfo;
 import android.content.pm.PermissionInfo;
-import android.content.pm.PackageInstaller.SessionInfo;
-import android.content.pm.PackageInstaller.SessionParams;
-import android.content.pm.PackageManager.NameNotFoundException;
 import android.content.pm.ResolveInfo;
 import android.content.pm.UserInfo;
 import android.content.pm.VersionedPackage;
@@ -73,7 +78,6 @@ import android.util.PrintWriterPrinter;
 
 import com.android.internal.content.PackageHelper;
 import com.android.internal.util.ArrayUtils;
-import com.android.internal.util.SizedInputStream;
 import com.android.server.LocalServices;
 import com.android.server.SystemConfig;
 
@@ -81,9 +85,8 @@ import dalvik.system.DexFile;
 
 import libcore.io.IoUtils;
 
+import java.io.FileDescriptor;
 import java.io.IOException;
-import java.io.InputStream;
-import java.io.OutputStream;
 import java.io.PrintWriter;
 import java.net.URISyntaxException;
 import java.util.ArrayList;
@@ -95,12 +98,6 @@ import java.util.WeakHashMap;
 import java.util.concurrent.SynchronousQueue;
 import java.util.concurrent.TimeUnit;
 
-import static android.content.pm.PackageManager.INTENT_FILTER_DOMAIN_VERIFICATION_STATUS_ALWAYS;
-import static android.content.pm.PackageManager.INTENT_FILTER_DOMAIN_VERIFICATION_STATUS_ALWAYS_ASK;
-import static android.content.pm.PackageManager.INTENT_FILTER_DOMAIN_VERIFICATION_STATUS_ASK;
-import static android.content.pm.PackageManager.INTENT_FILTER_DOMAIN_VERIFICATION_STATUS_NEVER;
-import static android.content.pm.PackageManager.INTENT_FILTER_DOMAIN_VERIFICATION_STATUS_UNDEFINED;
-
 class PackageManagerShellCommand extends ShellCommand {
     /** Path for streaming APK content */
     private static final String STDIN_PATH = "-";
@@ -2213,7 +2210,7 @@ class PackageManagerShellCommand extends ShellCommand {
         final PrintWriter pw = getOutPrintWriter();
         final ParcelFileDescriptor fd;
         if (STDIN_PATH.equals(inPath)) {
-            fd = null;
+            fd = new ParcelFileDescriptor(getInFileDescriptor());
         } else if (inPath != null) {
             fd = openFileForSystem(inPath, "r");
             if (fd == null) {
@@ -2225,53 +2222,27 @@ class PackageManagerShellCommand extends ShellCommand {
                 return -1;
             }
         } else {
-            fd = null;
+            fd = new ParcelFileDescriptor(getInFileDescriptor());
         }
         if (sizeBytes <= 0) {
             getErrPrintWriter().println("Error: must specify a APK size");
             return 1;
         }
 
-        final SessionInfo info = mInterface.getPackageInstaller().getSessionInfo(sessionId);
-
         PackageInstaller.Session session = null;
-        InputStream in = null;
-        OutputStream out = null;
         try {
             session = new PackageInstaller.Session(
                     mInterface.getPackageInstaller().openSession(sessionId));
-
-            if (fd != null) {
-                in = new ParcelFileDescriptor.AutoCloseInputStream(fd);
-            } else {
-                in = new SizedInputStream(getRawInputStream(), sizeBytes);
-            }
-            out = session.openWrite(splitName, 0, sizeBytes);
-
-            int total = 0;
-            byte[] buffer = new byte[1024 * 1024];
-            int c;
-            while ((c = in.read(buffer)) != -1) {
-                total += c;
-                out.write(buffer, 0, c);
-
-                if (info.sizeBytes > 0) {
-                    final float fraction = ((float) c / (float) info.sizeBytes);
-                    session.addProgress(fraction);
-                }
-            }
-            session.fsync(out);
+            session.write(splitName, 0, sizeBytes, fd);
 
             if (logSuccess) {
-                pw.println("Success: streamed " + total + " bytes");
+                pw.println("Success: streamed " + sizeBytes + " bytes");
             }
             return 0;
         } catch (IOException e) {
             getErrPrintWriter().println("Error: failed to write; " + e.getMessage());
             return 1;
         } finally {
-            IoUtils.closeQuietly(out);
-            IoUtils.closeQuietly(in);
             IoUtils.closeQuietly(session);
         }
     }