From 4eed6794c7929ad7d047eaf93481a8f5b6e2cbb9 Mon Sep 17 00:00:00 2001 From: topjohnwu Date: Wed, 13 Feb 2019 20:16:26 -0500 Subject: [PATCH] More MagiskHide optimizations - Use a general procfs traversal function with callbacks - Much better functions for killing processes --- native/jni/magiskhide/hide_utils.cpp | 192 +++++++++++++------------ native/jni/magiskhide/magiskhide.cpp | 4 +- native/jni/magiskhide/magiskhide.h | 22 ++- native/jni/magiskhide/proc_monitor.cpp | 136 ++++++------------ 4 files changed, 164 insertions(+), 190 deletions(-) diff --git a/native/jni/magiskhide/hide_utils.cpp b/native/jni/magiskhide/hide_utils.cpp index 96debcf1e..b75b1f3ac 100644 --- a/native/jni/magiskhide/hide_utils.cpp +++ b/native/jni/magiskhide/hide_utils.cpp @@ -20,7 +20,7 @@ using namespace std; // Protect access to both hide_list and hide_uid pthread_mutex_t list_lock; vector hide_list; -set hide_uid; +set hide_uid; // Treat GMS separately as we're only interested in one component int gms_uid = -1; @@ -37,11 +37,6 @@ static const char *prop_value[] = "enforcing", "0", "0", "0", "1", "user", "release-keys", "0", nullptr }; -struct ps_arg { - const char *name; - uid_t uid; -}; - void manage_selinux() { char val; int fd = xopen(SELINUX_ENFORCE, O_RDONLY); @@ -54,7 +49,7 @@ void manage_selinux() { } } -void hide_sensitive_props() { +static void hide_sensitive_props() { LOGI("hide_utils: Hiding sensitive props\n"); // Hide all sensitive props @@ -65,27 +60,32 @@ void hide_sensitive_props() { } } -static bool is_digits(const char *s) { - for (const char *c = s; *c; ++c) { - if (*c < '0' || *c > '9') - return false; +/* + * Bionic's atoi runs through strtol(). + * Use our own implementation for faster conversion. + */ +static inline int parse_int(const char *s) { + int val = 0; + char c; + while ((c = *(s++))) { + if (c > '9' || c < '0') + return -1; + val = val * 10 + c - '0'; } - return true; + return val; } -static void ps(void (*cb)(int, void*), void *arg) { - DIR *dir; - struct dirent *entry; - - if (!(dir = xopendir("/proc"))) - return; - - while ((entry = xreaddir(dir))) { - if (entry->d_type == DT_DIR && is_digits(entry->d_name)) - cb(atoi(entry->d_name), arg); +// Leave /proc fd opened as we're going to read from it repeatedly +static DIR *procfp; +void crawl_procfs(const function &fn) { + struct dirent *dp; + int pid; + rewinddir(procfp); + while ((dp = readdir(procfp))) { + pid = parse_int(dp->d_name); + if (pid > 0 && !fn(pid)) + break; } - - closedir(dir); } static bool proc_name_match(int pid, const char *name) { @@ -94,24 +94,24 @@ static bool proc_name_match(int pid, const char *name) { sprintf(buf, "/proc/%d/comm", pid); if ((f = fopen(buf, "re"))) { fgets(buf, sizeof(buf), f); + fclose(f); if (strcmp(buf, name) == 0) return true; } else { // The PID is already killed return false; } - fclose(f); sprintf(buf, "/proc/%d/cmdline", pid); if ((f = fopen(buf, "re"))) { fgets(buf, sizeof(buf), f); + fclose(f); if (strcmp(basename(buf), name) == 0) return true; } else { // The PID is already killed return false; } - fclose(f); sprintf(buf, "/proc/%d/exe", pid); ssize_t len; @@ -121,52 +121,38 @@ static bool proc_name_match(int pid, const char *name) { return strcmp(basename(buf), name) == 0; } -static void kill_proc_cb(int pid, void *v) { - auto args = static_cast(v); - if (proc_name_match(pid, args->name)) - kill(pid, SIGTERM); - else if (args->uid > 0) { - char buf[64]; - struct stat st; - sprintf(buf, "/proc/%d", pid); - stat(buf, &st); - if (args->uid == st.st_uid) - kill(pid, SIGTERM); - } - -} - static void kill_process(const char *name) { - ps_arg args; - char *slash = nullptr; - if (strcmp(name, SAFETYNET_COMPONENT) == 0) { - // We do NOT want to kill gms, it will cause massive system crashes - args.name = SAFETYNET_PROCESS; - } else { - // Only leave the package name part of component name temporarily - slash = strchr((char *)name, '/'); - if (slash) - *slash = '\0'; - args.name = name; - } - struct stat st; - int fd = xopen("/data/data", O_RDONLY | O_CLOEXEC); - if (fstatat(fd, args.name, &st, 0) == 0) - args.uid = st.st_uid; - else - args.uid = 0; - close(fd); - ps(kill_proc_cb, &args); - // Revert back to component name - if (slash) - *slash = '/'; + // We do NOT want to kill GMS itself + if (strcmp(name, SAFETYNET_PKG) == 0) + name = SAFETYNET_PROCESS; + crawl_procfs([=](int pid) -> bool { + if (proc_name_match(pid, name)) { + if (kill(pid, SIGTERM) == 0) + LOGD("hide_utils: killed PID=[%d] (%s)\n", pid, name); + return false; + } + return true; + }); } -static int add_pkg_uid(const char *proc) { +static void kill_process(int uid) { + // We do NOT want to kill all GMS processes + if (uid == gms_uid) { + kill_process(SAFETYNET_PROCESS); + return; + } + crawl_procfs([=](int pid) -> bool { + if (get_uid(pid) == uid && kill(pid, SIGTERM) == 0) + LOGD("hide_utils: killed PID=[%d]\n", pid); + return true; + }); +} + +static int add_pkg_uid(const char *pkg) { char path[4096]; struct stat st; const char *data = SDK_INT >= 24 ? "/data/user_de/0" : "/data/data"; - sprintf(path, "%s/%s", data, proc); + sprintf(path, "%s/%s", data, pkg); if (xstat(path, &st) == 0) { hide_uid.insert(st.st_uid); return st.st_uid; @@ -181,53 +167,52 @@ void refresh_uid() { } void clean_magisk_props() { - LOGD("hide_utils: Cleaning magisk props\n"); getprop([](const char *name, auto, auto) -> void { if (strstr(name, "magisk")) deleteprop(name); }, nullptr, false); } -int add_list(const char *proc) { +int add_list(const char *pkg) { for (auto &s : hide_list) { - if (s == proc) + if (s == pkg) return HIDE_ITEM_EXIST; } // Add to database char sql[4096]; - snprintf(sql, sizeof(sql), "INSERT INTO hidelist (process) VALUES('%s')", proc); + snprintf(sql, sizeof(sql), "INSERT INTO hidelist (process) VALUES('%s')", pkg); char *err = db_exec(sql); db_err_cmd(err, return DAEMON_ERROR); - LOGI("hide_list add: [%s]\n", proc); + LOGI("hide_list add: [%s]\n", pkg); // Critical region pthread_mutex_lock(&list_lock); - hide_list.emplace_back(proc); - add_pkg_uid(proc); + hide_list.emplace_back(pkg); + int uid = add_pkg_uid(pkg); pthread_mutex_unlock(&list_lock); - kill_process(proc); + kill_process(uid); return DAEMON_SUCCESS; } int add_list(int client) { - char *proc = read_string(client); - int ret = add_list(proc); - free(proc); + char *pkg = read_string(client); + int ret = add_list(pkg); + free(pkg); update_inotify_mask(); return ret; } -static int rm_list(const char *proc) { +static int rm_list(const char *pkg) { // Critical region bool remove = false; pthread_mutex_lock(&list_lock); for (auto it = hide_list.begin(); it != hide_list.end(); ++it) { - if (*it == proc) { + if (*it == pkg) { remove = true; - LOGI("hide_list rm: [%s]\n", proc); + LOGI("hide_list rm: [%s]\n", pkg); hide_list.erase(it); break; } @@ -238,7 +223,7 @@ static int rm_list(const char *proc) { if (remove) { char sql[4096]; - snprintf(sql, sizeof(sql), "DELETE FROM hidelist WHERE process='%s'", proc); + snprintf(sql, sizeof(sql), "DELETE FROM hidelist WHERE process='%s'", pkg); char *err = db_exec(sql); db_err(err); return DAEMON_SUCCESS; @@ -248,14 +233,14 @@ static int rm_list(const char *proc) { } int rm_list(int client) { - char *proc = read_string(client); - int ret = rm_list(proc); - free(proc); + char *pkg = read_string(client); + int ret = rm_list(pkg); + free(pkg); update_inotify_mask(); return ret; } -int init_list(void *, int, char **data, char**) { +static int init_list(void *, int, char **data, char**) { LOGI("hide_list init: [%s]\n", *data); hide_list.emplace_back(*data); kill_process(*data); @@ -265,8 +250,8 @@ int init_list(void *, int, char **data, char**) { return 0; } -void init_list(const char *proc) { - init_list(nullptr, 0, (char **) &proc, nullptr); +static void init_list(const char *pkg) { + init_list(nullptr, 0, (char **) &pkg, nullptr); } #define LEGACY_LIST MODULEROOT "/.core/hidelist" @@ -311,17 +296,36 @@ static void set_hide_config() { db_err(err); } -int launch_magiskhide(int client) { +static inline void launch_err(int client, int code = DAEMON_ERROR) { + if (code == DAEMON_ERROR) + hide_enabled = false; + if (client >= 0) { + write_int(client, code); + close(client); + } + pthread_exit(nullptr); +} + +#define LAUNCH_ERR launch_err(client) + +void launch_magiskhide(int client) { if (SDK_INT < 19) - goto error; + LAUNCH_ERR; if (hide_enabled) - return HIDE_IS_ENABLED; + launch_err(client, HIDE_IS_ENABLED); hide_enabled = true; set_hide_config(); LOGI("* Starting MagiskHide\n"); + if (procfp == nullptr) { + int fd = xopen("/proc", O_RDONLY | O_CLOEXEC); + if (fd < 0) + LAUNCH_ERR; + procfp = fdopendir(fd); + } + hide_sensitive_props(); // Initialize the mutex lock @@ -329,20 +333,20 @@ int launch_magiskhide(int client) { // Initialize the hide list if (!init_list()) - goto error; + LAUNCH_ERR; // Get thread reference proc_monitor_thread = pthread_self(); if (client >= 0) { write_int(client, DAEMON_SUCCESS); close(client); + client = -1; } // Start monitoring proc_monitor(); -error: - hide_enabled = false; - return DAEMON_ERROR; + // proc_monitor should not return + LAUNCH_ERR; } int stop_magiskhide() { diff --git a/native/jni/magiskhide/magiskhide.cpp b/native/jni/magiskhide/magiskhide.cpp index 05a82e207..5dfcd9d00 100644 --- a/native/jni/magiskhide/magiskhide.cpp +++ b/native/jni/magiskhide/magiskhide.cpp @@ -52,8 +52,8 @@ void magiskhide_handler(int client) { switch (req) { case LAUNCH_MAGISKHIDE: - res = launch_magiskhide(client); - break; + launch_magiskhide(client); + return; case STOP_MAGISKHIDE: res = stop_magiskhide(); break; diff --git a/native/jni/magiskhide/magiskhide.h b/native/jni/magiskhide/magiskhide.h index 9e80084aa..93caf3ed3 100644 --- a/native/jni/magiskhide/magiskhide.h +++ b/native/jni/magiskhide/magiskhide.h @@ -2,9 +2,13 @@ #define MAGISK_HIDE_H #include +#include +#include +#include #include #include #include +#include #include "daemon.h" @@ -15,7 +19,7 @@ #define SAFETYNET_PKG "com.google.android.gms" // Daemon entries -int launch_magiskhide(int client); +void launch_magiskhide(int client); int stop_magiskhide(); int add_list(int client); int rm_list(int client); @@ -29,14 +33,26 @@ void proc_monitor(); // Utility functions void manage_selinux(); -void hide_sensitive_props(); void clean_magisk_props(); void refresh_uid(); +void crawl_procfs(const std::function &fn); + +static inline int get_uid(const int pid) { + char path[16]; + struct stat st; + + sprintf(path, "/proc/%d", pid); + if (stat(path, &st) == -1) + return -1; + + // We don't care about multiuser + return st.st_uid % 100000; +} extern bool hide_enabled; extern pthread_mutex_t list_lock; extern std::vector hide_list; -extern std::set hide_uid; +extern std::set hide_uid; extern int gms_uid; enum { diff --git a/native/jni/magiskhide/proc_monitor.cpp b/native/jni/magiskhide/proc_monitor.cpp index a76b2288b..1ddaad493 100644 --- a/native/jni/magiskhide/proc_monitor.cpp +++ b/native/jni/magiskhide/proc_monitor.cpp @@ -11,7 +11,6 @@ #include #include #include -#include #include #include #include @@ -21,7 +20,7 @@ #include #include #include -#include +#include #include #include @@ -62,6 +61,9 @@ static inline void lazy_unmount(const char* mountpoint) { LOGD("hide_daemon: Unmounted (%s)\n", mountpoint); } +/* APK monitoring doesn't seem to require checking namespace + * separation from PPID. Preserve this function just in case */ +#if 0 static inline int parse_ppid(const int pid) { char path[32]; int ppid; @@ -77,17 +79,7 @@ static inline int parse_ppid(const int pid) { return ppid; } - -static inline uid_t get_uid(const int pid) { - char path[16]; - struct stat st; - - sprintf(path, "/proc/%d", pid); - if (stat(path, &st) == -1) - return -1; - - return st.st_uid; -} +#endif static bool is_pid_safetynet_process(const int pid) { char path[32]; @@ -109,7 +101,7 @@ static bool is_pid_safetynet_process(const int pid) { } static void hide_daemon(int pid) { - LOGD("hide_daemon: handling pid=[%d]\n", pid); + LOGD("hide_daemon: handling PID=[%d]\n", pid); char buffer[4096]; vector mounts; @@ -151,80 +143,49 @@ exit: _exit(0); } -/* - * Bionic's atoi runs through strtol() and fault-tolerence checkings. - * Since we don't need it, use our own implementation of atoi() - * for faster conversion. - */ -static inline int fast_atoi(const char *str) { - int val = 0; +// A mapping from pid to namespace inode to avoid time-consuming GC +static map pid_ns_map; - while (*str) - val = val * 10 + (*str++ - '0'); +static bool process_pid(int pid) { + // We're only interested in PIDs > 1000 + if (pid <= 1000) + return true; - return val; -} + struct stat ns; + int uid = get_uid(pid); + if (hide_uid.count(uid)) { + // Make sure we can read mount namespace + if (read_ns(pid, &ns)) + return true; -// Leave /proc fd opened as we're going to read from it repeatedly -static DIR *dfd; -// Use unordered map with pid and namespace inode number to avoid time-consuming GC -static unordered_map pid_ns_map; + // Check if it's a process we haven't already hijacked + auto pos = pid_ns_map.find(pid); + if (pos != pid_ns_map.end() && pos->second == ns.st_ino) + return true; -static void detect_new_processes() { - struct dirent *dp; - struct stat ns, pns; - int pid, ppid; - uid_t uid; + if (uid == gms_uid) { + // Check /proc/uid/cmdline to see if it's SAFETYNET_PROCESS + if (!is_pid_safetynet_process(pid)) + return true; - // Iterate through /proc and get a process that reads the target APK - rewinddir(dfd); - pthread_mutex_lock(&list_lock); - while ((dp = readdir(dfd))) { - if (!isdigit(dp->d_name[0])) - continue; - - // dp->d_name is now the pid - pid = fast_atoi(dp->d_name); - - // We're only interested in PIDs > 1000 - if (pid <= 1000) - continue; - - uid = get_uid(pid) % 100000; // Handle multiuser - bool is_target = hide_uid.count(uid) != 0; - if (is_target) { - // Make sure our target is alive - if ((ppid = parse_ppid(pid)) < 0 || read_ns(ppid, &pns) || read_ns(pid, &ns)) - continue; - - // Check if it's a process we haven't already hijacked - auto pos = pid_ns_map.find(pid); - if (pos == pid_ns_map.end() || pos->second != ns.st_ino) { - pid_ns_map[pid] = ns.st_ino; - if (uid == gms_uid) { - // Check /proc/uid/cmdline to see if it's SAFETYNET_PROCESS - if (!is_pid_safetynet_process(pid)) - continue; - - LOGI("proc_monitor: found %s\n", SAFETYNET_PROCESS); - } - - // Send pause signal ASAP - if (kill(pid, SIGSTOP) == -1) - continue; - - /* - * The setns system call do not support multithread processes - * We have to fork a new process, setns, then do the unmounts - */ - LOGI("proc_monitor: UID=[%ju] PID=[%d] ns=[%llu]\n", - (uintmax_t)uid, pid, ns.st_ino); - if (fork_dont_care() == 0) - hide_daemon(pid); - } + LOGD("proc_monitor: " SAFETYNET_PROCESS "\n"); } + + // Send pause signal ASAP + if (kill(pid, SIGSTOP) == -1) + return true; + + pid_ns_map[pid] = ns.st_ino; + LOGI("proc_monitor: UID=[%d] PID=[%d] ns=[%llu]\n", uid, pid, ns.st_ino); + + /* + * The setns system call do not support multithread processes + * We have to fork a new process, setns, then do the unmounts + */ + if (fork_dont_care() == 0) + hide_daemon(pid); } - pthread_mutex_unlock(&list_lock); + return true; } static void listdir_apk(const char *name) { @@ -310,14 +271,6 @@ void proc_monitor() { term_thread(TERM_THREAD); } - if ((dfd = opendir("/proc")) == NULL) { - LOGE("proc_monitor: Unable to open /proc\n"); - term_thread(TERM_THREAD); - } - - // Detect existing processes for the first time - detect_new_processes(); - // Read inotify events struct inotify_event *event; ssize_t len; @@ -337,8 +290,9 @@ void proc_monitor() { if (event->mask & IN_OPEN) { // Since we're just watching files, // extracting file name is not possible from querying event - // LOGI("proc_monitor: inotify: APK opened\n"); - detect_new_processes(); + pthread_mutex_lock(&list_lock); + crawl_procfs(process_pid); + pthread_mutex_unlock(&list_lock); } else { LOGI("proc_monitor: inotify: /data/app change detected\n"); pthread_mutex_lock(&list_lock);