commit ab8ad62939800811249ff5c28183b29179277b37
author Mark Salyzyn <salyzyn@google.com> Fri Oct 21 09:46:42 2016 -0700
committer Mark Salyzyn <salyzyn@google.com> Mon Oct 24 07:41:05 2016 -0700
tree 0d19d3726cf0542c5c3b90ac30ddcc576f5f41a8
parent a3bd97a019084035334cd18eae5485076d1282de

logd: mLastWorstPidOFSystem crash

(cherry pick from commit fa07f9dc4b4d101a49fba5dbbf35c88cdfec4433)

mLastWorstPidOfSystem is filled with iterator references
that are not from AID_SYSTEM to aid the performance. But
we only clear entries from the list during erase if they
are from AID_SYSTEM. Remove the filter check in erase so
the stale references will be removed.

The conditions that caused this failure are difficult to
reproduce and are rare.

Test: gTests logd-unit-tests, liblog-unit-tests and logcat-unit-tests
Bug: 32247044
Bug: 31237377
Change-Id: Ie405dd643203b816cac15eef5c97600551cee450

logd/LogBuffer.cpp

diff --git a/logd/LogBuffer.cpp b/logd/LogBuffer.cpp
index 79a7e8c..0497a89 100644
--- a/logd/LogBuffer.cpp
+++ b/logd/LogBuffer.cpp
@@ -313,6 +313,9 @@
     LogBufferElement *element = *it;
     log_id_t id = element->getLogId();
 
+    // Remove iterator references in the various lists that will become stale
+    // after the element is erased from the main logging list.
+
     {   // start of scope for uid found iterator
         LogBufferIteratorMap::iterator found =
             mLastWorstUid[id].find(element->getUid());
@@ -322,8 +325,8 @@
         }
     }
 
-    if (element->getUid() == AID_SYSTEM) {
-        // start of scope for pid found iterator
+    {   // start of scope for pid found iterator
+        // element->getUid() may not be AID_SYSTEM for next-best-watermark.
         LogBufferPidIteratorMap::iterator found =
             mLastWorstPidOfSystem[id].find(element->getPid());
         if ((found != mLastWorstPidOfSystem[id].end())
@@ -639,6 +642,7 @@
                 ++it;
                 continue;
             }
+            // below this point element->getLogId() == id
 
             if (leading && (!mLastSet[id] || ((*mLast[id])->getLogId() != id))) {
                 mLast[id] = it;
@@ -691,6 +695,8 @@
                         && ((!gc && (element->getPid() == worstPid))
                             || (mLastWorstPidOfSystem[id].find(element->getPid())
                                 == mLastWorstPidOfSystem[id].end()))) {
+                    // element->getUid() may not be AID_SYSTEM, next best
+                    // watermark if current one empty.
                     mLastWorstPidOfSystem[id][element->getPid()] = it;
                 }
                 if ((!gc && !worstPid && (element->getUid() == worst))
@@ -709,6 +715,8 @@
                 ++it;
                 continue;
             }
+            // key == worst below here
+            // If worstPid set, then element->getPid() == worstPid below here
 
             pruneRows--;
             if (pruneRows == 0) {
@@ -732,6 +740,8 @@
                     if (worstPid && (!gc
                                 || (mLastWorstPidOfSystem[id].find(worstPid)
                                     == mLastWorstPidOfSystem[id].end()))) {
+                        // element->getUid() may not be AID_SYSTEM, next best
+                        // watermark if current one empty.
                         mLastWorstPidOfSystem[id][worstPid] = it;
                     }
                     if ((!gc && !worstPid) || (mLastWorstUid[id].find(worst)