Remove some USAP commit

Revert "Fix error formatting issues"

This reverts commit 3244d59.

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

This reverts commit 44cc0a2.

Revert "Don't fork USAPs with open argument buffer"

This reverts commit 97d6916.

Author: Genkzsz11

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

@@ -91,9 +91,7 @@ protected void handlePreloadApp(ApplicationInfo appInfo) {
                     } 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

@@ -500,36 +500,6 @@ static void allowAppFilesAcrossFork(ApplicationInfo appInfo) {
         }
     }
 
-    /**
-     * 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/java/com/android/internal/os/ZygoteConnection.java

@@ -150,11 +150,8 @@ Runnable processCommand(ZygoteServer zygoteServer, boolean multipleOK) {
                     return null;
                 }
 
-                if (parsedArgs.mUsapPoolStatusSpecified
-                        || parsedArgs.mApiBlacklistExemptions != null
-                        || parsedArgs.mHiddenApiAccessLogSampleRate != -1
-                        || parsedArgs.mHiddenApiAccessStatslogSampleRate != -1) {
-                    // Handle these once we've released argBuffer, to avoid opening a second one.
+                if (parsedArgs.mUsapPoolStatusSpecified) {
+                    // Handle this once we've released the argBuffer, to avoid opening a second one.
                     break;
                 }
 
@@ -187,7 +184,17 @@ Runnable processCommand(ZygoteServer zygoteServer, boolean multipleOK) {
                     return null;
                 }
 
+                if (parsedArgs.mApiBlacklistExemptions != null) {
+                    return handleApiBlacklistExemptions(zygoteServer,
+                            parsedArgs.mApiBlacklistExemptions);
+                }
 
+                if (parsedArgs.mHiddenApiAccessLogSampleRate != -1
+                        || parsedArgs.mHiddenApiAccessStatslogSampleRate != -1) {
+                    return handleHiddenApiAccessLogSampleRate(zygoteServer,
+                            parsedArgs.mHiddenApiAccessLogSampleRate,
+                            parsedArgs.mHiddenApiAccessStatslogSampleRate);
+                }
 
                 if (parsedArgs.mPermittedCapabilities != 0
                         || parsedArgs.mEffectiveCapabilities != 0) {
@@ -309,20 +316,10 @@ Runnable processCommand(ZygoteServer zygoteServer, boolean multipleOK) {
                 }
             }
         }
-        // Handle anything that may need a ZygoteCommandBuffer after we've released ours.
         if (parsedArgs.mUsapPoolStatusSpecified) {
+            // Now that we've released argBuffer:
             return handleUsapPoolStatusChange(zygoteServer, parsedArgs.mUsapPoolEnabled);
         }
-        if (parsedArgs.mApiBlacklistExemptions != null) {
-            return handleApiBlacklistExemptions(zygoteServer,
-                    parsedArgs.mApiBlacklistExemptions);
-        }
-        if (parsedArgs.mHiddenApiAccessLogSampleRate != -1
-                || parsedArgs.mHiddenApiAccessStatslogSampleRate != -1) {
-            return handleHiddenApiAccessLogSampleRate(zygoteServer,
-                    parsedArgs.mHiddenApiAccessLogSampleRate,
-                    parsedArgs.mHiddenApiAccessStatslogSampleRate);
-        }
         throw new AssertionError("Shouldn't get here");
     }
 

core/jni/com_android_internal_os_Zygote.cpp

@@ -37,11 +37,9 @@
 #include <sys/types.h>
 #include <dirent.h>
 
-#include <algorithm>
 #include <array>
 #include <atomic>
 #include <functional>
-#include <iterator>
 #include <list>
 #include <optional>
 #include <sstream>
@@ -1944,9 +1942,6 @@ 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,
@@ -1972,12 +1967,9 @@ 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,
-  // 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 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 (gOpenFdTable == nullptr) {
     gOpenFdTable = FileDescriptorTable::Create(fds_to_ignore, fail_fn);
   } else {
@@ -2074,12 +2066,7 @@ static jint com_android_internal_os_Zygote_nativeForkAndSpecialize(
         fds_to_ignore.push_back(gSystemServerSocketFd);
     }
 
-    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);
+    pid_t pid = zygote::ForkCommon(env, false, fds_to_close, fds_to_ignore, true);
 
     if (pid == 0) {
       SpecializeCommon(env, uid, gid, gids, runtime_flags, rlimits,
@@ -2220,10 +2207,6 @@ 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);
 }
@@ -2481,35 +2464,6 @@ static jboolean com_android_internal_os_Zygote_nativeSupportsTaggedPointers(JNIE
 #endif
 }
 
-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/"
@@ -2552,14 +2506,8 @@ static const JNINativeMethod gMethods[] = {
          (void*)com_android_internal_os_Zygote_nativeBoostUsapPriority},
         {"nativeParseSigChld", "([BI[I)I",
          (void*)com_android_internal_os_Zygote_nativeParseSigChld},
-
         {"nativeSupportsTaggedPointers", "()Z",
          (void*)com_android_internal_os_Zygote_nativeSupportsTaggedPointers},
-
-        {"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) {
@@ -2572,4 +2520,4 @@ int register_com_android_internal_os_Zygote(JNIEnv* env) {
 
   return RegisterMethodsOrDie(env, "com/android/internal/os/Zygote", gMethods, NELEM(gMethods));
 }
-}  // namespace android
+}  // namespace android

core/jni/com_android_internal_os_ZygoteCommandBuffer.cpp

@@ -402,7 +402,7 @@ jboolean com_android_internal_os_ZygoteCommandBuffer_nativeForkRepeatedly(
   socklen_t cred_size = sizeof credentials;
   if (getsockopt(n_buffer->getFd(), SOL_SOCKET, SO_PEERCRED, &credentials, &cred_size) == -1
       || cred_size != sizeof credentials) {
-    fail_fn_1(CREATE_ERROR("ForkMany failed to get initial credentials, %s", strerror(errno)));
+    fail_fn_1("ForkMany failed to get initial credentials, %s", strerror(errno));
   }
 
   bool first_time = true;
@@ -453,7 +453,7 @@ jboolean com_android_internal_os_ZygoteCommandBuffer_nativeForkRepeatedly(
       close(session_socket);
       int new_fd = accept(zygote_socket_fd, nullptr, nullptr);
       if (new_fd == -1) {
-        fail_fn_z(CREATE_ERROR("Accept(%d) failed: %s", zygote_socket_fd, strerror(errno)));
+        fail_fn_z("Accept(%d) failed: %s", zygote_socket_fd, strerror(errno));
       }
       if (new_fd != session_socket) {
           // Move new_fd back to the old value, so that we don't have to change Java-level data

core/jni/fd_utils.cpp

@@ -59,6 +59,7 @@ static const char* kPathWhitelist[] = {
 
 static const char kFdPath[] = "/proc/self/fd";
 
+// static
 FileDescriptorWhitelist* FileDescriptorWhitelist::Get() {
   if (instance_ == nullptr) {
     instance_ = new FileDescriptorWhitelist();
@@ -171,8 +172,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 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;
@@ -187,10 +188,8 @@ 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);
 
@@ -208,6 +207,7 @@ 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
@@ -469,36 +469,49 @@ void FileDescriptorInfo::DetachSocket(fail_fn_t fail_fn) const {
   }
 }
 
-// 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);
-
+// static
 FileDescriptorTable* FileDescriptorTable::Create(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);
+  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::unordered_map<int, FileDescriptorInfo*> open_fd_map;
-  for (auto fd : *open_fds) {
+  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;
+    }
 
     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);
 }
 
-static std::unique_ptr<std::set<int>> GetOpenFdsIgnoring(const std::vector<int>& fds_to_ignore,
-                                                         fail_fn_t fail_fn) {
+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.
   DIR* proc_fd_dir = opendir(kFdPath);
   if (proc_fd_dir == nullptr) {
     fail_fn(android::base::StringPrintf("Unable to open directory %s: %s",
                                         kFdPath,
                                         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) {
@@ -511,26 +524,14 @@ static std::unique_ptr<std::set<int>> GetOpenFdsIgnoring(const std::vector<int>&
       continue;
     }
 
-    result->insert(fd);
+    open_fds.insert(fd);
   }
 
   if (closedir(proc_fd_dir) == -1) {
     fail_fn(android::base::StringPrintf("Unable to close directory: %s", strerror(errno)));
   }
-  return result;
-}
 
-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);
+  RestatInternal(open_fds, fail_fn);
 }
 
 // Reopens all file descriptors that are contained in the table.
@@ -551,12 +552,6 @@ 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.
@@ -627,7 +622,8 @@ void FileDescriptorTable::RestatInternal(std::set<int>& open_fds, fail_fn_t fail
   }
 }
 
-static int ParseFd(dirent* dir_entry, int dir_fd) {
+// static
+int FileDescriptorTable::ParseFd(dirent* dir_entry, int dir_fd) {
   char* end;
   const int fd = strtol(dir_entry->d_name, &end, 10);
   if ((*end) != '\0') {