Resolve reference leak and warnings in binder interception (#122)
This merge addresses a critical strong reference leak in the ioctl hook that occurred during binder transaction interception. The leak was caused by a double increment of the reference count, once manually and once by a smart pointer's constructor, with only a single corresponding decrement. The fix ensures a balanced increment and decrement, preventing the leak and subsequent crashes. Additionally, this change: - Reverts a now-unnecessary compatibility layer for the Android 11 RefBase ABI. - Implements `getInterfaceDescriptor` in the `BinderStub` to silence framework warnings that appeared after the primary leak was fixed.
This commit is contained in:
@@ -12,7 +12,7 @@ add_subdirectory(external/LSPlt/lsplt/src/main/jni)
|
|||||||
|
|
||||||
add_compile_definitions(BINDER_DISABLE_NATIVE_HANDLE)
|
add_compile_definitions(BINDER_DISABLE_NATIVE_HANDLE)
|
||||||
add_library(utils SHARED stub/stub_utils.cpp)
|
add_library(utils SHARED stub/stub_utils.cpp)
|
||||||
target_include_directories(utils PUBLIC external/AOSP/include compat)
|
target_include_directories(utils PUBLIC external/AOSP/include)
|
||||||
|
|
||||||
add_library(binder SHARED stub/stub_binder.cpp)
|
add_library(binder SHARED stub/stub_binder.cpp)
|
||||||
target_include_directories(binder PUBLIC external/AOSP/include)
|
target_include_directories(binder PUBLIC external/AOSP/include)
|
||||||
@@ -22,7 +22,7 @@ add_executable(libinject.so inject/main.cpp inject/utils.cpp)
|
|||||||
target_include_directories(libinject.so PUBLIC include)
|
target_include_directories(libinject.so PUBLIC include)
|
||||||
target_link_libraries(libinject.so PRIVATE lsplt_static)
|
target_link_libraries(libinject.so PRIVATE lsplt_static)
|
||||||
|
|
||||||
add_library(${CMAKE_PROJECT_NAME} SHARED binder_interceptor.cpp compat/refbase_compat.cpp)
|
add_library(${CMAKE_PROJECT_NAME} SHARED binder_interceptor.cpp)
|
||||||
target_include_directories(${CMAKE_PROJECT_NAME} PUBLIC external/linux-kernel/include include)
|
target_include_directories(${CMAKE_PROJECT_NAME} PUBLIC external/linux-kernel/include include)
|
||||||
target_link_libraries(${CMAKE_PROJECT_NAME} PRIVATE binder lsplt_static utils)
|
target_link_libraries(${CMAKE_PROJECT_NAME} PRIVATE binder lsplt_static utils)
|
||||||
|
|
||||||
|
|||||||
@@ -276,6 +276,12 @@ static sp<BinderInterceptor> g_interceptor_instance = nullptr;
|
|||||||
// =============================================================================================
|
// =============================================================================================
|
||||||
|
|
||||||
class BinderStub : public BBinder {
|
class BinderStub : public BBinder {
|
||||||
|
public:
|
||||||
|
const String16& getInterfaceDescriptor() const override {
|
||||||
|
static const String16 kDescriptor("org.matrix.TEESimulator.BinderStub");
|
||||||
|
return kDescriptor;
|
||||||
|
}
|
||||||
|
|
||||||
protected:
|
protected:
|
||||||
status_t onTransact(uint32_t code, const Parcel &data, Parcel *reply, uint32_t flags) override {
|
status_t onTransact(uint32_t code, const Parcel &data, Parcel *reply, uint32_t flags) override {
|
||||||
if (code != intercept::kBackdoorCode) {
|
if (code != intercept::kBackdoorCode) {
|
||||||
@@ -376,18 +382,17 @@ void inspectAndRewriteTransaction(binder_transaction_data *txn_data) {
|
|||||||
// The raw pointer to the binder object itself is stored in the cookie
|
// The raw pointer to the binder object itself is stored in the cookie
|
||||||
BBinder *target_binder_ptr = reinterpret_cast<BBinder *>(txn_data->cookie);
|
BBinder *target_binder_ptr = reinterpret_cast<BBinder *>(txn_data->cookie);
|
||||||
|
|
||||||
// This is safe ONLY because we successfully called attemptIncStrong().
|
// Create a weak pointer for the lookup and to store in our context map.
|
||||||
// The sp<> constructor will not increment the ref count again, it just adopts the one we have.
|
// This is safe because we are holding a strong reference.
|
||||||
// When sp_target goes out of scope, it will call decStrong(), releasing our temporary reference.
|
wp<BBinder> wp_target = target_binder_ptr;
|
||||||
sp<BBinder> sp_target = sp<BBinder>::fromExisting(target_binder_ptr);
|
|
||||||
|
|
||||||
// Now we can safely use sp_target (which implicitly converts to a wp) for the lookup.
|
if (g_interceptor_instance->isBinderIntercepted(wp_target)) {
|
||||||
if (g_interceptor_instance->isBinderIntercepted(sp_target)) {
|
|
||||||
info.transaction_code = txn_data->code;
|
info.transaction_code = txn_data->code;
|
||||||
info.target_binder = sp_target; // Assign the valid weak pointer
|
info.target_binder = wp_target; // Assign the valid weak pointer
|
||||||
hijack = true;
|
hijack = true;
|
||||||
}
|
}
|
||||||
// No need to manually call decStrong(); the sp destructor handles it.
|
// Manually release the temporary strong reference we acquired at the start.
|
||||||
|
target_binder_ptr->decStrong(nullptr);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1,61 +0,0 @@
|
|||||||
#include "refbase_compat.h"
|
|
||||||
#include "utils/RefBase.h"
|
|
||||||
#include <atomic>
|
|
||||||
#include <cstdlib>
|
|
||||||
#include <cstring> // For memcpy
|
|
||||||
#include <dlfcn.h>
|
|
||||||
#include <mutex>
|
|
||||||
#include <sys/system_properties.h>
|
|
||||||
|
|
||||||
namespace android {
|
|
||||||
|
|
||||||
// Helper function to get the Android API level at runtime.
|
|
||||||
// It caches the result for performance.
|
|
||||||
int32_t get_android_api_level() {
|
|
||||||
static std::atomic<int32_t> api_level = -1;
|
|
||||||
if (api_level.load(std::memory_order_relaxed) == -1) {
|
|
||||||
char sdk_version_str[PROP_VALUE_MAX];
|
|
||||||
if (__system_property_get("ro.build.version.sdk", sdk_version_str) > 0) {
|
|
||||||
api_level.store(atoi(sdk_version_str), std::memory_order_relaxed);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
return api_level.load(std::memory_order_relaxed);
|
|
||||||
}
|
|
||||||
|
|
||||||
// Define the function pointer type for the const member function
|
|
||||||
// RefBase::incStrongRequireStrong.
|
|
||||||
using incStrongRequireStrong_t = void (RefBase::*)(const void *) const;
|
|
||||||
|
|
||||||
// This is the implementation of our compatibility wrapper.
|
|
||||||
void incStrongFromExisting(const RefBase *ref, const void *id) {
|
|
||||||
// Only attempt to use the new function on Android 12 (API 31) or higher.
|
|
||||||
if (get_android_api_level() >= 31) {
|
|
||||||
static incStrongRequireStrong_t sIncStrongRequireStrong = nullptr;
|
|
||||||
static std::once_flag sFlag;
|
|
||||||
|
|
||||||
// Thread-safe, one-time initialization.
|
|
||||||
std::call_once(sFlag, []() {
|
|
||||||
// Find the symbol in the already loaded libraries.
|
|
||||||
// The mangled symbol is _ZNK7android7RefBase22incStrongRequireStrongEPKv
|
|
||||||
void *sym = dlsym(RTLD_DEFAULT,
|
|
||||||
"_ZNK7android7RefBase22incStrongRequireStrongEPKv");
|
|
||||||
if (sym) {
|
|
||||||
// Safely cast the void* symbol to our member function pointer.
|
|
||||||
memcpy(&sIncStrongRequireStrong, &sym, sizeof(void *));
|
|
||||||
}
|
|
||||||
});
|
|
||||||
|
|
||||||
if (sIncStrongRequireStrong) {
|
|
||||||
// If the symbol was found, call it as member function.
|
|
||||||
(ref->*sIncStrongRequireStrong)(id);
|
|
||||||
return; // Success, we are done.
|
|
||||||
}
|
|
||||||
// If dlsym failed for any reason, we fall through to the old method.
|
|
||||||
}
|
|
||||||
|
|
||||||
// Fallback for older Android versions or if dlsym failed.
|
|
||||||
// This calls the universally available incStrong method.
|
|
||||||
ref->incStrong(id);
|
|
||||||
}
|
|
||||||
|
|
||||||
} // namespace android
|
|
||||||
@@ -1,11 +0,0 @@
|
|||||||
#pragma once
|
|
||||||
|
|
||||||
namespace android {
|
|
||||||
|
|
||||||
// Forward-declare the RefBase class.
|
|
||||||
class RefBase;
|
|
||||||
|
|
||||||
// Declares our compatibility function.
|
|
||||||
void incStrongFromExisting(const RefBase *ref, const void *id);
|
|
||||||
|
|
||||||
} // namespace android
|
|
||||||
@@ -17,7 +17,6 @@
|
|||||||
#ifndef ANDROID_STRONG_POINTER_H
|
#ifndef ANDROID_STRONG_POINTER_H
|
||||||
#define ANDROID_STRONG_POINTER_H
|
#define ANDROID_STRONG_POINTER_H
|
||||||
|
|
||||||
#include "refbase_compat.h"
|
|
||||||
#include <functional>
|
#include <functional>
|
||||||
#include <type_traits> // for common_type.
|
#include <type_traits> // for common_type.
|
||||||
|
|
||||||
@@ -213,7 +212,7 @@ sp<T> sp<T>::make(Args&&... args) {
|
|||||||
template <typename T>
|
template <typename T>
|
||||||
sp<T> sp<T>::fromExisting(T* other) {
|
sp<T> sp<T>::fromExisting(T* other) {
|
||||||
if (other) {
|
if (other) {
|
||||||
incStrongFromExisting(other, other);
|
other->incStrongRequireStrong(other);
|
||||||
sp<T> result;
|
sp<T> result;
|
||||||
result.m_ptr = other;
|
result.m_ptr = other;
|
||||||
return result;
|
return result;
|
||||||
|
|||||||
Reference in New Issue
Block a user