From 9556bb1250d05c2447ad4694be4e84b319b5aea9 Mon Sep 17 00:00:00 2001
From: Andrei Homescu <ah@immunant.com>
Date: Mon, 21 Sep 2020 16:59:45 -0700
Subject: Lock accesses to mClazz in AIBinder with a mutex

The associateClass method mutates the mClazz member of AIBinder in a
non-thread-safe way. This patch fixes this problem by adding a
std::mutex locking that variable, fixing test flakiness in the Rust
tests for the AIDL compiler

Test: atest libbinder_ndk_unit_test binderVendorDoubleLoadTest CtsNdkBinderTestCases
Test: atest aidl_integration_test for AIDL Rust tests
Change-Id: I4b0ec21755a9995c1f591ab937e9a3280992be80
---
 libs/binder/ndk/ibinder.cpp        | 28 ++++++++++++++++++++--------
 libs/binder/ndk/ibinder_internal.h |  5 +++++
 2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/libs/binder/ndk/ibinder.cpp b/libs/binder/ndk/ibinder.cpp
index 6995e77667..d7075b2ede 100644
--- a/libs/binder/ndk/ibinder.cpp
+++ b/libs/binder/ndk/ibinder.cpp
@@ -73,12 +73,11 @@ void clean(const void* id, void* obj, void* cookie) {
 AIBinder::AIBinder(const AIBinder_Class* clazz) : mClazz(clazz) {}
 AIBinder::~AIBinder() {}
 
-bool AIBinder::associateClass(const AIBinder_Class* clazz) {
-    if (clazz == nullptr) return false;
+std::optional<bool> AIBinder::associateClassInternal(const AIBinder_Class* clazz,
+                                                     const String8& newDescriptor, bool set) {
+    std::lock_guard<std::mutex> lock(mClazzMutex);
     if (mClazz == clazz) return true;
 
-    String8 newDescriptor(clazz->getInterfaceDescriptor());
-
     if (mClazz != nullptr) {
         String8 currentDescriptor(mClazz->getInterfaceDescriptor());
         if (newDescriptor == currentDescriptor) {
@@ -97,6 +96,22 @@ bool AIBinder::associateClass(const AIBinder_Class* clazz) {
         return false;
     }
 
+    if (set) {
+        // if this is a local object, it's not one known to libbinder_ndk
+        mClazz = clazz;
+    }
+
+    return {};
+}
+
+bool AIBinder::associateClass(const AIBinder_Class* clazz) {
+    if (clazz == nullptr) return false;
+
+    String8 newDescriptor(clazz->getInterfaceDescriptor());
+
+    auto result = associateClassInternal(clazz, newDescriptor, false);
+    if (result.has_value()) return *result;
+
     CHECK(asABpBinder() != nullptr);  // ABBinder always has a descriptor
 
     String8 descriptor(getBinder()->getInterfaceDescriptor());
@@ -112,10 +127,7 @@ bool AIBinder::associateClass(const AIBinder_Class* clazz) {
         return false;
     }
 
-    // if this is a local object, it's not one known to libbinder_ndk
-    mClazz = clazz;
-
-    return true;
+    return associateClassInternal(clazz, newDescriptor, true).value_or(true);
 }
 
 ABBinder::ABBinder(const AIBinder_Class* clazz, void* userData)
diff --git a/libs/binder/ndk/ibinder_internal.h b/libs/binder/ndk/ibinder_internal.h
index 57794279f2..f4e2704882 100644
--- a/libs/binder/ndk/ibinder_internal.h
+++ b/libs/binder/ndk/ibinder_internal.h
@@ -22,6 +22,7 @@
 
 #include <atomic>
 #include <mutex>
+#include <optional>
 #include <vector>
 
 #include <binder/Binder.h>
@@ -52,10 +53,14 @@ struct AIBinder : public virtual ::android::RefBase {
     }
 
    private:
+    std::optional<bool> associateClassInternal(const AIBinder_Class* clazz,
+                                               const ::android::String8& newDescriptor, bool set);
+
     // AIBinder instance is instance of this class for a local object. In order to transact on a
     // remote object, this also must be set for simplicity (although right now, only the
     // interfaceDescriptor from it is used).
     const AIBinder_Class* mClazz;
+    std::mutex mClazzMutex;
 };
 
 // This is a local AIBinder object with a known class.
-- 
cgit v1.2.3-59-g8ed1b