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

Commit 418914a7 authored by Steven Moreland's avatar Steven Moreland
Browse files

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
parent d9154a7a

libs/binder/Parcel.cpp

+4 −0
Original line number Original line Diff line number Diff line
@@ -992,6 +992,10 @@ void Parcel::setServiceFuzzing() { 
    mServiceFuzzing = true;

    mServiceFuzzing = true;

}

}





bool Parcel::isServiceFuzzing() const {

    return mServiceFuzzing;

}



binder::Status Parcel::enforceNoDataAvail() const {

binder::Status Parcel::enforceNoDataAvail() const {

    if (!mEnforceNoDataAvail) {

    if (!mEnforceNoDataAvail) {

        return binder::Status::ok();

        return binder::Status::ok();

libs/binder/include/binder/Parcel.h

+1 −0
Original line number Original line Diff line number Diff line
@@ -152,6 +152,7 @@ public: 
    // When fuzzing, we want to remove certain ABI checks that cause significant

    // 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.

    // lost coverage, and we also want to avoid logs that cost too much to write.

    void setServiceFuzzing();

    void setServiceFuzzing();

    bool isServiceFuzzing() const;





    void                freeData();

    void                freeData();

libs/binder/ndk/ibinder.cpp

+1 −1
Original line number Original line Diff line number Diff line
@@ -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

    // 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

    // 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.

    // more flake-proof. However, the check is not dependent on the lock.

    if (descriptor != newDescriptor) {

    if (descriptor != newDescriptor && !(asABpBinder() && asABpBinder()->isServiceFuzzing())) {

        if (getBinder()->isBinderAlive()) {

        if (getBinder()->isBinderAlive()) {

            LOG(ERROR) << __func__ << ": Expecting binder to have class '" << newDescriptor

            LOG(ERROR) << __func__ << ": Expecting binder to have class '" << newDescriptor

                       << "' but descriptor is actually '" << SanitizeString(descriptor) << "'.";

                       << "' but descriptor is actually '" << SanitizeString(descriptor) << "'.";

libs/binder/ndk/ibinder_internal.h

+4 −0
Original line number Original line Diff line number Diff line
@@ -104,10 +104,14 @@ struct ABpBinder : public AIBinder { 
    ::android::sp<::android::IBinder> getBinder() override { return mRemote; }

    ::android::sp<::android::IBinder> getBinder() override { return mRemote; }

    ABpBinder* asABpBinder() override { return this; }

    ABpBinder* asABpBinder() override { return this; }





    bool isServiceFuzzing() const { return mServiceFuzzing; }

    void setServiceFuzzing() { mServiceFuzzing = true; }



   private:

   private:

    friend android::sp<ABpBinder>;

    friend android::sp<ABpBinder>;

    explicit ABpBinder(const ::android::sp<::android::IBinder>& binder);

    explicit ABpBinder(const ::android::sp<::android::IBinder>& binder);

    ::android::sp<::android::IBinder> mRemote;

    ::android::sp<::android::IBinder> mRemote;

    bool mServiceFuzzing = false;

};

};





struct AIBinder_Class {

struct AIBinder_Class {

libs/binder/ndk/parcel.cpp

+7 −0
Original line number Original line Diff line number Diff line
@@ -270,6 +270,13 @@ binder_status_t AParcel_readStrongBinder(const AParcel* parcel, AIBinder** binde 
    }

    }

    sp<AIBinder> ret = ABpBinder::lookupOrCreateFromBinder(readBinder);

    sp<AIBinder> ret = ABpBinder::lookupOrCreateFromBinder(readBinder);

    AIBinder_incStrong(ret.get());

    AIBinder_incStrong(ret.get());



    if (ret.get() != nullptr && parcel->get()->isServiceFuzzing()) {

        if (auto bp = ret->asABpBinder(); bp != nullptr) {

            bp->setServiceFuzzing();

        }

    }



    *binder = ret.get();

    *binder = ret.get();

    return PruneStatusT(status);

    return PruneStatusT(status);

}

}