Merge "Allow app zygote preload to retain files across fork"
diff --git a/core/java/com/android/internal/os/AppZygoteInit.java b/core/java/com/android/internal/os/AppZygoteInit.java
index 0e83e41..f925afc 100644
--- a/core/java/com/android/internal/os/AppZygoteInit.java
+++ b/core/java/com/android/internal/os/AppZygoteInit.java
@@ -91,7 +91,9 @@
} 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 644e032..fb9942a 100644
--- a/core/java/com/android/internal/os/Zygote.java
+++ b/core/java/com/android/internal/os/Zygote.java
@@ -495,6 +495,36 @@
}
/**
+ * 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
diff --git a/core/jni/com_android_internal_os_Zygote.cpp b/core/jni/com_android_internal_os_Zygote.cpp
index 6ac43bd3..fa9eead 100644
--- a/core/jni/com_android_internal_os_Zygote.cpp
+++ b/core/jni/com_android_internal_os_Zygote.cpp
@@ -27,9 +27,11 @@
#include <sys/types.h>
#include <dirent.h>
+#include <algorithm>
#include <array>
#include <atomic>
#include <functional>
+#include <iterator>
#include <list>
#include <optional>
#include <sstream>
@@ -1964,6 +1966,9 @@
__builtin_unreachable();
}
+static std::set<int>* gPreloadFds = nullptr;
+static bool gPreloadFdsExtracted = false;
+
// Utility routine to fork a process from the zygote.
pid_t zygote::ForkCommon(JNIEnv* env, bool is_system_server,
const std::vector<int>& fds_to_close,
@@ -1989,9 +1994,12 @@
__android_log_close();
AStatsSocket_close();
- // If this is the first fork for this zygote, create the open FD table. If
- // it isn't, we just need to check whether the list of open files has changed
- // (and it shouldn't in the normal case).
+ // If this is the first fork for this zygote, create the open FD table,
+ // verifying that files are of supported type and allowlisted. Otherwise (not
+ // the first fork), check that the open files have not changed. Newly open
+ // files are not expected, and will be disallowed in the future. Currently
+ // they are allowed if they pass the same checks as in the
+ // FileDescriptorTable::Create() above.
if (gOpenFdTable == nullptr) {
gOpenFdTable = FileDescriptorTable::Create(fds_to_ignore, fail_fn);
} else {
@@ -2087,7 +2095,12 @@
fds_to_ignore.push_back(gSystemServerSocketFd);
}
- pid_t pid = zygote::ForkCommon(env, false, fds_to_close, fds_to_ignore, true);
+ if (gPreloadFds && gPreloadFdsExtracted) {
+ fds_to_ignore.insert(fds_to_ignore.end(), gPreloadFds->begin(), gPreloadFds->end());
+ }
+
+ pid_t pid = zygote::ForkCommon(env, /* is_system_server= */ false, fds_to_close, fds_to_ignore,
+ true);
if (pid == 0) {
SpecializeCommon(env, uid, gid, gids, runtime_flags, rlimits, capabilities, capabilities,
@@ -2224,6 +2237,10 @@
}
fds_to_ignore.push_back(gSystemServerSocketFd);
}
+ if (gPreloadFds && gPreloadFdsExtracted) {
+ fds_to_ignore.insert(fds_to_ignore.end(), gPreloadFds->begin(), gPreloadFds->end());
+ }
+
return zygote::ForkCommon(env, /* is_system_server= */ false, fds_to_close,
fds_to_ignore, is_priority_fork == JNI_TRUE, purge);
}
@@ -2527,6 +2544,35 @@
#endif // defined(__aarch64__)
}
+static void com_android_internal_os_Zygote_nativeMarkOpenedFilesBeforePreload(JNIEnv* env, jclass) {
+ // Ignore invocations when too early or too late.
+ if (gPreloadFds) {
+ return;
+ }
+
+ // App Zygote Preload starts soon. Save FDs remaining open. After the
+ // preload finishes newly open files will be determined.
+ auto fail_fn = std::bind(zygote::ZygoteFailure, env, "zygote", nullptr, _1);
+ gPreloadFds = GetOpenFds(fail_fn).release();
+}
+
+static void com_android_internal_os_Zygote_nativeAllowFilesOpenedByPreload(JNIEnv* env, jclass) {
+ // Ignore invocations when too early or too late.
+ if (!gPreloadFds || gPreloadFdsExtracted) {
+ return;
+ }
+
+ // Find the newly open FDs, if any.
+ auto fail_fn = std::bind(zygote::ZygoteFailure, env, "zygote", nullptr, _1);
+ std::unique_ptr<std::set<int>> current_fds = GetOpenFds(fail_fn);
+ auto difference = std::make_unique<std::set<int>>();
+ std::set_difference(current_fds->begin(), current_fds->end(), gPreloadFds->begin(),
+ gPreloadFds->end(), std::inserter(*difference, difference->end()));
+ delete gPreloadFds;
+ gPreloadFds = difference.release();
+ gPreloadFdsExtracted = true;
+}
+
static const JNINativeMethod gMethods[] = {
{"nativeForkAndSpecialize",
"(II[II[[IILjava/lang/String;Ljava/lang/String;[I[IZLjava/lang/String;Ljava/lang/"
@@ -2575,6 +2621,10 @@
(void*)com_android_internal_os_Zygote_nativeSupportsTaggedPointers},
{"nativeCurrentTaggingLevel", "()I",
(void*)com_android_internal_os_Zygote_nativeCurrentTaggingLevel},
+ {"nativeMarkOpenedFilesBeforePreload", "()V",
+ (void*)com_android_internal_os_Zygote_nativeMarkOpenedFilesBeforePreload},
+ {"nativeAllowFilesOpenedByPreload", "()V",
+ (void*)com_android_internal_os_Zygote_nativeAllowFilesOpenedByPreload},
};
int register_com_android_internal_os_Zygote(JNIEnv* env) {
diff --git a/core/jni/fd_utils.cpp b/core/jni/fd_utils.cpp
index eac1d99..f4772ff 100644
--- a/core/jni/fd_utils.cpp
+++ b/core/jni/fd_utils.cpp
@@ -50,7 +50,6 @@
static const char kFdPath[] = "/proc/self/fd";
-// static
FileDescriptorAllowlist* FileDescriptorAllowlist::Get() {
if (instance_ == nullptr) {
instance_ = new FileDescriptorAllowlist();
@@ -167,8 +166,8 @@
// Create a FileDescriptorInfo for a given file descriptor.
static FileDescriptorInfo* CreateFromFd(int fd, fail_fn_t fail_fn);
- // Checks whether the file descriptor associated with this object
- // refers to the same description.
+ // Checks whether the file descriptor associated with this object refers to
+ // the same description.
bool RefersToSameFile() const;
void ReopenOrDetach(fail_fn_t fail_fn) const;
@@ -183,8 +182,10 @@
const bool is_sock;
private:
+ // Constructs for sockets.
explicit FileDescriptorInfo(int fd);
+ // Constructs for non-socket file descriptors.
FileDescriptorInfo(struct stat stat, const std::string& file_path, int fd, int open_flags,
int fd_flags, int fs_flags, off_t offset);
@@ -202,7 +203,6 @@
DISALLOW_COPY_AND_ASSIGN(FileDescriptorInfo);
};
-// static
FileDescriptorInfo* FileDescriptorInfo::CreateFromFd(int fd, fail_fn_t fail_fn) {
struct stat f_stat;
// This should never happen; the zygote should always have the right set
@@ -463,42 +463,24 @@
}
}
-// static
+// TODO: Move the definitions here and eliminate the forward declarations. They
+// temporarily help making code reviews easier.
+static int ParseFd(dirent* dir_entry, int dir_fd);
+static std::unique_ptr<std::set<int>> GetOpenFdsIgnoring(const std::vector<int>& fds_to_ignore,
+ fail_fn_t fail_fn);
+
FileDescriptorTable* FileDescriptorTable::Create(const std::vector<int>& fds_to_ignore,
fail_fn_t fail_fn) {
- DIR* proc_fd_dir = opendir(kFdPath);
- if (proc_fd_dir == nullptr) {
- fail_fn(std::string("Unable to open directory ").append(kFdPath));
- }
-
- int dir_fd = dirfd(proc_fd_dir);
- dirent* dir_entry;
-
+ std::unique_ptr<std::set<int>> open_fds = GetOpenFdsIgnoring(fds_to_ignore, fail_fn);
std::unordered_map<int, FileDescriptorInfo*> open_fd_map;
- while ((dir_entry = readdir(proc_fd_dir)) != nullptr) {
- const int fd = ParseFd(dir_entry, dir_fd);
- if (fd == -1) {
- continue;
- }
-
- if (std::find(fds_to_ignore.begin(), fds_to_ignore.end(), fd) != fds_to_ignore.end()) {
- continue;
- }
-
+ for (auto fd : *open_fds) {
open_fd_map[fd] = FileDescriptorInfo::CreateFromFd(fd, fail_fn);
}
-
- if (closedir(proc_fd_dir) == -1) {
- fail_fn("Unable to close directory");
- }
-
return new FileDescriptorTable(open_fd_map);
}
-void FileDescriptorTable::Restat(const std::vector<int>& fds_to_ignore, fail_fn_t fail_fn) {
- std::set<int> open_fds;
-
- // First get the list of open descriptors.
+static std::unique_ptr<std::set<int>> GetOpenFdsIgnoring(const std::vector<int>& fds_to_ignore,
+ fail_fn_t fail_fn) {
DIR* proc_fd_dir = opendir(kFdPath);
if (proc_fd_dir == nullptr) {
fail_fn(android::base::StringPrintf("Unable to open directory %s: %s",
@@ -506,6 +488,7 @@
strerror(errno)));
}
+ auto result = std::make_unique<std::set<int>>();
int dir_fd = dirfd(proc_fd_dir);
dirent* dir_entry;
while ((dir_entry = readdir(proc_fd_dir)) != nullptr) {
@@ -518,14 +501,26 @@
continue;
}
- open_fds.insert(fd);
+ result->insert(fd);
}
if (closedir(proc_fd_dir) == -1) {
fail_fn(android::base::StringPrintf("Unable to close directory: %s", strerror(errno)));
}
+ return result;
+}
- RestatInternal(open_fds, fail_fn);
+std::unique_ptr<std::set<int>> GetOpenFds(fail_fn_t fail_fn) {
+ const std::vector<int> nothing_to_ignore;
+ return GetOpenFdsIgnoring(nothing_to_ignore, fail_fn);
+}
+
+void FileDescriptorTable::Restat(const std::vector<int>& fds_to_ignore, fail_fn_t fail_fn) {
+ std::unique_ptr<std::set<int>> open_fds = GetOpenFdsIgnoring(fds_to_ignore, fail_fn);
+
+ // Check that the files did not change, and leave only newly opened FDs in
+ // |open_fds|.
+ RestatInternal(*open_fds, fail_fn);
}
// Reopens all file descriptors that are contained in the table.
@@ -546,6 +541,12 @@
: open_fd_map_(map) {
}
+FileDescriptorTable::~FileDescriptorTable() {
+ for (auto& it : open_fd_map_) {
+ delete it.second;
+ }
+}
+
void FileDescriptorTable::RestatInternal(std::set<int>& open_fds, fail_fn_t fail_fn) {
// ART creates a file through memfd for optimization purposes. We make sure
// there is at most one being created.
@@ -616,8 +617,7 @@
}
}
-// static
-int FileDescriptorTable::ParseFd(dirent* dir_entry, int dir_fd) {
+static int ParseFd(dirent* dir_entry, int dir_fd) {
char* end;
const int fd = strtol(dir_entry->d_name, &end, 10);
if ((*end) != '\0') {
diff --git a/core/jni/fd_utils.h b/core/jni/fd_utils.h
index 14c318e..a28ebf1 100644
--- a/core/jni/fd_utils.h
+++ b/core/jni/fd_utils.h
@@ -69,6 +69,9 @@
DISALLOW_COPY_AND_ASSIGN(FileDescriptorAllowlist);
};
+// Returns the set of file descriptors currently open by the process.
+std::unique_ptr<std::set<int>> GetOpenFds(fail_fn_t fail_fn);
+
// A FileDescriptorTable is a collection of FileDescriptorInfo objects
// keyed by their FDs.
class FileDescriptorTable {
@@ -79,6 +82,14 @@
static FileDescriptorTable* Create(const std::vector<int>& fds_to_ignore,
fail_fn_t fail_fn);
+ ~FileDescriptorTable();
+
+ // Checks that the currently open FDs did not change their metadata from
+ // stat(2), readlink(2) etc. Ignores FDs from |fds_to_ignore|.
+ //
+ // Temporary: allows newly open FDs if they pass the same checks as in
+ // Create(). This will be further restricted. See TODOs in the
+ // implementation.
void Restat(const std::vector<int>& fds_to_ignore, fail_fn_t fail_fn);
// Reopens all file descriptors that are contained in the table. Returns true
@@ -91,8 +102,6 @@
void RestatInternal(std::set<int>& open_fds, fail_fn_t fail_fn);
- static int ParseFd(dirent* e, int dir_fd);
-
// Invariant: All values in this unordered_map are non-NULL.
std::unordered_map<int, FileDescriptorInfo*> open_fd_map_;