remove: Rooted mns state for simplification

This commit removes the "Rooted" mount namespace state, as it wouldn't fit in the categories made by root implementations. The three major root implementations divide apps into three categories: SU, Umounted, (with) Mounts. The SU one consists of apps that are allowed to elevate privileges, those don't have their mounts modified, and keep root mounts. As for Umounted, it consists of apps that will have root-related mounts umounted before being executed. And finally (with) mounts which are apps that although are not allowed to elevate privileges, still have the root-related mounts kept.
Knowing that we can merge both SU and (with) Mounts into the same category: No modification to mounts/Keep root-related mounts. With that, "Mounts" mns state is also unmodified, compared to old "Modules" mns state.

Those changes also reflect on the issue where new apps couldn't ask Magisk for root as they would have SU mount umounted, which now, as ReZygisk leave them unmodified, they can.

loader/src/include/daemon.h

@@ -50,8 +50,7 @@ struct rezygisk_info {
 
 enum mount_namespace_state {
   Clean,
-  Rooted,
-  Module
+  Mounted
 };
 
 #define TMP_PATH "/data/adb/rezygisk"

loader/src/injector/hook.cpp

@@ -158,8 +158,7 @@ bool update_mnt_ns(enum mount_namespace_state mns_state, bool dry_run) {
 
     const char *mns_state_str = NULL;
     if (mns_state == Clean) mns_state_str = "clean";
-    else if (mns_state == Module) mns_state_str = "module";
-    else if (mns_state == Rooted) mns_state_str = "rooted";
+    else if (mns_state == Mounted) mns_state_str = "mounted";
     else mns_state_str = "unknown";
 
     LOGD("set mount namespace to [%s] fd=[%d]: %s", ns_path, updated_ns, mns_state_str);
@@ -183,17 +182,23 @@ DCL_HOOK_FUNC(int, unshare, int flags) {
         // This is reproducible on the official AVD running API 26 and 27.
         // Simply avoid doing any unmounts for SysUI to avoid potential issues.
         !g_ctx->flags[SERVER_FORK_AND_SPECIALIZE] && !(g_ctx->info_flags & PROCESS_IS_FIRST_STARTED)) {
-        if (g_ctx->info_flags & (PROCESS_IS_MANAGER | PROCESS_GRANTED_ROOT)) {
-            update_mnt_ns(Rooted, false);
-        } else if (!(g_ctx->flags[DO_REVERT_UNMOUNT])) {
-            update_mnt_ns(Module, false);
-        }
 
         /* INFO: There might be cases, specifically in Magisk, where the app is in
                    DenyList but also has root privileges. For those, it is up to the
                    user remove it, and the weird behavior is expected, as the weird
                    user behavior. */
 
+        /* INFO: For cases like Magisk, where you can only give an app SU if it was
+                   either requested before or if it's not in DenyList, we cannot
+                   umount it, or else it will not be (easily) possible to give new
+                   apps SU. Apps that are not marked in APatch/KernelSU to be umounted
+                   are also expected to have AP/KSU mounts there, so we will follow the
+                   same idea by not umounting any mount. */
+
+        if (g_ctx->info_flags & (PROCESS_IS_MANAGER | PROCESS_GRANTED_ROOT) || !(g_ctx->flags[DO_REVERT_UNMOUNT])) {
+            update_mnt_ns(Mounted, false);
+        }
+
         old_unshare(CLONE_NEWNS);
     }
 

zygiskd/src/constants.h

@@ -49,8 +49,7 @@ enum RootImplState {
 
 enum MountNamespaceState {
   Clean,
-  Rooted,
-  Module
+  Mounted
 };
 
 #endif /* CONSTANTS_H */

zygiskd/src/utils.c

@@ -23,8 +23,7 @@
 #include "root_impl/magisk.h"
 
 int clean_namespace_fd = 0;
-int rooted_namespace_fd = 0;
-int module_namespace_fd = 0;
+int mounted_namespace_fd = 0;
 
 bool switch_mount_namespace(pid_t pid) {
   char path[PATH_MAX];
@@ -599,7 +598,7 @@ enum mns_umount_state {
   Error
 };
 
-enum mns_umount_state unmount_root(bool modules_only, struct root_impl impl) {
+enum mns_umount_state unmount_root(struct root_impl impl) {
   /* INFO: We are already in the target pid mount namespace, so actually,
              when we use self here, we meant its pid.
   */
@@ -627,6 +626,8 @@ enum mns_umount_state unmount_root(bool modules_only, struct root_impl impl) {
       if (impl.impl == KernelSU) strcpy(source_name, "KSU");
       else strcpy(source_name, "APatch");
 
+      LOGI("[%s] Unmounting root", source_name);
+
       const char **targets_to_unmount = NULL;
       size_t num_targets = 0;
 
@@ -635,10 +636,6 @@ enum mns_umount_state unmount_root(bool modules_only, struct root_impl impl) {
 
         bool should_unmount = false;
 
-        if (modules_only) {
-          if (strncmp(mount.target, "/debug_ramdisk", strlen("/debug_ramdisk")) == 0)
-            should_unmount = true;
-        } else {
         /* INFO: KernelSU has its own /system mounts, so we only skip the mount
                     if they are from a module, not KSU itself.
         */
@@ -648,7 +645,6 @@ enum mns_umount_state unmount_root(bool modules_only, struct root_impl impl) {
         if (strcmp(mount.source, source_name) == 0) should_unmount = true;
         if (strncmp(mount.root, "/adb/modules", strlen("/adb/modules")) == 0) should_unmount = true;
         if (strncmp(mount.target, "/data/adb/modules", strlen("/data/adb/modules")) == 0) should_unmount = true;
-        }
 
         if (!should_unmount) continue;
 
@@ -680,7 +676,7 @@ enum mns_umount_state unmount_root(bool modules_only, struct root_impl impl) {
       break;
     }
     case Magisk: {
-      LOGI("[Magisk] Unmounting root %s modules\n", modules_only ? "only" : "with");
+      LOGI("[Magisk] Unmounting root");
 
       const char **targets_to_unmount = NULL;
       size_t num_targets = 0;
@@ -689,19 +685,17 @@ enum mns_umount_state unmount_root(bool modules_only, struct root_impl impl) {
         struct mountinfo mount = mounts.mounts[i];
 
         bool should_unmount = false;
-        if (modules_only) {
-          if (strcmp(mount.source, "magisk") == 0) should_unmount = true;
-          if (strncmp(mount.target, "/debug_ramdisk", strlen("/debug_ramdisk")) == 0) should_unmount = true;
-          if (strncmp(mount.target, "/system/bin", strlen("/system/bin")) == 0) should_unmount = true;
-        } else {
-          if (strncmp(mount.target, "/system/", strlen("/system/")) == 0) continue;
+        /* INFO: Magisk has its own /system mounts, so we only skip the mount
+                    if they are from a module, not Magisk itself.
+        */
+        if (strncmp(mount.target, "/system/", strlen("/system/")) == 0 &&
+            strncmp(mount.root, "/adb/modules", strlen("/adb/modules")) == 0) continue;
 
         if (strcmp(mount.source, "magisk") == 0) should_unmount = true;
         if (strncmp(mount.target, "/debug_ramdisk", strlen("/debug_ramdisk")) == 0) should_unmount = true;
         if (strncmp(mount.target, "/data/adb/modules", strlen("/data/adb/modules")) == 0) should_unmount = true;
         if (strncmp(mount.root, "/adb/modules", strlen("/adb/modules")) == 0) should_unmount = true;
         if (strncmp(mount.target, "/system/bin", strlen("/system/bin")) == 0) should_unmount = true;
-        }
 
         if (!should_unmount) continue;
 
@@ -743,8 +737,7 @@ enum mns_umount_state unmount_root(bool modules_only, struct root_impl impl) {
 
 int save_mns_fd(int pid, enum MountNamespaceState mns_state, struct root_impl impl) {
   if (mns_state == Clean && clean_namespace_fd != 0) return clean_namespace_fd;
-  if (mns_state == Rooted && rooted_namespace_fd != 0) return rooted_namespace_fd;
-  if (mns_state == Module && module_namespace_fd != 0) return module_namespace_fd;
+  if (mns_state == Mounted && mounted_namespace_fd != 0) return mounted_namespace_fd;
 
   int sockets[2];
   if (socketpair(AF_UNIX, SOCK_STREAM, 0, sockets) == -1) {
@@ -757,17 +750,45 @@ int save_mns_fd(int pid, enum MountNamespaceState mns_state, struct root_impl im
   int writer = sockets[1];
 
   pid_t fork_pid = fork();
+  if (fork_pid < 0) {
+    LOGE("fork: %s\n", strerror(errno));
+
+    if (close(reader) == -1) {
+      LOGE("Failed to close reader: %s\n", strerror(errno));
+    }
+
+    if (close(writer) == -1) {
+      LOGE("Failed to close writer: %s\n", strerror(errno));
+    }
+
+    return -1;
+  }
+
   if (fork_pid == 0) {
-    switch_mount_namespace(pid);
+    close(reader);
+
+    if (switch_mount_namespace(pid) == false) {
+      LOGE("Failed to switch mount namespace\n");
+
+      write_uint8_t(writer, (uint8_t)Error);
+
+      close(writer);
+
+      _exit(1);
+    }
 
     enum mns_umount_state umount_state = Complete;
 
-    if (mns_state != Rooted) {
+    if (mns_state == Clean) {
       unshare(CLONE_NEWNS);
-      umount_state = unmount_root(mns_state == Module, impl);
+      umount_state = unmount_root(impl);
       if (umount_state == Error) {
+        LOGE("Failed to umount root");
+
         write_uint8_t(writer, (uint8_t)umount_state);
 
+        close(writer);
+
         _exit(1);
       }
     }
@@ -779,14 +800,20 @@ int save_mns_fd(int pid, enum MountNamespaceState mns_state, struct root_impl im
       read_uint32_t(reader, &mypid);
     }
 
+    close(writer);
+
     _exit(0);
-  } else if (fork_pid > 0) {
+  }
+
   enum mns_umount_state umount_state = (enum mns_umount_state)0;
   read_uint8_t(reader, (uint8_t *)&umount_state);
 
   if (umount_state == Error) {
     LOGE("Failed to unmount root\n");
 
+    close(reader);
+    close(writer);
+
     return -1;
   }
 
@@ -797,6 +824,9 @@ int save_mns_fd(int pid, enum MountNamespaceState mns_state, struct root_impl im
   if (ns_fd == -1) {
     LOGE("open: %s\n", strerror(errno));
 
+    close(reader);
+    close(writer);
+
     return -1;
   }
 
@@ -820,15 +850,8 @@ int save_mns_fd(int pid, enum MountNamespaceState mns_state, struct root_impl im
     return -1;
   }
 
-    if (mns_state == Rooted) return (rooted_namespace_fd = ns_fd);
-    else if (mns_state == Clean && umount_state == Complete) return (clean_namespace_fd = ns_fd);
-    else if (mns_state == Module && umount_state == Complete) return (module_namespace_fd = ns_fd);
-    else return ns_fd;
-  } else {
-    LOGE("fork: %s\n", strerror(errno));
+  if (mns_state == Clean) clean_namespace_fd = ns_fd;
+  else if (mns_state == Mounted) mounted_namespace_fd = ns_fd;
 
-    return -1;
-  }
-
-  return -1;
+  return ns_fd;
 }

zygiskd/src/zygiskd.c

@@ -643,8 +643,7 @@ void zygiskd_start(char *restrict argv[]) {
         ASSURE_SIZE_WRITE_BREAK("UpdateMountNamespace", "our_pid", ret, sizeof(our_pid));
 
         if ((enum MountNamespaceState)mns_state == Clean) {
-          save_mns_fd(pid, Rooted, impl);
-          save_mns_fd(pid, Module, impl);
+          save_mns_fd(pid, Mounted, impl);
         }
 
         int clean_namespace_fd = save_mns_fd(pid, (enum MountNamespaceState)mns_state, impl);