From 418914a7c54f4aa0418b6ddbb5096b66286cd80e Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Wed, 28 Jun 2023 21:47:14 +0000 Subject: libbinder_ndk: fwd fuzzing status to NDK binders When passing binders into NDK backend services, we always type check them immediately. This allows errors to show up earlier there, but may be inefficient because the type will also be checked on every transaction. Anyway... This poses a problem for our automatic fuzzers because callbacks passed into services (e.g. RandomBinder) will be ignored for NDK backend fuzzers unless they correctly guess their interface descriptor. There are a few things we could do: - use random strings from the environment - export a list of possible interface descriptors from AIDL - generate our corpuses from other data However, the simplest thing we can do is ignore the check, which this CL does. Of course, it isn't great to continue differentiated fuzzer behavior from actual behavior, so we'd like to revert this once we have a more comprehensive solution. However, callbacks are a fundamental AIDL building blocks, so forcing good fuzzer coverage for these pieces seems justified. Bug: N/A Test: I added an abort in an NDK backend service. Without this change, that path is never found, but with this change, it was hit after only ~6,000 iterations. Change-Id: I4cbe5c56b93b9300fbd57e72e24075c02df38ba9 --- libs/binder/Parcel.cpp | 4 ++++ libs/binder/include/binder/Parcel.h | 1 + libs/binder/ndk/ibinder.cpp | 2 +- libs/binder/ndk/ibinder_internal.h | 4 ++++ libs/binder/ndk/parcel.cpp | 7 +++++++ 5 files changed, 17 insertions(+), 1 deletion(-) diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index 2c2a1b636e..9b685f9145 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -992,6 +992,10 @@ void Parcel::setServiceFuzzing() { mServiceFuzzing = true; } +bool Parcel::isServiceFuzzing() const { + return mServiceFuzzing; +} + binder::Status Parcel::enforceNoDataAvail() const { if (!mEnforceNoDataAvail) { return binder::Status::ok(); diff --git a/libs/binder/include/binder/Parcel.h b/libs/binder/include/binder/Parcel.h index 15bb325459..87b63e5a50 100644 --- a/libs/binder/include/binder/Parcel.h +++ b/libs/binder/include/binder/Parcel.h @@ -152,6 +152,7 @@ public: // When fuzzing, we want to remove certain ABI checks that cause significant // lost coverage, and we also want to avoid logs that cost too much to write. void setServiceFuzzing(); + bool isServiceFuzzing() const; void freeData(); diff --git a/libs/binder/ndk/ibinder.cpp b/libs/binder/ndk/ibinder.cpp index d0de7b96b5..f7dd9c9715 100644 --- a/libs/binder/ndk/ibinder.cpp +++ b/libs/binder/ndk/ibinder.cpp @@ -137,7 +137,7 @@ bool AIBinder::associateClass(const AIBinder_Class* clazz) { // since it's an error condition. Do the comparison after we take the lock and // check the pointer equality fast path. By always taking the lock, it's also // more flake-proof. However, the check is not dependent on the lock. - if (descriptor != newDescriptor) { + if (descriptor != newDescriptor && !(asABpBinder() && asABpBinder()->isServiceFuzzing())) { if (getBinder()->isBinderAlive()) { LOG(ERROR) << __func__ << ": Expecting binder to have class '" << newDescriptor << "' but descriptor is actually '" << SanitizeString(descriptor) << "'."; diff --git a/libs/binder/ndk/ibinder_internal.h b/libs/binder/ndk/ibinder_internal.h index 67bb092f0f..9d5368f674 100644 --- a/libs/binder/ndk/ibinder_internal.h +++ b/libs/binder/ndk/ibinder_internal.h @@ -104,10 +104,14 @@ struct ABpBinder : public AIBinder { ::android::sp<::android::IBinder> getBinder() override { return mRemote; } ABpBinder* asABpBinder() override { return this; } + bool isServiceFuzzing() const { return mServiceFuzzing; } + void setServiceFuzzing() { mServiceFuzzing = true; } + private: friend android::sp; explicit ABpBinder(const ::android::sp<::android::IBinder>& binder); ::android::sp<::android::IBinder> mRemote; + bool mServiceFuzzing = false; }; struct AIBinder_Class { diff --git a/libs/binder/ndk/parcel.cpp b/libs/binder/ndk/parcel.cpp index b5a2e2ff0b..037aa2e120 100644 --- a/libs/binder/ndk/parcel.cpp +++ b/libs/binder/ndk/parcel.cpp @@ -270,6 +270,13 @@ binder_status_t AParcel_readStrongBinder(const AParcel* parcel, AIBinder** binde } sp ret = ABpBinder::lookupOrCreateFromBinder(readBinder); AIBinder_incStrong(ret.get()); + + if (ret.get() != nullptr && parcel->get()->isServiceFuzzing()) { + if (auto bp = ret->asABpBinder(); bp != nullptr) { + bp->setServiceFuzzing(); + } + } + *binder = ret.get(); return PruneStatusT(status); } -- cgit v1.2.3-59-g8ed1b