summaryrefslogtreecommitdiff
path: root/core/java
diff options
context:
space:
mode:
authorEgor Pasko <pasko@google.com>2021-06-08 19:01:38 +0200
committerEgor Pasko <pasko@google.com>2021-06-23 12:59:32 +0000
commitff6ac69e69423107a626a00c3e01e9bf5eb2814c (patch)
tree309d276b4493f665c80826f6fffcb25a9fa8ec4d /core/java
parent1a10b25d486e3a7e4632608fa9855ff4d4e29c77 (diff)
Allow app zygote preload to retain files across fork
The bug proposes to 'move' the /proc/self/fd/ readlink/stat/etc checks performed by the FileDescriptorAllowlist from before-fork to an earlier stage. The original aim was to allow the app zygote Preload hook to open ashmem/memfd read-only regions to save more RAM (around 5MiB on aarch64) via sharing more across processes. Potentially other files/sockets can be opened - the app zygote takes responsibility of managing file descriptor access controls across its own processes. App Zygote Preload does not run 3rd party code. Unfortunately a straightforward move of the checks to just-before-preload has disadvantages: * opens more codepaths for potential accidental misuse (the zygote accepts commands between preload and fork, there are valid usecases for extending these commands) * this way FileDescriptorAllowlist would need to support more file descriptor types (sockets and maybe pipes), which is not needed now because these FDs are closed right before forking The solution proposed here is to: 1. Determine the set of file descriptors open before preload 2. Run the preload hook 3. Determine FDs opened by the hook and allow them to remain open across fork 4. Hypothetical new attempts to preload (if ever supported) will not affect the allowed FDs - the preload will be able to toss its own FDs the way it wants, but not open the new-new ones Bug: 184808875 Test: Manual: unreleased Chrome patch: while in app zygote preload, create ashmem region, passes it to 'untrusted_app' (=browser process), and call mmap(2) on it. Change-Id: Ie302eabca83a0e4f409cb131e4308b73e5f6a580 Merged-In: Ie302eabca83a0e4f409cb131e4308b73e5f6a580
Diffstat (limited to 'core/java')
-rw-r--r--core/java/com/android/internal/os/AppZygoteInit.java2
-rw-r--r--core/java/com/android/internal/os/Zygote.java30
2 files changed, 32 insertions, 0 deletions
diff --git a/core/java/com/android/internal/os/AppZygoteInit.java b/core/java/com/android/internal/os/AppZygoteInit.java
index 0e83e41a7423..f925afc2a921 100644
--- a/core/java/com/android/internal/os/AppZygoteInit.java
+++ b/core/java/com/android/internal/os/AppZygoteInit.java
@@ -91,7 +91,9 @@ class AppZygoteInit {
} else {
Constructor<?> ctor = cl.getConstructor();
ZygotePreload preloadObject = (ZygotePreload) ctor.newInstance();
+ Zygote.markOpenedFilesBeforePreload();
preloadObject.doPreload(appInfo);
+ Zygote.allowFilesOpenedByPreload();
}
} catch (ReflectiveOperationException e) {
Log.e(TAG, "AppZygote application preload failed for "
diff --git a/core/java/com/android/internal/os/Zygote.java b/core/java/com/android/internal/os/Zygote.java
index 0c9dded42bda..e4e28a926ed6 100644
--- a/core/java/com/android/internal/os/Zygote.java
+++ b/core/java/com/android/internal/os/Zygote.java
@@ -500,6 +500,36 @@ public final class Zygote {
}
/**
+ * Scans file descriptors in /proc/self/fd/, stores their metadata from readlink(2)/stat(2) when
+ * available. Saves this information in a global on native side, to be used by subsequent call
+ * to allowFilesOpenedByPreload(). Fatally fails if the FDs are of unsupported type and are not
+ * explicitly allowed. Ignores repeated invocations.
+ *
+ * Inspecting the FDs is more permissive than in forkAndSpecialize() because preload is invoked
+ * earlier and hence needs to allow a few open sockets. The checks in forkAndSpecialize()
+ * enforce that these sockets are closed when forking.
+ */
+ static void markOpenedFilesBeforePreload() {
+ nativeMarkOpenedFilesBeforePreload();
+ }
+
+ private static native void nativeMarkOpenedFilesBeforePreload();
+
+ /**
+ * By scanning /proc/self/fd/ determines file descriptor numbers in this process opened since
+ * the first call to markOpenedFilesBeforePreload(). These FDs are treated as 'owned' by the
+ * custom preload of the App Zygote - the app is responsible for not sharing data with its other
+ * processes using these FDs, including by lseek(2). File descriptor types and file names are
+ * not checked. Changes in FDs recorded by markOpenedFilesBeforePreload() are not expected and
+ * kill the current process.
+ */
+ static void allowFilesOpenedByPreload() {
+ nativeAllowFilesOpenedByPreload();
+ }
+
+ private static native void nativeAllowFilesOpenedByPreload();
+
+ /**
* Installs a seccomp filter that limits setresuid()/setresgid() to the passed-in range
* @param uidGidMin The smallest allowed uid/gid
* @param uidGidMax The largest allowed uid/gid