Fix racing issue when caching mount profiles (#12)

There are two calls of `update_mount_namespace` with parameter `zygiskd::MountNamespace::Clean`.
In some rare case, this causes a racing issue, for example, between the daemon requests of [com.android.systemui] and [webview_zygote].

To fully solve the problem of thread racing on `save_mount_namespace`, we should cache mount profiles in the main thread of daemon.

loader/src/common/daemon.cpp

@@ -15,8 +15,9 @@ std::string GetTmpPath() { return TMP_PATH; }
 
 int Connect(uint8_t retry) {
     int fd = socket(PF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
-    struct sockaddr_un addr {
-        .sun_family = AF_UNIX, .sun_path = {0},
+    struct sockaddr_un addr{
+        .sun_family = AF_UNIX,
+        .sun_path = {0},
     };
     auto socket_path = TMP_PATH + kCPSocketName;
     strcpy(addr.sun_path, socket_path.c_str());
@@ -56,14 +57,22 @@ uint32_t GetProcessFlags(uid_t uid) {
     return socket_utils::read_u32(fd);
 }
 
-std::string UpdateMountNamespace(pid_t pid, MountNamespace type) {
+void CacheMountNamespace(pid_t pid) {
+    UniqueFd fd = Connect(1);
+    if (fd == -1) {
+        PLOGE("CacheMountNamespace");
+    }
+    socket_utils::write_u8(fd, (uint8_t) SocketAction::CacheMountNamespace);
+    socket_utils::write_u32(fd, (uint32_t) pid);
+}
+
+std::string UpdateMountNamespace(MountNamespace type) {
     UniqueFd fd = Connect(1);
     if (fd == -1) {
         PLOGE("UpdateMountNamespace");
         return "";
     }
     socket_utils::write_u8(fd, (uint8_t) SocketAction::UpdateMountNamespace);
-    socket_utils::write_u32(fd, (uint32_t) pid);
     socket_utils::write_u8(fd, (uint8_t) type);
     uint32_t target_pid = socket_utils::read_u32(fd);
     int target_fd = (int) socket_utils::read_u32(fd);

loader/src/include/daemon.hpp

@@ -58,6 +58,7 @@ struct Module {
 enum class SocketAction {
     PingHeartbeat,
     GetProcessFlags,
+    CacheMountNamespace,
     UpdateMountNamespace,
     ReadModules,
     RequestCompanionSocket,
@@ -78,7 +79,9 @@ std::vector<Module> ReadModules();
 
 uint32_t GetProcessFlags(uid_t uid);
 
-std::string UpdateMountNamespace(pid_t pid, MountNamespace type);
+void CacheMountNamespace(pid_t pid);
+
+std::string UpdateMountNamespace(MountNamespace type);
 
 int ConnectCompanion(size_t index);
 

loader/src/injector/hook.cpp

@@ -106,9 +106,9 @@ DCL_HOOK_FUNC(static int, unshare, int flags) {
         // Skip system server and the first app process since we don't need to hide traces for them
         !(g_ctx->flags & SERVER_FORK_AND_SPECIALIZE) && !(g_ctx->info_flags & IS_FIRST_PROCESS)) {
         if (g_ctx->info_flags & (PROCESS_IS_MANAGER | PROCESS_GRANTED_ROOT)) {
-            ZygiskContext::update_mount_namespace(getpid(), zygiskd::MountNamespace::Root);
+            ZygiskContext::update_mount_namespace(zygiskd::MountNamespace::Root);
         } else if (!(g_ctx->flags & DO_REVERT_UNMOUNT)) {
-            ZygiskContext::update_mount_namespace(getpid(), zygiskd::MountNamespace::Module);
+            ZygiskContext::update_mount_namespace(zygiskd::MountNamespace::Module);
         }
         old_unshare(CLONE_NEWNS);
     }

loader/src/injector/module.cpp

@@ -328,7 +328,7 @@ void ZygiskContext::app_specialize_pre() {
 
     info_flags = zygiskd::GetProcessFlags(g_ctx->args.app->uid);
     if (info_flags & IS_FIRST_PROCESS) {
-        update_mount_namespace(getpid(), zygiskd::MountNamespace::Clean, true);
+        zygiskd::CacheMountNamespace(getpid());
     }
     if ((info_flags & UNMOUNT_MASK) == UNMOUNT_MASK) {
         LOGI("[%s] is on the denylist\n", process);
@@ -397,7 +397,7 @@ void ZygiskContext::nativeForkAndSpecialize_pre() {
     flags |= APP_FORK_AND_SPECIALIZE;
 
     // unmount the root implementation for Zygote
-    update_mount_namespace(getpid(), zygiskd::MountNamespace::Clean, false);
+    update_mount_namespace(zygiskd::MountNamespace::Clean);
 
     fork_pre();
     if (is_child()) {
@@ -416,19 +416,12 @@ void ZygiskContext::nativeForkAndSpecialize_post() {
 
 // -----------------------------------------------------------------
 
-bool ZygiskContext::update_mount_namespace(pid_t pid, zygiskd::MountNamespace namespace_type,
-                                           bool dry_run) {
-    if (pid < 0) {
-        LOGD("update mount namespace with an invalid pid %d", pid);
-        return false;
-    }
-
-    std::string ns_path = zygiskd::UpdateMountNamespace(pid, namespace_type);
+bool ZygiskContext::update_mount_namespace(zygiskd::MountNamespace namespace_type) {
+    std::string ns_path = zygiskd::UpdateMountNamespace(namespace_type);
     if (!ns_path.starts_with("/proc/")) {
         PLOGE("update mount namespace [%s]", ns_path.data());
         return false;
     }
-    if (dry_run) return true;
 
     auto updated_ns = open(ns_path.data(), O_RDONLY);
     if (updated_ns >= 0) {

loader/src/injector/module.hpp

@@ -310,8 +310,7 @@ struct ZygiskContext {
 
     bool plt_hook_commit();
 
-    static bool update_mount_namespace(pid_t pid, zygiskd::MountNamespace namespace_type,
-                                       bool dry_run = false);
+    static bool update_mount_namespace(zygiskd::MountNamespace namespace_type);
 };
 
 #undef DCL_PRE_POST

loader/src/injector/zygisk.hpp

@@ -5,8 +5,6 @@
 
 void hook_entry(void *start_addr, size_t block_size);
 
-bool update_mnt_ns(pid_t pid, bool clean, bool dry_run = false);
-
 void hookJniNativeMethods(JNIEnv *env, const char *clz, JNINativeMethod *methods, int numMethods);
 
 void clean_trace(const char *path, size_t load, size_t unload, bool spoof_maps);

zygiskd/src/constants.rs

@@ -27,6 +27,7 @@ pub const SYSTEM_SERVER_STARTED: i32 = 10;
 pub enum DaemonSocketAction {
     PingHeartbeat,
     GetProcessFlags,
+    CacheMountNamespace,
     UpdateMountNamespace,
     ReadModules,
     RequestCompanionSocket,

zygiskd/src/utils.rs

@@ -141,7 +141,16 @@ pub fn save_mount_namespace(pid: i32, namespace_type: MountNamespace) -> Result<
         MountNamespace::Root => ROOT_MNT_NS_FD.initiated(),
         MountNamespace::Module => MODULE_MNT_NS_FD.initiated(),
     };
+
     if !is_initialized {
+        if pid == -1 {
+            bail!(
+                "mount profiles not fully cached: {}, {}, {}",
+                CLEAN_MNT_NS_FD.initiated(),
+                ROOT_MNT_NS_FD.initiated(),
+                MODULE_MNT_NS_FD.initiated()
+            );
+        }
         // Use a pipe to keep the forked child process open
         // till the namespace is read.
 

zygiskd/src/zygiskd.rs

@@ -93,6 +93,12 @@ pub fn main() -> Result<()> {
         let action = DaemonSocketAction::try_from(action)?;
         trace!("New daemon action {:?}", action);
         match action {
+            DaemonSocketAction::CacheMountNamespace => {
+                let pid = stream.read_u32()? as i32;
+                save_mount_namespace(pid, MountNamespace::Clean)?;
+                save_mount_namespace(pid, MountNamespace::Root)?;
+                save_mount_namespace(pid, MountNamespace::Module)?;
+            }
             DaemonSocketAction::PingHeartbeat => {
                 let value = constants::ZYGOTE_INJECTED;
                 utils::unix_datagram_sendto(&CONTROLLER_SOCKET, &value.to_le_bytes())?;
@@ -280,17 +286,10 @@ fn handle_daemon_action(
             stream.write_u32(flags.bits())?;
         }
         DaemonSocketAction::UpdateMountNamespace => {
-            let pid = stream.read_u32()?;
             let namespace_type = stream.read_u8()?;
             let namespace_type = MountNamespace::try_from(namespace_type)?;
             stream.write_u32(unsafe { libc::getpid() } as u32)?;
-            if namespace_type == MountNamespace::Clean {
-                // we will only clean once, which is exactly the moment
-                // to cache mount namespaces
-                save_mount_namespace(pid as i32, MountNamespace::Root)?;
-                save_mount_namespace(pid as i32, MountNamespace::Module)?;
-            }
-            let fd = save_mount_namespace(pid as i32, namespace_type)?;
+            let fd = save_mount_namespace(-1, namespace_type)?;
             stream.write_u32(fd as u32)?;
         }
         DaemonSocketAction::ReadModules => {