From 36f53993418ed77f323995a7421260326923bb25 Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Wed, 6 Sep 2017 10:07:37 -0700 Subject: [PATCH] logd: handle uidToName() directly uidToName() originally used a separate worker thread with additional group permissions. Threads are not security boundaries however, so these group permissions are removed in a previous change. This change handles the lookup for uidToName() directly without using a separate thread. Test: boot CF, logd unit tests Change-Id: If245388bc221bc77102a0bbcee82c8f42b140760 --- logd/main.cpp | 69 ++++++++++++++++----------------------------------- 1 file changed, 21 insertions(+), 48 deletions(-) diff --git a/logd/main.cpp b/logd/main.cpp index 28dcc5ab7..23bbf864d 100644 --- a/logd/main.cpp +++ b/logd/main.cpp @@ -150,43 +150,14 @@ void android::prdebug(const char* fmt, ...) { } } -static sem_t uidName; -static uid_t uid; -static char* name; - static sem_t reinit; static bool reinit_running = false; static LogBuffer* logBuf = nullptr; -static bool package_list_parser_cb(pkg_info* info, void* /* userdata */) { - bool rc = true; - if (info->uid == uid) { - name = strdup(info->name); - // false to stop processing - rc = false; - } - - packagelist_free(info); - return rc; -} - static void* reinit_thread_start(void* /*obj*/) { prctl(PR_SET_NAME, "logd.daemon"); while (reinit_running && !sem_wait(&reinit) && reinit_running) { - // uidToName Privileged Worker - if (uid) { - name = nullptr; - - // if we got the perms wrong above, this would spam if we reported - // problems with acquisition of an uid name from the packages. - (void)packagelist_parse(package_list_parser_cb, nullptr); - - uid = 0; - sem_post(&uidName); - continue; - } - if (fdDmesg >= 0) { static const char reinit_message[] = { KMSG_PRIORITY(LOG_INFO), 'l', @@ -223,26 +194,30 @@ static void* reinit_thread_start(void* /*obj*/) { return nullptr; } -static sem_t sem_name; - char* android::uidToName(uid_t u) { - if (!u || !reinit_running) { - return nullptr; - } + struct Userdata { + uid_t uid; + char* name; + } userdata = { + .uid = u, + .name = nullptr, + }; - sem_wait(&sem_name); + packagelist_parse( + [](pkg_info* info, void* callback_parameter) { + auto userdata = reinterpret_cast(callback_parameter); + bool result = true; + if (info->uid == userdata->uid) { + userdata->name = strdup(info->name); + // false to stop processing + result = false; + } + packagelist_free(info); + return result; + }, + &userdata); - // Not multi-thread safe, we use sem_name to protect - uid = u; - - name = nullptr; - sem_post(&reinit); - sem_wait(&uidName); - char* ret = name; - - sem_post(&sem_name); - - return ret; + return userdata.name; } // Serves as a global method to trigger reinitialization @@ -363,8 +338,6 @@ int main(int argc, char* argv[]) { // Reinit Thread sem_init(&reinit, 0, 0); - sem_init(&uidName, 0, 0); - sem_init(&sem_name, 0, 1); pthread_attr_t attr; if (!pthread_attr_init(&attr)) { struct sched_param param;