diff options
| author | Egor Pasko <pasko@google.com> | 2021-06-08 19:01:38 +0200 |
|---|---|---|
| committer | Egor Pasko <pasko@google.com> | 2021-06-23 12:59:32 +0000 |
| commit | ff6ac69e69423107a626a00c3e01e9bf5eb2814c (patch) | |
| tree | 309d276b4493f665c80826f6fffcb25a9fa8ec4d /core/java | |
| parent | 1a10b25d486e3a7e4632608fa9855ff4d4e29c77 (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.java | 2 | ||||
| -rw-r--r-- | core/java/com/android/internal/os/Zygote.java | 30 |
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 |
