commit 10acf7832f065bcdb07a72bae7e0d59a7bd90250
author Mark Salyzyn <salyzyn@google.com> Wed Jun 24 16:22:54 2015 -0700
committer Mark Salyzyn <salyzyn@google.com> Thu Jun 25 07:39:24 2015 -0700
tree 7f060af8f54b96d93368cf22e0acd6c2140c5872
parent 5f14edb8db353d4da34b38241b08839fb94b4fdd

logd: serialize accesses to stats helpers

Quick low-risk to resolve possible hash table corruption.
Resolved an unlikely path memory leak.

ToDo: replace lock with nested lock so no lock
      helpers are required.

Bug: 22068332
Change-Id: I303ab06608502c7d61d42f111a9c43366f184d0c

logd/LogAudit.cpp

diff --git a/logd/LogAudit.cpp b/logd/LogAudit.cpp
index 4ec2e59..4b3547c 100644
--- a/logd/LogAudit.cpp
+++ b/logd/LogAudit.cpp
@@ -145,7 +145,9 @@
             ++cp;
         }
         tid = pid;
+        logbuf->lock();
         uid = logbuf->pidToUid(pid);
+        logbuf->unlock();
         memmove(pidptr, cp, strlen(cp) + 1);
     }
 
@@ -180,14 +182,20 @@
     static const char comm_str[] = " comm=\"";
     const char *comm = strstr(str, comm_str);
     const char *estr = str + strlen(str);
+    char *commfree = NULL;
     if (comm) {
         estr = comm;
         comm += sizeof(comm_str) - 1;
     } else if (pid == getpid()) {
         pid = tid;
         comm = "auditd";
-    } else if (!(comm = logbuf->pidToName(pid))) {
-        comm = "unknown";
+    } else {
+        logbuf->lock();
+        comm = commfree = logbuf->pidToName(pid);
+        logbuf->unlock();
+        if (!comm) {
+            comm = "unknown";
+        }
     }
 
     const char *ecomm = strchr(comm, '"');
@@ -218,6 +226,7 @@
         }
     }
 
+    free(commfree);
     free(str);
 
     if (notify) {

logd/LogBuffer.h

diff --git a/logd/LogBuffer.h b/logd/LogBuffer.h
index 00b19b6..a13fded 100644
--- a/logd/LogBuffer.h
+++ b/logd/LogBuffer.h
@@ -71,10 +71,12 @@
     // *strp uses malloc, use free to release.
     void formatPrune(char **strp) { mPrune.format(strp); }
 
-    // helper
+    // helper must be protected directly or implicitly by lock()/unlock()
     char *pidToName(pid_t pid) { return stats.pidToName(pid); }
     uid_t pidToUid(pid_t pid) { return stats.pidToUid(pid); }
     char *uidToName(uid_t uid) { return stats.uidToName(uid); }
+    void lock() { pthread_mutex_lock(&mLogElementsLock); }
+    void unlock() { pthread_mutex_unlock(&mLogElementsLock); }
 
 private:
     void maybePrune(log_id_t id);

logd/LogBufferElement.cpp

diff --git a/logd/LogBufferElement.cpp b/logd/LogBufferElement.cpp
index 3f5fdce..9fb1439 100644
--- a/logd/LogBufferElement.cpp
+++ b/logd/LogBufferElement.cpp
@@ -111,13 +111,17 @@
     }
 
     static const char format_uid[] = "uid=%u%s%s expire %u line%s";
+    parent->lock();
     char *name = parent->uidToName(mUid);
+    parent->unlock();
     char *commName = android::tidToName(mTid);
     if (!commName && (mTid != mPid)) {
         commName = android::tidToName(mPid);
     }
     if (!commName) {
+        parent->lock();
         commName = parent->pidToName(mPid);
+        parent->unlock();
     }
     size_t len = name ? strlen(name) : 0;
     if (len && commName && !strncmp(name, commName, len)) {

logd/LogStatistics.h

diff --git a/logd/LogStatistics.h b/logd/LogStatistics.h
index b9e9650..760d6b2 100644
--- a/logd/LogStatistics.h
+++ b/logd/LogStatistics.h
@@ -334,7 +334,7 @@
     // *strp = malloc, balance with free
     void format(char **strp, uid_t uid, unsigned int logMask);
 
-    // helper
+    // helper (must be locked directly or implicitly by mLogElementsLock)
     char *pidToName(pid_t pid);
     uid_t pidToUid(pid_t pid);
     char *uidToName(uid_t uid);