Merge changes from topic "revert-1707789-SWNRVZMCZA"
* changes:
Revert "libbinder: introduce guards for getCalling*"
Revert "libbinder: binder RPC - using getCalling* aborts"
diff --git a/libs/binder/IPCThreadState.cpp b/libs/binder/IPCThreadState.cpp
index 18b77e6..ef7fd44 100644
--- a/libs/binder/IPCThreadState.cpp
+++ b/libs/binder/IPCThreadState.cpp
@@ -366,45 +366,19 @@
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;
}
-IPCThreadState::SpGuard* IPCThreadState::pushGetCallingSpGuard(SpGuard* guard) {
- SpGuard* orig = mServingStackPointerGuard;
- mServingStackPointerGuard = guard;
- return orig;
-}
-
-void IPCThreadState::restoreGetCallingSpGuard(SpGuard* guard) {
- mServingStackPointerGuard = guard;
-}
-
-void IPCThreadState::checkContextIsBinderForUse(const char* use) const {
- if (mServingStackPointerGuard == nullptr) return;
-
- if (!mServingStackPointer || mServingStackPointerGuard < mServingStackPointer) {
- LOG_ALWAYS_FATAL("In context %s, %s does not make sense.",
- mServingStackPointerGuard->context, use);
- }
-
- // 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
@@ -873,15 +847,15 @@
}
IPCThreadState::IPCThreadState()
- : mProcess(ProcessState::self()),
- mServingStackPointer(nullptr),
- mServingStackPointerGuard(nullptr),
- mWorkSource(kUnsetWorkSource),
- mPropagateWorkSource(false),
- mIsLooper(false),
- mStrictModePolicy(0),
- mLastTransactionBinderFlags(0),
- mCallRestriction(mProcess->mCallRestriction) {
+ : mProcess(ProcessState::self()),
+ mServingStackPointer(nullptr),
+ mWorkSource(kUnsetWorkSource),
+ mPropagateWorkSource(false),
+ mIsLooper(false),
+ mStrictModePolicy(0),
+ mLastTransactionBinderFlags(0),
+ mCallRestriction(mProcess->mCallRestriction)
+{
pthread_setspecific(gTLS, this);
clearCaller();
mIn.setDataCapacity(256);
diff --git a/libs/binder/RpcState.cpp b/libs/binder/RpcState.cpp
index e5a6026..2ba9fa2 100644
--- a/libs/binder/RpcState.cpp
+++ b/libs/binder/RpcState.cpp
@@ -18,9 +18,7 @@
#include "RpcState.h"
-#include <android-base/scopeguard.h>
#include <binder/BpBinder.h>
-#include <binder/IPCThreadState.h>
#include <binder/RpcServer.h>
#include "Debug.h"
@@ -30,8 +28,6 @@
namespace android {
-using base::ScopeGuard;
-
RpcState::RpcState() {}
RpcState::~RpcState() {}
@@ -474,18 +470,6 @@
status_t RpcState::processServerCommand(const base::unique_fd& fd, const sp<RpcSession>& session,
const RpcWireHeader& command) {
- IPCThreadState* kernelBinderState = IPCThreadState::selfOrNull();
- IPCThreadState::SpGuard spGuard{"processing binder RPC command"};
- IPCThreadState::SpGuard* origGuard;
- if (kernelBinderState != nullptr) {
- origGuard = kernelBinderState->pushGetCallingSpGuard(&spGuard);
- }
- ScopeGuard guardUnguard = [&]() {
- if (kernelBinderState != nullptr) {
- kernelBinderState->restoreGetCallingSpGuard(origGuard);
- }
- };
-
switch (command.command) {
case RPC_COMMAND_TRANSACT:
return processTransact(fd, session, command);
diff --git a/libs/binder/include/binder/IPCThreadState.h b/libs/binder/include/binder/IPCThreadState.h
index 5220b62..23a0cb0 100644
--- a/libs/binder/include/binder/IPCThreadState.h
+++ b/libs/binder/include/binder/IPCThreadState.h
@@ -81,32 +81,6 @@
*/
uid_t getCallingUid() const;
- /**
- * Make it an abort to rely on getCalling* for a section of
- * execution.
- *
- * Usage:
- * IPCThreadState::SpGuard guard { "..." };
- * auto* orig = pushGetCallingSpGuard(&guard);
- * {
- * // will abort if you call getCalling*, unless you are
- * // serving a nested binder transaction
- * }
- * restoreCallingSpGuard(orig);
- */
- struct SpGuard {
- const char* context;
- };
- SpGuard* pushGetCallingSpGuard(SpGuard* guard);
- void restoreGetCallingSpGuard(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;
@@ -229,7 +203,6 @@
Parcel mOut;
status_t mLastError;
const void* mServingStackPointer;
- SpGuard* mServingStackPointerGuard;
pid_t mCallingPid;
const char* mCallingSid;
uid_t mCallingUid;
diff --git a/libs/binder/tests/IBinderRpcTest.aidl b/libs/binder/tests/IBinderRpcTest.aidl
index 41daccc..ef4198d 100644
--- a/libs/binder/tests/IBinderRpcTest.aidl
+++ b/libs/binder/tests/IBinderRpcTest.aidl
@@ -55,6 +55,4 @@
oneway void sleepMsAsync(int ms);
void die(boolean cleanup);
-
- void useKernelBinderCallingId();
}
diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp
index 45b2776..0c3fbcd 100644
--- a/libs/binder/tests/binderLibTest.cpp
+++ b/libs/binder/tests/binderLibTest.cpp
@@ -73,7 +73,6 @@
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,
@@ -605,24 +604,6 @@
EXPECT_THAT(callBack->getResult(), StatusEq(NO_ERROR));
}
-TEST_F(BinderLibTest, NoBinderCallContextGuard) {
- IPCThreadState::SpGuard spGuard{"NoBinderCallContext"};
- IPCThreadState::SpGuard *origGuard = IPCThreadState::self()->pushGetCallingSpGuard(&spGuard);
-
- // yes, this test uses threads, but it's careful and uses fork in addServer
- EXPECT_DEATH({ IPCThreadState::self()->getCallingPid(); },
- "In context NoBinderCallContext, getCallingPid does not make sense.");
-
- IPCThreadState::self()->restoreGetCallingSpGuard(origGuard);
-}
-
-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();
@@ -1281,18 +1262,6 @@
pthread_mutex_unlock(&m_serverWaitMutex);
return ret;
}
- case BINDER_LIB_TEST_USE_CALLING_GUARD_TRANSACTION: {
- IPCThreadState::SpGuard spGuard{"GuardInBinderTransaction"};
- 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;
@@ -1520,11 +1489,6 @@
{
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{"main server thread"};
- (void)IPCThreadState::self()->pushGetCallingSpGuard(&spGuard);
-
status_t ret;
sp<IServiceManager> sm = defaultServiceManager();
BinderLibTestService* testServicePtr;
diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp
index 3f94df2..a96deb5 100644
--- a/libs/binder/tests/binderRpcTest.cpp
+++ b/libs/binder/tests/binderRpcTest.cpp
@@ -23,7 +23,6 @@
#include <android/binder_libbinder.h>
#include <binder/Binder.h>
#include <binder/BpBinder.h>
-#include <binder/IPCThreadState.h>
#include <binder/IServiceManager.h>
#include <binder/ProcessState.h>
#include <binder/RpcServer.h>
@@ -192,13 +191,6 @@
_exit(1);
}
}
- Status useKernelBinderCallingId() override {
- // this is WRONG! It does not make sense when using RPC binder, and
- // because it is SO wrong, and so much code calls this, it should abort!
-
- (void)IPCThreadState::self()->getCallingPid();
- return Status::ok();
- }
};
sp<IBinder> MyBinderRpcTest::mHeldBinder;
@@ -895,19 +887,6 @@
}
}
-TEST_P(BinderRpc, UseKernelBinderCallingId) {
- auto proc = createRpcTestSocketServerProcess(1);
-
- // we can't allocate IPCThreadState so actually the first time should
- // succeed :(
- EXPECT_OK(proc.rootIface->useKernelBinderCallingId());
-
- // second time! we catch the error :)
- EXPECT_EQ(DEAD_OBJECT, proc.rootIface->useKernelBinderCallingId().transactionError());
-
- proc.expectInvalid = true;
-}
-
TEST_P(BinderRpc, WorksWithLibbinderNdkPing) {
auto proc = createRpcTestSocketServerProcess(1);