Merge "Allow app zygote preload to retain files across fork"

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 "

core/java/com/android/internal/os/Zygote.java

@@ -494,6 +494,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

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 @@ void zygote::ZygoteFailure(JNIEnv* env,
   __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 @@ pid_t zygote::ForkCommon(JNIEnv* env, bool is_system_server,
   __android_log_close();
   AStatsSocket_close();
 
-  // If this is the first fork for this zygote, create the open FD table.  If
+  // If this is the first fork for this zygote, create the open FD table,
-  // it isn't, we just need to check whether the list of open files has changed
+  // verifying that files are of supported type and allowlisted.  Otherwise (not
-  // (and it shouldn't in the normal case).
+  // 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 @@ static jint com_android_internal_os_Zygote_nativeForkAndSpecialize(
         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 @@ int zygote::forkApp(JNIEnv* env,
       }
       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 @@ static jint com_android_internal_os_Zygote_nativeCurrentTaggingLevel(JNIEnv* env
 #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 @@ static const JNINativeMethod gMethods[] = {
          (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) {

core/jni/fd_utils.cpp

@@ -50,7 +50,6 @@ static const char* kPathAllowlist[] = {
 
 static const char kFdPath[] = "/proc/self/fd";
 
-// static
 FileDescriptorAllowlist* FileDescriptorAllowlist::Get() {
     if (instance_ == nullptr) {
         instance_ = new FileDescriptorAllowlist();
@@ -167,8 +166,8 @@ class FileDescriptorInfo {
   // 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
+  // Checks whether the file descriptor associated with this object refers to
-  // refers to the same description.
+  // the same description.
   bool RefersToSameFile() const;
 
   void ReopenOrDetach(fail_fn_t fail_fn) const;
@@ -183,8 +182,10 @@ class FileDescriptorInfo {
   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 @@ class FileDescriptorInfo {
   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 @@ void FileDescriptorInfo::DetachSocket(fail_fn_t fail_fn) const {
   }
 }
 
-// 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);
+  std::unique_ptr<std::set<int>> open_fds = GetOpenFdsIgnoring(fds_to_ignore, fail_fn);
-  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::unordered_map<int, FileDescriptorInfo*> open_fd_map;
-  while ((dir_entry = readdir(proc_fd_dir)) != nullptr) {
+  for (auto fd : *open_fds) {
-    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;
-    }
-
     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) {
+static std::unique_ptr<std::set<int>> GetOpenFdsIgnoring(const std::vector<int>& fds_to_ignore,
-  std::set<int> open_fds;
+                                                         fail_fn_t fail_fn) {
-
-  // First get the list of open descriptors.
   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 @@ void FileDescriptorTable::Restat(const std::vector<int>& fds_to_ignore, fail_fn_
                                         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 @@ void FileDescriptorTable::Restat(const std::vector<int>& fds_to_ignore, fail_fn_
       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 @@ FileDescriptorTable::FileDescriptorTable(
     : 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 @@ void FileDescriptorTable::RestatInternal(std::set<int>& open_fds, fail_fn_t fail
   }
 }
 
-// static
+static int ParseFd(dirent* dir_entry, int dir_fd) {
-int FileDescriptorTable::ParseFd(dirent* dir_entry, int dir_fd) {
   char* end;
   const int fd = strtol(dir_entry->d_name, &end, 10);
   if ((*end) != '\0') {

core/jni/fd_utils.h

@@ -69,6 +69,9 @@ private:
     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 @@ class FileDescriptorTable {
   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 @@ class FileDescriptorTable {
 
   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_;