Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 9556bb12 authored by Andrei Homescu's avatar Andrei Homescu
Browse files

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
parent 3dd6b0f0

libs/binder/ndk/ibinder.cpp

+20 −8
Original line number Diff line number Diff line
@@ -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)

libs/binder/ndk/ibinder_internal.h

+5 −0
Original line number Diff line number Diff line
@@ -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.