Close race condition in binderDied()

It was possible for a binderDied() call to occur while the death
recipient list containing the object was being iterated, in which
case we could invalidate an object reference out from under the
iteration, causing a VM abort.  We now interlock the binderDied()
deref operation with the list's locking semantics to prevent this.

Bug 15831054

Change-Id: If0027d3ac4da1153284a425dd9b2819a203481ab

diff --git a/core/jni/android_util_Binder.cpp b/core/jni/android_util_Binder.cpp
index fb82075..634c9f7 100644 (file)
--- a/core/jni/android_util_Binder.cpp
+++ b/core/jni/android_util_Binder.cpp
@@ -358,6 +358,8 @@ public:
     void add(const sp<JavaDeathRecipient>& recipient);
     void remove(const sp<JavaDeathRecipient>& recipient);
     sp<JavaDeathRecipient> find(jobject recipient);
+
+    Mutex& lock();  // Use with care; specifically for mutual exclusion during binder death
 };
 
 // ----------------------------------------------------------------------------
@@ -392,11 +394,18 @@ public:
                         "*** Uncaught exception returned from death notification!");
             }
 
-            // Demote from strong ref to weak after binderDied() has been delivered,
-            // to allow the DeathRecipient and BinderProxy to be GC'd if no longer needed.
-            mObjectWeak = env->NewWeakGlobalRef(mObject);
-            env->DeleteGlobalRef(mObject);
-            mObject = NULL;
+            // Serialize with our containing DeathRecipientList so that we can't
+            // delete the global ref on mObject while the list is being iterated.
+            sp<DeathRecipientList> list = mList.promote();
+            if (list != NULL) {
+                AutoMutex _l(list->lock());
+
+                // Demote from strong ref to weak after binderDied() has been delivered,
+                // to allow the DeathRecipient and BinderProxy to be GC'd if no longer needed.
+                mObjectWeak = env->NewWeakGlobalRef(mObject);
+                env->DeleteGlobalRef(mObject);
+                mObject = NULL;
+            }
         }
     }
 
@@ -518,6 +527,10 @@ sp<JavaDeathRecipient> DeathRecipientList::find(jobject recipient) {
     return NULL;
 }
 
+Mutex& DeathRecipientList::lock() {
+    return mLock;
+}
+
 // ----------------------------------------------------------------------------
 
 namespace android {