diff --git a/loader/src/include/daemon.h b/loader/src/include/daemon.h index 2a30761..4bbe89c 100644 --- a/loader/src/include/daemon.h +++ b/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" diff --git a/loader/src/injector/hook.cpp b/loader/src/injector/hook.cpp index ff897f8..add369f 100644 --- a/loader/src/injector/hook.cpp +++ b/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); } diff --git a/zygiskd/src/constants.h b/zygiskd/src/constants.h index 4db4df0..a9d9f44 100644 --- a/zygiskd/src/constants.h +++ b/zygiskd/src/constants.h @@ -49,8 +49,7 @@ enum RootImplState { enum MountNamespaceState { Clean, - Rooted, - Module + Mounted }; #endif /* CONSTANTS_H */ diff --git a/zygiskd/src/utils.c b/zygiskd/src/utils.c index 8e3d244..d4ba865 100644 --- a/zygiskd/src/utils.c +++ b/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,20 +636,15 @@ 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. - */ - if (strncmp(mount.target, "/system/", strlen("/system/")) == 0 && - strncmp(mount.root, "/adb/modules", strlen("/adb/modules")) == 0) continue; + /* INFO: KernelSU has its own /system mounts, so we only skip the mount + if they are from a module, not KSU itself. + */ + if (strncmp(mount.target, "/system/", strlen("/system/")) == 0 && + strncmp(mount.root, "/adb/modules", strlen("/adb/modules")) == 0) continue; - 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 (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 (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,56 +800,58 @@ 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"); + enum mns_umount_state umount_state = (enum mns_umount_state)0; + read_uint8_t(reader, (uint8_t *)&umount_state); - return -1; - } + if (umount_state == Error) { + LOGE("Failed to unmount root\n"); - char ns_path[PATH_MAX]; - snprintf(ns_path, PATH_MAX, "/proc/%d/ns/mnt", fork_pid); - - int ns_fd = open(ns_path, O_RDONLY); - if (ns_fd == -1) { - LOGE("open: %s\n", strerror(errno)); - - return -1; - } - - write_uint32_t(writer, (uint32_t)fork_pid); - - if (close(reader) == -1) { - LOGE("Failed to close reader: %s\n", strerror(errno)); - - return -1; - } - - if (close(writer) == -1) { - LOGE("Failed to close writer: %s\n", strerror(errno)); - - return -1; - } - - if (waitpid(fork_pid, NULL, 0) == -1) { - LOGE("waitpid: %s\n", strerror(errno)); - - 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)); + close(reader); + close(writer); return -1; } - return -1; + char ns_path[PATH_MAX]; + snprintf(ns_path, PATH_MAX, "/proc/%d/ns/mnt", fork_pid); + + int ns_fd = open(ns_path, O_RDONLY); + if (ns_fd == -1) { + LOGE("open: %s\n", strerror(errno)); + + close(reader); + close(writer); + + return -1; + } + + write_uint32_t(writer, (uint32_t)fork_pid); + + if (close(reader) == -1) { + LOGE("Failed to close reader: %s\n", strerror(errno)); + + return -1; + } + + if (close(writer) == -1) { + LOGE("Failed to close writer: %s\n", strerror(errno)); + + return -1; + } + + if (waitpid(fork_pid, NULL, 0) == -1) { + LOGE("waitpid: %s\n", strerror(errno)); + + return -1; + } + + if (mns_state == Clean) clean_namespace_fd = ns_fd; + else if (mns_state == Mounted) mounted_namespace_fd = ns_fd; + + return ns_fd; } diff --git a/zygiskd/src/zygiskd.c b/zygiskd/src/zygiskd.c index 1eac1c9..47dce5d 100644 --- a/zygiskd/src/zygiskd.c +++ b/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);