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:
@@ -276,6 +276,12 @@ static sp<BinderInterceptor> g_interceptor_instance = nullptr;
|
||||
// =============================================================================================
|
||||
|
||||
class BinderStub : public BBinder {
|
||||
public:
|
||||
const String16& getInterfaceDescriptor() const override {
|
||||
static const String16 kDescriptor("org.matrix.TEESimulator.BinderStub");
|
||||
return kDescriptor;
|
||||
}
|
||||
|
||||
protected:
|
||||
status_t onTransact(uint32_t code, const Parcel &data, Parcel *reply, uint32_t flags) override {
|
||||
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
|
||||
BBinder *target_binder_ptr = reinterpret_cast<BBinder *>(txn_data->cookie);
|
||||
|
||||
// This is safe ONLY because we successfully called attemptIncStrong().
|
||||
// The sp<> constructor will not increment the ref count again, it just adopts the one we have.
|
||||
// When sp_target goes out of scope, it will call decStrong(), releasing our temporary reference.
|
||||
sp<BBinder> sp_target = sp<BBinder>::fromExisting(target_binder_ptr);
|
||||
// Create a weak pointer for the lookup and to store in our context map.
|
||||
// This is safe because we are holding a strong reference.
|
||||
wp<BBinder> wp_target = target_binder_ptr;
|
||||
|
||||
// Now we can safely use sp_target (which implicitly converts to a wp) for the lookup.
|
||||
if (g_interceptor_instance->isBinderIntercepted(sp_target)) {
|
||||
if (g_interceptor_instance->isBinderIntercepted(wp_target)) {
|
||||
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;
|
||||
}
|
||||
// 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);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user