Allow modifying denylist without enforcement

app/src/main/java/com/topjohnwu/magisk/ui/settings/SettingsItems.kt

@@ -277,7 +277,6 @@ object DenyList : BaseSettingsItem.Toggle() {
                 if (result.isSuccess) Config.denyList = it
                 else field = !it
             }
-            DenyListConfig.isEnabled = it
         }
 
     override fun refresh() {

native/jni/core/daemon.cpp

@@ -46,6 +46,9 @@ void register_poll(const pollfd *pfd, poll_callback callback) {
 }
 
 void unregister_poll(int fd, bool auto_close) {
+    if (fd < 0)
+        return;
+
     if (gettid() == getpid()) {
         // On main thread, directly modify
         poll_map->erase(fd);

native/jni/zygisk/deny/cli.cpp

@@ -34,17 +34,6 @@ void denylist_handler(int client, const sock_cred *cred) {
     int req = read_int(client);
     int res = DAEMON_ERROR;
 
-    switch (req) {
-    case ADD_LIST:
-    case RM_LIST:
-    case LS_LIST:
-        if (!denylist_enabled) {
-            write_int(client, DENY_NOT_ENFORCED);
-            close(client);
-            return;
-        }
-    }
-
     switch (req) {
     case ENFORCE_DENY:
         res = enable_deny();

native/jni/zygisk/deny/deny.hpp

@@ -23,7 +23,7 @@ bool is_deny_target(int uid, std::string_view process);
 void revert_unmount();
 
 extern std::atomic<bool> denylist_enabled;
-extern int cached_manager_app_id;
+extern std::atomic<int> cached_manager_app_id;
 
 enum : int {
     ENFORCE_DENY,

native/jni/zygisk/deny/utils.cpp

@@ -23,9 +23,9 @@ static pthread_mutex_t data_lock = PTHREAD_MUTEX_INITIALIZER;
 
 atomic<bool> denylist_enabled = false;
 
+#define do_kill (zygisk_enabled && denylist_enabled)
+
 static void rebuild_map() {
-    if (!zygisk_enabled)
-        return;
     app_id_proc_map->clear();
     string data_path(APP_DATA_DIR);
     size_t len = data_path.length();
@@ -148,9 +148,9 @@ static bool validate(const char *pkg, const char *proc) {
 static void add_hide_set(const char *pkg, const char *proc) {
     LOGI("denylist add: [%s/%s]\n", pkg, proc);
     deny_set->emplace(pkg, proc);
-    if (!zygisk_enabled)
+    if (!do_kill)
         return;
-    if (strcmp(pkg, ISOLATED_MAGIC) == 0) {
+    if (str_eql(pkg, ISOLATED_MAGIC)) {
         // Kill all matching isolated processes
         kill_process<&str_starts>(proc, true);
     } else {
@@ -158,6 +158,63 @@ static void add_hide_set(const char *pkg, const char *proc) {
     }
 }
 
+static void clear_data() {
+    delete deny_set;
+    delete app_id_proc_map;
+    deny_set = nullptr;
+    app_id_proc_map = nullptr;
+    unregister_poll(inotify_fd, true);
+    inotify_fd = -1;
+}
+
+static void inotify_handler(pollfd *pfd) {
+    union {
+        inotify_event event;
+        char buf[512];
+    } u{};
+    read(pfd->fd, u.buf, sizeof(u.buf));
+    if (u.event.name == "packages.xml"sv) {
+        cached_manager_app_id = -1;
+        exec_task([] {
+            mutex_guard lock(data_lock);
+            rebuild_map();
+        });
+    }
+}
+
+static bool ensure_data() {
+    if (app_id_proc_map)
+        return true;
+
+    LOGI("denylist: initializing internal data structures\n");
+
+    default_new(deny_set);
+    char *err = db_exec("SELECT * FROM denylist", [](db_row &row) -> bool {
+        add_hide_set(row["package_name"].data(), row["process"].data());
+        return true;
+    });
+    db_err_cmd(err, goto error);
+
+    default_new(app_id_proc_map);
+    rebuild_map();
+
+    inotify_fd = xinotify_init1(IN_CLOEXEC);
+    if (inotify_fd < 0) {
+        goto error;
+    } else {
+        // Monitor packages.xml
+        inotify_add_watch(inotify_fd, "/data/system", IN_CLOSE_WRITE);
+        pollfd inotify_pfd = { inotify_fd, POLLIN, 0 };
+        register_poll(&inotify_pfd, inotify_handler);
+    }
+
+    return true;
+
+error:
+    clear_data();
+    return false;
+}
+
 static int add_list(const char *pkg, const char *proc) {
     if (proc[0] == '\0')
         proc = pkg;
@@ -167,6 +224,9 @@ static int add_list(const char *pkg, const char *proc) {
 
     {
         mutex_guard lock(data_lock);
+        if (!ensure_data())
+            return DAEMON_ERROR;
+
         for (const auto &hide : *deny_set)
             if (hide.first == pkg && hide.second == proc)
                 return DENYLIST_ITEM_EXIST;
@@ -191,8 +251,11 @@ int add_list(int client) {
 
 static int rm_list(const char *pkg, const char *proc) {
     {
-        bool remove = false;
         mutex_guard lock(data_lock);
+        if (!ensure_data())
+            return DAEMON_ERROR;
+
+        bool remove = false;
         for (auto it = deny_set->begin(); it != deny_set->end();) {
             if (it->first == pkg && (proc[0] == '\0' || it->second == proc)) {
                 remove = true;
@@ -224,36 +287,15 @@ int rm_list(int client) {
     return rm_list(pkg.data(), proc.data());
 }
 
-static bool str_ends_safe(string_view s, string_view ss) {
-    // Never kill webview zygote
-    if (s == "webview_zygote")
-        return false;
-    return str_ends(s, ss);
-}
-
-static bool init_list() {
-    LOGD("denylist: initialize\n");
-
-    char *err = db_exec("SELECT * FROM denylist", [](db_row &row) -> bool {
-        add_hide_set(row["package_name"].data(), row["process"].data());
-        return true;
-    });
-    db_err_cmd(err, return false);
-
-    // If Android Q+, also kill blastula pool and all app zygotes
-    if (SDK_INT >= 29 && zygisk_enabled) {
-        kill_process("usap32", true);
-        kill_process("usap64", true);
-        kill_process<&str_ends_safe>("_zygote", true);
-    }
-
-    return true;
-}
-
 void ls_list(int client) {
-    write_int(client, DAEMON_SUCCESS);
     {
         mutex_guard lock(data_lock);
+        if (!ensure_data()) {
+            write_int(client, DAEMON_ERROR);
+            return;
+        }
+
+        write_int(client, DAEMON_SUCCESS);
         for (const auto &hide : *deny_set) {
             write_int(client, hide.first.size() + hide.second.size() + 1);
             xwrite(client, hide.first.data(), hide.first.size());
@@ -265,6 +307,13 @@ void ls_list(int client) {
     close(client);
 }
 
+static bool str_ends_safe(string_view s, string_view ss) {
+    // Never kill webview zygote
+    if (s == "webview_zygote")
+        return false;
+    return str_ends(s, ss);
+}
+
 static void update_deny_config() {
     char sql[64];
     sprintf(sql, "REPLACE INTO settings (key,value) VALUES('%s',%d)",
@@ -273,21 +322,6 @@ static void update_deny_config() {
     db_err(err);
 }
 
-static void inotify_handler(pollfd *pfd) {
-    union {
-        inotify_event event;
-        char buf[512];
-    } u{};
-    read(pfd->fd, u.buf, sizeof(u.buf));
-    if (u.event.name == "packages.xml"sv) {
-        cached_manager_app_id = -1;
-        exec_task([] {
-            mutex_guard lock(data_lock);
-            rebuild_map();
-        });
-    }
-}
-
 int enable_deny() {
     if (denylist_enabled) {
         return DAEMON_SUCCESS;
@@ -304,26 +338,18 @@ int enable_deny() {
 
         LOGI("* Enable DenyList\n");
 
-        default_new(deny_set);
+        denylist_enabled = true;
-        if (!init_list()) {
+
-            delete deny_set;
+        if (!ensure_data()) {
-            deny_set = nullptr;
+            denylist_enabled = false;
             return DAEMON_ERROR;
         }
 
-        denylist_enabled = true;
+        // On Android Q+, also kill blastula pool and all app zygotes
-
+        if (SDK_INT >= 29 && zygisk_enabled) {
-        if (zygisk_enabled) {
+            kill_process("usap32", true);
-            default_new(app_id_proc_map);
+            kill_process("usap64", true);
-            rebuild_map();
+            kill_process<&str_ends_safe>("_zygote", true);
-
-            inotify_fd = xinotify_init1(IN_CLOEXEC);
-            if (inotify_fd >= 0) {
-                // Monitor packages.xml
-                inotify_add_watch(inotify_fd, "/data/system", IN_CLOSE_WRITE);
-                pollfd inotify_pfd = { inotify_fd, POLLIN, 0 };
-                register_poll(&inotify_pfd, inotify_handler);
-            }
         }
     }
 
@@ -337,12 +363,7 @@ int disable_deny() {
         LOGI("* Disable DenyList\n");
 
         mutex_guard lock(data_lock);
-        delete app_id_proc_map;
+        clear_data();
-        delete deny_set;
-        app_id_proc_map = nullptr;
-        deny_set = nullptr;
-        unregister_poll(inotify_fd, true);
-        inotify_fd = -1;
     }
     update_deny_config();
     return DAEMON_SUCCESS;
@@ -359,6 +380,8 @@ void initialize_denylist() {
 
 bool is_deny_target(int uid, string_view process) {
     mutex_guard lock(data_lock);
+    if (!ensure_data())
+        return false;
 
     int app_id = to_app_id(uid);
     if (app_id >= 90000) {

native/jni/zygisk/entry.cpp

@@ -312,8 +312,7 @@ static void magiskd_passthrough(int client) {
     send_fd(client, is_64_bit ? app_process_64 : app_process_32);
 }
 
-int cached_manager_app_id = -1;
+atomic<int> cached_manager_app_id = -1;
-static time_t last_modified = 0;
 
 static void get_process_info(int client, const sock_cred *cred) {
     AppInfo info{};
@@ -323,25 +322,12 @@ static void get_process_info(int client, const sock_cred *cred) {
     // This function is called on every single zygote process specialization,
     // so performance is critical. get_manager_app_id() is expensive as it goes
     // through a SQLite query and potentially multiple filesystem stats, so we
-    // really want to cache the app ID value. Check the last modify timestamp of
+    // really want to cache the app ID value. inotify will invalidate the app ID
-    // packages.xml and only re-fetch the manager app ID if something changed since
+    // cache for us.
-    // we last checked. Granularity in seconds is good enough.
-    // If denylist is enabled, inotify will invalidate the app ID cache for us.
-    // In this case, we can skip the timestamp check all together.
 
     if (uid != 1000) {
         int manager_app_id = cached_manager_app_id;
 
-        // Denylist not enabled, check packages.xml timestamp
-        if (!denylist_enabled && manager_app_id > 0) {
-            struct stat st{};
-            stat("/data/system/packages.xml", &st);
-            if (st.st_atim.tv_sec > last_modified) {
-                manager_app_id = -1;
-                last_modified = st.st_atim.tv_sec;
-            }
-        }
-
         if (manager_app_id < 0) {
             manager_app_id = get_manager_app_id();
             cached_manager_app_id = manager_app_id;