diff --git a/app/src/main/java/com/topjohnwu/magisk/ui/settings/SettingsItems.kt b/app/src/main/java/com/topjohnwu/magisk/ui/settings/SettingsItems.kt index 8dac8bad7..bdc0697aa 100644 --- a/app/src/main/java/com/topjohnwu/magisk/ui/settings/SettingsItems.kt +++ b/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() { diff --git a/native/jni/core/daemon.cpp b/native/jni/core/daemon.cpp index 2d841d017..617646f48 100644 --- a/native/jni/core/daemon.cpp +++ b/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); diff --git a/native/jni/zygisk/deny/cli.cpp b/native/jni/zygisk/deny/cli.cpp index 6188ed08d..0f8602c6d 100644 --- a/native/jni/zygisk/deny/cli.cpp +++ b/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(); diff --git a/native/jni/zygisk/deny/deny.hpp b/native/jni/zygisk/deny/deny.hpp index b6bb2d0d7..67896ffc5 100644 --- a/native/jni/zygisk/deny/deny.hpp +++ b/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 denylist_enabled; -extern int cached_manager_app_id; +extern std::atomic cached_manager_app_id; enum : int { ENFORCE_DENY, diff --git a/native/jni/zygisk/deny/utils.cpp b/native/jni/zygisk/deny/utils.cpp index 60c7a6223..7f252c299 100644 --- a/native/jni/zygisk/deny/utils.cpp +++ b/native/jni/zygisk/deny/utils.cpp @@ -23,9 +23,9 @@ static pthread_mutex_t data_lock = PTHREAD_MUTEX_INITIALIZER; atomic 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); - if (!init_list()) { - delete deny_set; - deny_set = nullptr; + denylist_enabled = true; + + if (!ensure_data()) { + denylist_enabled = false; return DAEMON_ERROR; } - denylist_enabled = true; - - if (zygisk_enabled) { - default_new(app_id_proc_map); - rebuild_map(); - - 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); - } + // On 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); } } @@ -337,12 +363,7 @@ int disable_deny() { LOGI("* Disable DenyList\n"); mutex_guard lock(data_lock); - delete app_id_proc_map; - delete deny_set; - app_id_proc_map = nullptr; - deny_set = nullptr; - unregister_poll(inotify_fd, true); - inotify_fd = -1; + clear_data(); } 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) { diff --git a/native/jni/zygisk/entry.cpp b/native/jni/zygisk/entry.cpp index 29d97857a..912d22f83 100644 --- a/native/jni/zygisk/entry.cpp +++ b/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; -static time_t last_modified = 0; +atomic cached_manager_app_id = -1; 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 - // packages.xml and only re-fetch the manager app ID if something changed since - // 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. + // really want to cache the app ID value. inotify will invalidate the app ID + // cache for us. 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;