From 09ef19f7ec0f49d02b51ccd0559d1db7eb8f4e9e Mon Sep 17 00:00:00 2001 From: topjohnwu Date: Thu, 4 Oct 2018 01:49:52 -0400 Subject: [PATCH] Code cleanups --- native/jni/daemon/bootstages.c | 2 +- native/jni/daemon/db.c | 5 ----- native/jni/include/db.h | 15 ++++++++------- native/jni/su/connect.c | 22 ++++++++++----------- native/jni/su/su.c | 30 ++++++++++++++--------------- native/jni/su/su.h | 5 ++++- native/jni/su/su_daemon.c | 35 ++++++++++++++++------------------ 7 files changed, 55 insertions(+), 59 deletions(-) diff --git a/native/jni/daemon/bootstages.c b/native/jni/daemon/bootstages.c index f778db9f6..9937dc54a 100644 --- a/native/jni/daemon/bootstages.c +++ b/native/jni/daemon/bootstages.c @@ -869,7 +869,7 @@ core_only: // Check whether we have a valid manager installed sqlite3 *db = get_magiskdb(); struct db_strings str; - INIT_DB_STRINGS(&str); + memset(&str, 0, sizeof(str)); get_db_strings(db, SU_MANAGER, &str); if (validate_manager(str.s[SU_MANAGER], 0, NULL)) { // There is no manager installed, install the stub diff --git a/native/jni/daemon/db.c b/native/jni/daemon/db.c index 92ca42009..f676f6ec2 100644 --- a/native/jni/daemon/db.c +++ b/native/jni/daemon/db.c @@ -8,11 +8,6 @@ #include "magisk.h" #include "db.h" -void INIT_DB_STRINGS(struct db_strings *str) { - for (int i = 0; i < DB_STRING_NUM; ++i) - str->s[i][0] = '\0'; -} - static int policy_cb(void *v, int col_num, char **data, char **col_name) { struct su_access *su = v; for (int i = 0; i < col_num; i++) { diff --git a/native/jni/include/db.h b/native/jni/include/db.h index 0b56c980e..25a6b3a4c 100644 --- a/native/jni/include/db.h +++ b/native/jni/include/db.h @@ -8,7 +8,8 @@ * DB Settings * ***************/ -#define DB_SETTING_KEYS ((char *[]) { \ +#define DB_SETTING_KEYS \ +((char *[]) { \ "root_access", \ "multiuser_mode", \ "mnt_ns" \ @@ -49,18 +50,20 @@ struct db_settings { int v[DB_SETTINGS_NUM]; }; -#define DEFAULT_DB_SETTINGS (struct db_settings) { .v = {\ +#define DEFAULT_DB_SETTINGS \ +(struct db_settings) { .v = { \ ROOT_ACCESS_APPS_AND_ADB, \ MULTIUSER_MODE_OWNER_ONLY, \ -NAMESPACE_MODE_REQUESTER \ +NAMESPACE_MODE_REQUESTER, \ }} /************** * DB Strings * **************/ -#define DB_STRING_KEYS ((char *[]) { \ -"requester" \ +#define DB_STRING_KEYS \ +((char *[]) { \ +"requester", \ }) #define DB_STRING_NUM (sizeof(DB_STRING_KEYS) / sizeof(char*)) @@ -74,8 +77,6 @@ struct db_strings { char s[DB_STRING_NUM][128]; }; -void INIT_DB_STRINGS(struct db_strings *str); - /************* * SU Access * *************/ diff --git a/native/jni/su/connect.c b/native/jni/su/connect.c index 8197d1045..54cb5d149 100644 --- a/native/jni/su/connect.c +++ b/native/jni/su/connect.c @@ -42,14 +42,14 @@ static void silent_run(char * const args[]) { } static void setup_user(char *user) { - switch (su_ctx->info->dbs.v[SU_MULTIUSER_MODE]) { - case MULTIUSER_MODE_OWNER_ONLY: - case MULTIUSER_MODE_OWNER_MANAGED: - sprintf(user, "%d", 0); - break; - case MULTIUSER_MODE_USER: - sprintf(user, "%d", su_ctx->info->uid / 100000); - break; + switch (DB_SET(su_ctx->info, SU_MULTIUSER_MODE)) { + case MULTIUSER_MODE_OWNER_ONLY: + case MULTIUSER_MODE_OWNER_MANAGED: + sprintf(user, "%d", 0); + break; + case MULTIUSER_MODE_USER: + sprintf(user, "%d", su_ctx->info->uid / 100000); + break; } } @@ -59,7 +59,7 @@ void app_log() { char fromUid[8]; sprintf(fromUid, "%d", - su_ctx->info->dbs.v[SU_MULTIUSER_MODE] == MULTIUSER_MODE_OWNER_MANAGED ? + DB_SET(su_ctx->info, SU_MULTIUSER_MODE) == MULTIUSER_MODE_OWNER_MANAGED ? su_ctx->info->uid % 100000 : su_ctx->info->uid); char toUid[8]; @@ -74,7 +74,7 @@ void app_log() { char *cmd[] = { AM_PATH, "broadcast", "-a", "android.intent.action.BOOT_COMPLETED", - "-p", su_ctx->info->str.s[SU_MANAGER], + "-p", DB_STR(su_ctx->info, SU_MANAGER), "--user", user, "--es", "action", "log", "--ei", "from.uid", fromUid, @@ -93,7 +93,7 @@ void app_connect(const char *socket) { char *cmd[] = { AM_PATH, "broadcast", "-a", "android.intent.action.BOOT_COMPLETED", - "-p", su_ctx->info->str.s[SU_MANAGER], + "-p", DB_STR(su_ctx->info, SU_MANAGER), "--user", user, "--es", "action", "request", "--es", "socket", (char *) socket, diff --git a/native/jni/su/su.c b/native/jni/su/su.c index 8ab0518e7..c797ab5ca 100644 --- a/native/jni/su/su.c +++ b/native/jni/su/su.c @@ -189,7 +189,7 @@ int su_daemon_main(int argc, char **argv) { // Do nothing, placed here for legacy support :) break; case 'M': - su_ctx->info->dbs.v[SU_MNT_NS] = NAMESPACE_MODE_GLOBAL; + DB_SET(su_ctx->info, SU_MNT_NS) = NAMESPACE_MODE_GLOBAL; break; default: /* Bionic getopt_long doesn't terminate its error output by newline */ @@ -214,21 +214,21 @@ int su_daemon_main(int argc, char **argv) { } // Handle namespaces - switch (su_ctx->info->dbs.v[SU_MNT_NS]) { - case NAMESPACE_MODE_GLOBAL: - LOGD("su: use global namespace\n"); - break; - case NAMESPACE_MODE_REQUESTER: - LOGD("su: use namespace of pid=[%d]\n", su_ctx->pid); - if (switch_mnt_ns(su_ctx->pid)) { - LOGD("su: setns failed, fallback to isolated\n"); + switch (DB_SET(su_ctx->info, SU_MNT_NS)) { + case NAMESPACE_MODE_GLOBAL: + LOGD("su: use global namespace\n"); + break; + case NAMESPACE_MODE_REQUESTER: + LOGD("su: use namespace of pid=[%d]\n", su_ctx->pid); + if (switch_mnt_ns(su_ctx->pid)) { + LOGD("su: setns failed, fallback to isolated\n"); + xunshare(CLONE_NEWNS); + } + break; + case NAMESPACE_MODE_ISOLATE: + LOGD("su: use new isolated namespace\n"); xunshare(CLONE_NEWNS); - } - break; - case NAMESPACE_MODE_ISOLATE: - LOGD("su: use new isolated namespace\n"); - xunshare(CLONE_NEWNS); - break; + break; } // Change directory to cwd diff --git a/native/jni/su/su.h b/native/jni/su/su.h index d7a814137..b66df0b19 100644 --- a/native/jni/su/su.h +++ b/native/jni/su/su.h @@ -21,13 +21,16 @@ struct su_info { struct db_settings dbs; struct db_strings str; struct su_access access; - struct stat manager_stat; + struct stat mgr_st; /* These should be guarded with global cache lock */ int ref; int life; }; +#define DB_SET(i, e) (i)->dbs.v[e] +#define DB_STR(i, e) (i)->str.s[e] + struct su_request { unsigned uid; int login; diff --git a/native/jni/su/su_daemon.c b/native/jni/su/su_daemon.c index 8232dc3e1..f9bf6b33c 100644 --- a/native/jni/su/su_daemon.c +++ b/native/jni/su/su_daemon.c @@ -85,7 +85,7 @@ static void database_check(struct su_info *info) { get_db_strings(db, -1, &info->str); // Check multiuser settings - switch (info->dbs.v[SU_MULTIUSER_MODE]) { + switch (DB_SET(info, SU_MULTIUSER_MODE)) { case MULTIUSER_MODE_OWNER_ONLY: if (info->uid / 100000) { uid = -1; @@ -107,7 +107,7 @@ static void database_check(struct su_info *info) { // We need to check our manager if (info->access.log || info->access.notify) - validate_manager(info->str.s[SU_MANAGER], uid / 100000, &info->manager_stat); + validate_manager(DB_STR(info, SU_MANAGER), uid / 100000, &info->mgr_st); } static struct su_info *get_su_info(unsigned uid) { @@ -120,13 +120,10 @@ static struct su_info *get_su_info(unsigned uid) { info = cache; } else { cache_miss = 1; - info = malloc(sizeof(*info)); + info = xcalloc(1, sizeof(*info)); info->uid = uid; info->dbs = DEFAULT_DB_SETTINGS; info->access = DEFAULT_SU_ACCESS; - INIT_DB_STRINGS(&info->str); - info->ref = 0; - info->count = 0; pthread_mutex_init(&info->lock, NULL); cache = info; } @@ -154,7 +151,7 @@ static struct su_info *get_su_info(unsigned uid) { database_check(info); // Check su access settings - switch (info->dbs.v[ROOT_ACCESS]) { + switch (DB_SET(info, ROOT_ACCESS)) { case ROOT_ACCESS_DISABLED: LOGE("Root access is disabled!\n"); info->access = NO_SU_ACCESS; @@ -177,7 +174,7 @@ static struct su_info *get_su_info(unsigned uid) { } // If it's the manager, allow it silently - if ((info->uid % 100000) == (info->manager_stat.st_uid % 100000)) + if ((info->uid % 100000) == (info->mgr_st.st_uid % 100000)) info->access = SILENT_SU_ACCESS; // Allow if it's root @@ -185,7 +182,7 @@ static struct su_info *get_su_info(unsigned uid) { info->access = SILENT_SU_ACCESS; // If still not determined, check if manager exists - if (info->access.policy == QUERY && info->str.s[SU_MANAGER][0] == '\0') + if (info->access.policy == QUERY && DB_STR(info, SU_MANAGER)[0] == '\0') info->access = NO_SU_ACCESS; } return info; @@ -297,16 +294,16 @@ void su_daemon_receiver(int client, struct ucred *credential) { // Default values struct su_context ctx = { - .info = get_su_info(credential->uid), - .to = { - .uid = UID_ROOT, - .login = 0, - .keepenv = 0, - .shell = DEFAULT_SHELL, - .command = NULL, - }, - .pid = credential->pid, - .pipefd = { -1, -1 } + .info = get_su_info(credential->uid), + .to = { + .uid = UID_ROOT, + .login = 0, + .keepenv = 0, + .shell = DEFAULT_SHELL, + .command = NULL, + }, + .pid = credential->pid, + .pipefd = { -1, -1 } }; // Fail fast