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

Commit 35626658 authored by Steven Moreland's avatar Steven Moreland
Browse files

Revert^2 "libbinder: introduce guards for getCalling*"

df2e017a

Rein in the context! These "global" functions make sense sometimes, and
sometimes they make NO sense. Specifically, if you're going to make a
binder RPC call, you shouldn't be relying on getCalling* (silly!).

This is added as a generic mechanism in order that it might see more
use.

Bug: 186647790
Test: binderLibTest

Change-Id: Ic64bde4e8e0d6b203f58eef1ba09f2229ad2008b
parent c85de8c2
Loading
Loading
Loading
Loading
+37 −10
Original line number Diff line number Diff line
@@ -366,19 +366,46 @@ status_t IPCThreadState::clearLastError()

pid_t IPCThreadState::getCallingPid() const
{
    checkContextIsBinderForUse(__func__);
    return mCallingPid;
}

const char* IPCThreadState::getCallingSid() const
{
    checkContextIsBinderForUse(__func__);
    return mCallingSid;
}

uid_t IPCThreadState::getCallingUid() const
{
    checkContextIsBinderForUse(__func__);
    return mCallingUid;
}

const IPCThreadState::SpGuard* IPCThreadState::pushGetCallingSpGuard(const SpGuard* guard) {
    const SpGuard* orig = mServingStackPointerGuard;
    mServingStackPointerGuard = guard;
    return orig;
}

void IPCThreadState::restoreGetCallingSpGuard(const SpGuard* guard) {
    mServingStackPointerGuard = guard;
}

void IPCThreadState::checkContextIsBinderForUse(const char* use) const {
    if (LIKELY(mServingStackPointerGuard == nullptr)) return;

    if (!mServingStackPointer || mServingStackPointerGuard->address < mServingStackPointer) {
        LOG_ALWAYS_FATAL("In context %s, %s does not make sense (binder sp: %p, guard: %p).",
                         mServingStackPointerGuard->context, use, mServingStackPointer,
                         mServingStackPointerGuard->address);
    }

    // in the case mServingStackPointer is deeper in the stack than the guard,
    // we must be serving a binder transaction (maybe nested). This is a binder
    // context, so we don't abort
}

int64_t IPCThreadState::clearCallingIdentity()
{
    // ignore mCallingSid for legacy reasons
@@ -849,13 +876,13 @@ status_t IPCThreadState::clearDeathNotification(int32_t handle, BpBinder* proxy)
IPCThreadState::IPCThreadState()
      : mProcess(ProcessState::self()),
        mServingStackPointer(nullptr),
        mServingStackPointerGuard(nullptr),
        mWorkSource(kUnsetWorkSource),
        mPropagateWorkSource(false),
        mIsLooper(false),
        mStrictModePolicy(0),
        mLastTransactionBinderFlags(0),
      mCallRestriction(mProcess->mCallRestriction)
{
        mCallRestriction(mProcess->mCallRestriction) {
    pthread_setspecific(gTLS, this);
    clearCaller();
    mIn.setDataCapacity(256);
@@ -1230,7 +1257,7 @@ status_t IPCThreadState::executeCommand(int32_t cmd)
                tr.offsets_size/sizeof(binder_size_t), freeBuffer);

            const void* origServingStackPointer = mServingStackPointer;
            mServingStackPointer = &origServingStackPointer; // anything on the stack
            mServingStackPointer = __builtin_frame_address(0);

            const pid_t origPid = mCallingPid;
            const char* origSid = mCallingSid;
+31 −0
Original line number Diff line number Diff line
@@ -81,6 +81,36 @@ public:
             */
            uid_t               getCallingUid() const;

            /**
             * Make it an abort to rely on getCalling* for a section of
             * execution.
             *
             * Usage:
             *     IPCThreadState::SpGuard guard {
             *        .address = __builtin_frame_address(0),
             *        .context = "...",
             *     };
             *     const auto* orig = pushGetCallingSpGuard(&guard);
             *     {
             *         // will abort if you call getCalling*, unless you are
             *         // serving a nested binder transaction
             *     }
             *     restoreCallingSpGuard(orig);
             */
            struct SpGuard {
                const void* address;
                const char* context;
            };
            const SpGuard* pushGetCallingSpGuard(const SpGuard* guard);
            void restoreGetCallingSpGuard(const SpGuard* guard);
            /**
             * Used internally by getCalling*. Can also be used to assert that
             * you are in a binder context (getCalling* is valid). This is
             * intentionally not exposed as a boolean API since code should be
             * written to know its environment.
             */
            void checkContextIsBinderForUse(const char* use) const;

            void                setStrictModePolicy(int32_t policy);
            int32_t             getStrictModePolicy() const;

@@ -203,6 +233,7 @@ private:
            Parcel              mOut;
            status_t            mLastError;
            const void*         mServingStackPointer;
            const SpGuard* mServingStackPointerGuard;
            pid_t               mCallingPid;
            const char*         mCallingSid;
            uid_t               mCallingUid;
+31 −0
Original line number Diff line number Diff line
@@ -73,6 +73,7 @@ enum BinderLibTestTranscationCode {
    BINDER_LIB_TEST_REGISTER_SERVER,
    BINDER_LIB_TEST_ADD_SERVER,
    BINDER_LIB_TEST_ADD_POLL_SERVER,
    BINDER_LIB_TEST_USE_CALLING_GUARD_TRANSACTION,
    BINDER_LIB_TEST_CALL_BACK,
    BINDER_LIB_TEST_CALL_BACK_VERIFY_BUF,
    BINDER_LIB_TEST_DELAYED_CALL_BACK,
@@ -604,6 +605,13 @@ TEST_F(BinderLibTest, CallBack)
    EXPECT_THAT(callBack->getResult(), StatusEq(NO_ERROR));
}

TEST_F(BinderLibTest, BinderCallContextGuard) {
    sp<IBinder> binder = addServer();
    Parcel data, reply;
    EXPECT_THAT(binder->transact(BINDER_LIB_TEST_USE_CALLING_GUARD_TRANSACTION, data, &reply),
                StatusEq(DEAD_OBJECT));
}

TEST_F(BinderLibTest, AddServer)
{
    sp<IBinder> server = addServer();
@@ -1262,6 +1270,21 @@ class BinderLibTestService : public BBinder
                pthread_mutex_unlock(&m_serverWaitMutex);
                return ret;
            }
            case BINDER_LIB_TEST_USE_CALLING_GUARD_TRANSACTION: {
                IPCThreadState::SpGuard spGuard{
                        .address = __builtin_frame_address(0),
                        .context = "GuardInBinderTransaction",
                };
                const IPCThreadState::SpGuard *origGuard =
                        IPCThreadState::self()->pushGetCallingSpGuard(&spGuard);

                // if the guard works, this should abort
                (void)IPCThreadState::self()->getCallingPid();

                IPCThreadState::self()->restoreGetCallingSpGuard(origGuard);
                return NO_ERROR;
            }

            case BINDER_LIB_TEST_GETPID:
                reply->writeInt32(getpid());
                return NO_ERROR;
@@ -1489,6 +1512,14 @@ int run_server(int index, int readypipefd, bool usePoll)
{
    binderLibTestServiceName += String16(binderserversuffix);

    // Testing to make sure that calls that we are serving can use getCallin*
    // even though we don't here.
    IPCThreadState::SpGuard spGuard{
            .address = __builtin_frame_address(0),
            .context = "main server thread",
    };
    (void)IPCThreadState::self()->pushGetCallingSpGuard(&spGuard);

    status_t ret;
    sp<IServiceManager> sm = defaultServiceManager();
    BinderLibTestService* testServicePtr;