From 7b25e74418a3b1f607961623d33bbcafb46cc0d5 Mon Sep 17 00:00:00 2001 From: topjohnwu Date: Fri, 17 Sep 2021 02:07:32 -0700 Subject: [PATCH] Simplify get manager app info logic --- native/jni/core/bootstages.cpp | 7 ++-- native/jni/core/db.cpp | 58 ++++++++++++++++------------------ native/jni/include/db.hpp | 4 +-- native/jni/su/connect.cpp | 6 ++-- native/jni/su/su.hpp | 2 +- native/jni/su/su_daemon.cpp | 11 +++---- native/jni/utils/misc.hpp | 3 ++ 7 files changed, 46 insertions(+), 45 deletions(-) diff --git a/native/jni/core/bootstages.cpp b/native/jni/core/bootstages.cpp index 22f142326..2a2b80968 100644 --- a/native/jni/core/bootstages.cpp +++ b/native/jni/core/bootstages.cpp @@ -106,9 +106,10 @@ static bool magisk_env() { LOGI("* Initializing Magisk environment\n"); string pkg; - check_manager(&pkg); + get_manager(&pkg); - sprintf(buf, "%s/0/%s/install", APP_DATA_DIR, pkg.data()); + sprintf(buf, "%s/0/%s/install", APP_DATA_DIR, + pkg.empty() ? "xxx" /* Ensure non-exist path */ : pkg.data()); // Alternative binaries paths const char *alt_bin[] = { "/cache/data_adb/magisk", "/data/magisk", buf }; @@ -354,7 +355,7 @@ void boot_complete(int client) { if (access(SECURE_DIR, F_OK) != 0) xmkdir(SECURE_DIR, 0700); - if (!check_manager()) { + if (!get_manager()) { if (access(MANAGERAPK, F_OK) == 0) { // Only try to install APK when no manager is installed // Magisk Manager should be upgraded by itself, not through recovery installs diff --git a/native/jni/core/db.cpp b/native/jni/core/db.cpp index 4667fd629..3e7bbda98 100644 --- a/native/jni/core/db.cpp +++ b/native/jni/core/db.cpp @@ -341,41 +341,39 @@ int get_uid_policy(su_access &su, int uid) { return 0; } -bool check_manager(string *pkg) { +bool get_manager(int user_id, std::string *pkg, struct stat *st) { db_strings str; get_db_strings(str, SU_MANAGER); - bool ret = validate_manager(str[SU_MANAGER], 0, nullptr); - if (pkg) { - if (ret) - pkg->swap(str[SU_MANAGER]); - else - *pkg = "xxx"; /* Make sure the return pkg can never exist */ - } - return ret; -} - -bool validate_manager(string &pkg, int userid, struct stat *st) { - struct stat tmp_st; - if (st == nullptr) - st = &tmp_st; - - // Prefer DE storage char app_path[128]; - sprintf(app_path, "%s/%d/%s", APP_DATA_DIR, userid, pkg.data()); - if (pkg.empty() || stat(app_path, st)) { - // Check the official package name - sprintf(app_path, "%s/%d/" JAVA_PACKAGE_NAME, APP_DATA_DIR, userid); - if (stat(app_path, st)) { - LOGE("su: cannot find manager\n"); - memset(st, 0, sizeof(*st)); - pkg.clear(); - return false; - } else { - // Switch to official package if exists - pkg = JAVA_PACKAGE_NAME; + + if (!str[SU_MANAGER].empty()) { + // App is repackaged + sprintf(app_path, "%s/%d/%s", APP_DATA_DIR, user_id, str[SU_MANAGER].data()); + if (stat(app_path, st) == 0) { + if (pkg) + pkg->swap(str[SU_MANAGER]); + return true; } } - return true; + + // Check the official package name + sprintf(app_path, "%s/%d/" JAVA_PACKAGE_NAME, APP_DATA_DIR, user_id); + if (stat(app_path, st) == 0) { + if (pkg) + *pkg = JAVA_PACKAGE_NAME; + return true; + } else { + LOGE("su: cannot find manager\n"); + memset(st, 0, sizeof(*st)); + if (pkg) + pkg->clear(); + return false; + } +} + +bool get_manager(string *pkg) { + struct stat st; + return get_manager(0, pkg, &st); } void exec_sql(int client) { diff --git a/native/jni/include/db.hpp b/native/jni/include/db.hpp index 59cc9d6e6..ca7f4ee3b 100644 --- a/native/jni/include/db.hpp +++ b/native/jni/include/db.hpp @@ -126,8 +126,8 @@ using db_row_cb = std::function; int get_db_settings(db_settings &cfg, int key = -1); int get_db_strings(db_strings &str, int key = -1); int get_uid_policy(su_access &su, int uid); -bool check_manager(std::string *pkg = nullptr); -bool validate_manager(std::string &pkg, int userid, struct stat *st); +bool get_manager(int user_id, std::string *pkg, struct stat *st); +bool get_manager(std::string *pkg = nullptr); void exec_sql(int client); char *db_exec(const char *sql); char *db_exec(const char *sql, const db_row_cb &fn); diff --git a/native/jni/su/connect.cpp b/native/jni/su/connect.cpp index 542b196a0..78bec8c6c 100644 --- a/native/jni/su/connect.cpp +++ b/native/jni/su/connect.cpp @@ -106,7 +106,7 @@ static void exec_cmd(const char *action, vector &data, // First try content provider call method if (mode >= CONTENT_PROVIDER) { - sprintf(target, "content://%s.provider", info->str[SU_MANAGER].data()); + sprintf(target, "content://%s.provider", info->mgr_pkg.data()); vector args{ CALL_PROVIDER }; for (auto &e : data) { e.add_bind(args); @@ -137,7 +137,7 @@ static void exec_cmd(const char *action, vector &data, if (mode >= PKG_ACTIVITY) { // Then try start activity without component name - strcpy(target, info->str[SU_MANAGER].data()); + strcpy(target, info->mgr_pkg.data()); exec_command_sync(exec); if (check_no_error(exec.fd)) return; @@ -145,7 +145,7 @@ static void exec_cmd(const char *action, vector &data, // Finally, fallback to start activity with component name args[4] = "-n"; - sprintf(target, "%s/.ui.surequest.SuRequestActivity", info->str[SU_MANAGER].data()); + sprintf(target, "%s/.ui.surequest.SuRequestActivity", info->mgr_pkg.data()); exec.fd = -2; exec.fork = fork_dont_care; exec_command(exec); diff --git a/native/jni/su/su.hpp b/native/jni/su/su.hpp index cd72eca49..070906400 100644 --- a/native/jni/su/su.hpp +++ b/native/jni/su/su.hpp @@ -20,8 +20,8 @@ public: /* These should be guarded with internal lock */ db_settings cfg; - db_strings str; su_access access; + std::string mgr_pkg; struct stat mgr_st; /* This should be guarded with global cache lock */ diff --git a/native/jni/su/su_daemon.cpp b/native/jni/su/su_daemon.cpp index 2a1fce3cc..ed696479d 100644 --- a/native/jni/su/su_daemon.cpp +++ b/native/jni/su/su_daemon.cpp @@ -47,18 +47,17 @@ void su_info::refresh() { static void database_check(const shared_ptr &info) { int uid = info->uid; get_db_settings(info->cfg); - get_db_strings(info->str); // Check multiuser settings switch (info->cfg[SU_MULTIUSER_MODE]) { case MULTIUSER_MODE_OWNER_ONLY: - if (info->uid / 100000) { + if (to_user_id(uid) != 0) { uid = -1; info->access = NO_SU_ACCESS; } break; case MULTIUSER_MODE_OWNER_MANAGED: - uid = info->uid % 100000; + uid = to_app_id(uid); break; case MULTIUSER_MODE_USER: default: @@ -70,7 +69,7 @@ static void database_check(const shared_ptr &info) { // We need to check our manager if (info->access.log || info->access.notify) - validate_manager(info->str[SU_MANAGER], uid / 100000, &info->mgr_st); + get_manager(to_user_id(uid), &info->mgr_pkg, &info->mgr_st); } static shared_ptr get_su_info(unsigned uid) { @@ -93,7 +92,7 @@ static shared_ptr get_su_info(unsigned uid) { database_check(info); // If it's root or the manager, allow it silently - if (info->uid == UID_ROOT || (info->uid % 100000) == (info->mgr_st.st_uid % 100000)) { + if (info->uid == UID_ROOT || to_app_id(info->uid) == to_app_id(info->mgr_st.st_uid)) { info->access = SILENT_SU_ACCESS; return info; } @@ -125,7 +124,7 @@ static shared_ptr get_su_info(unsigned uid) { return info; // If still not determined, check if manager exists - if (info->str[SU_MANAGER].empty()) { + if (info->mgr_pkg.empty()) { info->access = NO_SU_ACCESS; return info; } diff --git a/native/jni/utils/misc.hpp b/native/jni/utils/misc.hpp index 23acda4b2..5946cb940 100644 --- a/native/jni/utils/misc.hpp +++ b/native/jni/utils/misc.hpp @@ -12,6 +12,9 @@ clazz(const clazz &) = delete; \ clazz(clazz &&) = delete; +#define to_app_id(uid) (uid % 100000) +#define to_user_id(uid) (uid / 100000) + class mutex_guard { DISALLOW_COPY_AND_MOVE(mutex_guard) public: