From 296299743969f99a69da8b4d2a4d92eee11efcb6 Mon Sep 17 00:00:00 2001
From: snake-4 <18491360+snake-4@users.noreply.github.com>
Date: Tue, 16 Apr 2024 17:43:57 +0200
Subject: [PATCH] Moved unmount to unshare hook

This might break older Android versions. Not sure.
---
 module/jni/include/utils.hpp |  2 +-
 module/jni/main.cpp          | 53 ++++++++++++++++++------------------
 2 files changed, 27 insertions(+), 28 deletions(-)

diff --git a/module/jni/include/utils.hpp b/module/jni/include/utils.hpp
index 92fba95..dc725eb 100644
--- a/module/jni/include/utils.hpp
+++ b/module/jni/include/utils.hpp
@@ -3,7 +3,7 @@
 #include "zygisk.hpp"
 
 #define DCL_HOOK_FUNC(ret, func, ...) \
-    ret (*old_##func)(__VA_ARGS__);   \
+    ret (*old_##func)(__VA_ARGS__) = nullptr;   \
     ret new_##func(__VA_ARGS__)
 
 int isUserAppUID(int uid);
diff --git a/module/jni/main.cpp b/module/jni/main.cpp
index 2c6a010..c0f31d1 100644
--- a/module/jni/main.cpp
+++ b/module/jni/main.cpp
@@ -13,13 +13,30 @@ using zygisk::ServerSpecializeArgs;
 void doUnmount();
 void doRemount();
 
+/*
+ * [What's the purpose of this hook?]
+ * Calling unshare twice invalidates existing FD links, which fails Zygote sanity checks.
+ * So we prevent further namespaces by hooking unshare.
+ *
+ * [Doesn't Android already call unshare?]
+ * Whether there's going to be an unshare or not changes with each major Android version
+ * so we unconditionally unshare in preAppSpecialize.
+ * > Android 5: Conditionally unshares
+ * > Android 6: Always unshares
+ * > Android 7-11: Conditionally unshares
+ * > Android 12-14: Always unshares
+ */
 DCL_HOOK_FUNC(static int, unshare, int flags)
 {
+    doUnmount();
+    doRemount();
+
     // Do not allow CLONE_NEWNS.
     flags &= ~(CLONE_NEWNS);
     if (!flags)
     {
         // If CLONE_NEWNS was the only flag, skip the call.
+        errno = 0;
         return 0;
     }
     return old_unshare(flags);
@@ -36,7 +53,6 @@ public:
 
     void preAppSpecialize(AppSpecializeArgs *args) override
     {
-        isHooked = false;
         api->setOption(zygisk::Option::DLCLOSE_MODULE_LIBRARY);
 
         uint32_t flags = api->getFlags();
@@ -50,24 +66,9 @@ public:
         LOGD("Processing pid=%d ppid=%d uid=%d", getpid(), getppid(), args->uid);
 
         /*
-         * Calling unshare twice invalidates FD hard links, which fails Zygote sanity checks.
-         * So we hook unshare to prevent further namespace creations.
-         * The logic behind whether there's going to be an unshare or not changes with each major Android version.
-         * For maximum compatibility, we will always unshare but prevent further unshare by this Zygote fork in appSpecialize.
+         * Read the comment above unshare hook.
          */
-        if (!hookPLTByName("libandroid_runtime.so", "unshare", &new_unshare, &old_unshare))
-        {
-            LOGE("plt_hook_wrapper(\"libandroid_runtime.so\", \"unshare\", new_unshare, old_unshare) returned false");
-            return;
-        }
-        isHooked = true;
-
-        /*
-         * preAppSpecialize is before any possible unshare calls.
-         * postAppSpecialize is after seccomp setup.
-         * So we unshare here to create an app mount namespace.
-         */
-        if (old_unshare(CLONE_NEWNS) == -1)
+        if (unshare(CLONE_NEWNS) == -1)
         {
             LOGE("unshare(CLONE_NEWNS) returned -1: %d (%s)", errno, strerror(errno));
             return;
@@ -83,8 +84,11 @@ public:
             return;
         }
 
-        doUnmount();
-        doRemount();
+        if (!hookPLTByName("libandroid_runtime.so", "unshare", new_unshare, &old_unshare))
+        {
+            LOGE("plt_hook_wrapper(\"libandroid_runtime.so\", \"unshare\", new_unshare, old_unshare) returned false");
+            return;
+        }
     }
 
     void preServerSpecialize(ServerSpecializeArgs *args) override
@@ -94,13 +98,9 @@ public:
 
     void postAppSpecialize(const AppSpecializeArgs *args) override
     {
-        if (isHooked)
+        if (old_unshare != nullptr && !hookPLTByName("libandroid_runtime.so", "unshare", old_unshare))
         {
-            if (!hookPLTByName("libandroid_runtime.so", "unshare", old_unshare))
-            {
-                LOGE("plt_hook_wrapper(\"libandroid_runtime.so\", \"unshare\", old_unshare, nullptr) returned false");
-                return;
-            }
+            LOGE("plt_hook_wrapper(\"libandroid_runtime.so\", \"unshare\", old_unshare) returned false");
         }
     }
 
@@ -111,7 +111,6 @@ public:
     }
 
 private:
-    bool isHooked = false;
     Api *api;
     JNIEnv *env;
 };