commit 35626658834a9b4edcb47a09ca8b7e33d65c2e10
author Steven Moreland <smoreland@google.com> Sat May 15 01:32:04 2021 +0000
committer Steven Moreland <smoreland@google.com> Tue May 18 00:33:22 2021 +0000
tree 12ffe0a02fd265a674c610a4ca221e022193cec0
parent c85de8c2e1d36bf8ee8414169086cc292e220989

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

libs/binder/IPCThreadState.cpp

diff --git a/libs/binder/IPCThreadState.cpp b/libs/binder/IPCThreadState.cpp
index ef7fd44..d2919e7 100644
--- a/libs/binder/IPCThreadState.cpp
+++ b/libs/binder/IPCThreadState.cpp
@@ -366,19 +366,46 @@
 
 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 @@
 }
 
 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 @@
                 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;