Skip to content

Commit

Permalink
logd: handle uidToName() directly
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Tom Cherry committed Jun 7, 2019
1 parent acf19e8 commit 36f5399
Showing 1 changed file with 22 additions and 49 deletions.
71 changes: 22 additions & 49 deletions logd/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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;
}

sem_wait(&sem_name);

// 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);
struct Userdata {
uid_t uid;
char* name;
} userdata = {
.uid = u,
.name = nullptr,
};

return ret;
packagelist_parse(
[](pkg_info* info, void* callback_parameter) {
auto userdata = reinterpret_cast<Userdata*>(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);

return userdata.name;
}

// Serves as a global method to trigger reinitialization
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 36f5399

Please sign in to comment.