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

df2e017a4f492478137a27a566c560787017380e

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

3 files changed, 99 insertions, 10 deletions

diff --git a/libs/binder/IPCThreadState.cpp b/libs/binder/IPCThreadState.cpp
index ef7fd44419..d2919e7f4d 100644
--- a/libs/binder/IPCThreadState.cpp
+++ b/libs/binder/IPCThreadState.cpp
@@ -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
@@ -847,15 +874,15 @@ status_t IPCThreadState::clearDeathNotification(int32_t handle, BpBinder* proxy)
 }
 
 IPCThreadState::IPCThreadState()
-    : mProcess(ProcessState::self()),
-      mServingStackPointer(nullptr),
-      mWorkSource(kUnsetWorkSource),
-      mPropagateWorkSource(false),
-      mIsLooper(false),
-      mStrictModePolicy(0),
-      mLastTransactionBinderFlags(0),
-      mCallRestriction(mProcess->mCallRestriction)
-{
+      : mProcess(ProcessState::self()),
+        mServingStackPointer(nullptr),
+        mServingStackPointerGuard(nullptr),
+        mWorkSource(kUnsetWorkSource),
+        mPropagateWorkSource(false),
+        mIsLooper(false),
+        mStrictModePolicy(0),
+        mLastTransactionBinderFlags(0),
+        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;
diff --git a/libs/binder/include/binder/IPCThreadState.h b/libs/binder/include/binder/IPCThreadState.h
index 23a0cb0148..ee661a5948 100644
--- a/libs/binder/include/binder/IPCThreadState.h
+++ b/libs/binder/include/binder/IPCThreadState.h
@@ -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;
diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp
index 0c3fbcd2da..7679b46c4e 100644
--- a/libs/binder/tests/binderLibTest.cpp
+++ b/libs/binder/tests/binderLibTest.cpp
@@ -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;