Merge "ANDROID: binder: prevent double flushing when free buffer"
diff --git a/libs/binder/RpcServer.cpp b/libs/binder/RpcServer.cpp
index e31aea0..73facc1 100644
--- a/libs/binder/RpcServer.cpp
+++ b/libs/binder/RpcServer.cpp
@@ -16,14 +16,12 @@
#define LOG_TAG "RpcServer"
-#include <poll.h>
#include <sys/socket.h>
#include <sys/un.h>
#include <thread>
#include <vector>
-#include <android-base/macros.h>
#include <android-base/scopeguard.h>
#include <binder/Parcel.h>
#include <binder/RpcServer.h>
@@ -39,7 +37,9 @@
using base::unique_fd;
RpcServer::RpcServer() {}
-RpcServer::~RpcServer() {}
+RpcServer::~RpcServer() {
+ (void)shutdown();
+}
sp<RpcServer> RpcServer::make() {
return sp<RpcServer>::make();
@@ -128,16 +128,6 @@
return ret;
}
-std::unique_ptr<RpcServer::FdTrigger> RpcServer::FdTrigger::make() {
- auto ret = std::make_unique<RpcServer::FdTrigger>();
- if (!android::base::Pipe(&ret->mRead, &ret->mWrite)) return nullptr;
- return ret;
-}
-
-void RpcServer::FdTrigger::trigger() {
- mWrite.reset();
-}
-
void RpcServer::join() {
LOG_ALWAYS_FATAL_IF(!mAgreedExperimental, "no!");
@@ -146,27 +136,12 @@
LOG_ALWAYS_FATAL_IF(!mServer.ok(), "RpcServer must be setup to join.");
LOG_ALWAYS_FATAL_IF(mShutdownTrigger != nullptr, "Already joined");
mJoinThreadRunning = true;
- mShutdownTrigger = FdTrigger::make();
+ mShutdownTrigger = RpcSession::FdTrigger::make();
LOG_ALWAYS_FATAL_IF(mShutdownTrigger == nullptr, "Cannot create join signaler");
}
- while (true) {
- pollfd pfd[]{{.fd = mServer.get(), .events = POLLIN, .revents = 0},
- {.fd = mShutdownTrigger->readFd().get(), .events = POLLHUP, .revents = 0}};
- int ret = TEMP_FAILURE_RETRY(poll(pfd, arraysize(pfd), -1));
- if (ret < 0) {
- ALOGE("Could not poll socket: %s", strerror(errno));
- continue;
- }
- if (ret == 0) {
- continue;
- }
- if (pfd[1].revents & POLLHUP) {
- LOG_RPC_DETAIL("join() exiting because shutdown requested.");
- break;
- }
-
- (void)acceptOneNoCheck();
+ while (mShutdownTrigger->triggerablePollRead(mServer)) {
+ (void)acceptOne();
}
{
@@ -177,12 +152,6 @@
}
bool RpcServer::acceptOne() {
- LOG_ALWAYS_FATAL_IF(!mAgreedExperimental, "no!");
- LOG_ALWAYS_FATAL_IF(!hasServer(), "RpcServer must be setup to acceptOne.");
- return acceptOneNoCheck();
-}
-
-bool RpcServer::acceptOneNoCheck() {
unique_fd clientFd(
TEMP_FAILURE_RETRY(accept4(mServer.get(), nullptr, nullptr /*length*/, SOCK_CLOEXEC)));
@@ -194,9 +163,8 @@
{
std::lock_guard<std::mutex> _l(mLock);
- std::thread thread =
- std::thread(&RpcServer::establishConnection, this,
- std::move(sp<RpcServer>::fromExisting(this)), std::move(clientFd));
+ std::thread thread = std::thread(&RpcServer::establishConnection,
+ sp<RpcServer>::fromExisting(this), std::move(clientFd));
mConnectingThreads[thread.get_id()] = std::move(thread);
}
@@ -204,7 +172,6 @@
}
bool RpcServer::shutdown() {
- LOG_ALWAYS_FATAL_IF(!mAgreedExperimental, "no!");
std::unique_lock<std::mutex> _l(mLock);
if (mShutdownTrigger == nullptr) return false;
@@ -231,10 +198,8 @@
}
void RpcServer::establishConnection(sp<RpcServer>&& server, base::unique_fd clientFd) {
- LOG_ALWAYS_FATAL_IF(this != server.get(), "Must pass same ownership object");
-
// TODO(b/183988761): cannot trust this simple ID
- LOG_ALWAYS_FATAL_IF(!mAgreedExperimental, "no!");
+ LOG_ALWAYS_FATAL_IF(!server->mAgreedExperimental, "no!");
bool idValid = true;
int32_t id;
if (sizeof(id) != read(clientFd.get(), &id, sizeof(id))) {
@@ -245,30 +210,30 @@
std::thread thisThread;
sp<RpcSession> session;
{
- std::lock_guard<std::mutex> _l(mLock);
+ std::lock_guard<std::mutex> _l(server->mLock);
- auto threadId = mConnectingThreads.find(std::this_thread::get_id());
- LOG_ALWAYS_FATAL_IF(threadId == mConnectingThreads.end(),
+ auto threadId = server->mConnectingThreads.find(std::this_thread::get_id());
+ LOG_ALWAYS_FATAL_IF(threadId == server->mConnectingThreads.end(),
"Must establish connection on owned thread");
thisThread = std::move(threadId->second);
ScopeGuard detachGuard = [&]() { thisThread.detach(); };
- mConnectingThreads.erase(threadId);
+ server->mConnectingThreads.erase(threadId);
if (!idValid) {
return;
}
if (id == RPC_SESSION_ID_NEW) {
- LOG_ALWAYS_FATAL_IF(mSessionIdCounter >= INT32_MAX, "Out of session IDs");
- mSessionIdCounter++;
+ LOG_ALWAYS_FATAL_IF(server->mSessionIdCounter >= INT32_MAX, "Out of session IDs");
+ server->mSessionIdCounter++;
session = RpcSession::make();
- session->setForServer(wp<RpcServer>::fromExisting(this), mSessionIdCounter);
+ session->setForServer(wp<RpcServer>(server), server->mSessionIdCounter);
- mSessions[mSessionIdCounter] = session;
+ server->mSessions[server->mSessionIdCounter] = session;
} else {
- auto it = mSessions.find(id);
- if (it == mSessions.end()) {
+ auto it = server->mSessions.find(id);
+ if (it == server->mSessions.end()) {
ALOGE("Cannot add thread, no record of session with ID %d", id);
return;
}
@@ -281,10 +246,6 @@
// avoid strong cycle
server = nullptr;
- //
- //
- // DO NOT ACCESS MEMBER VARIABLES BELOW
- //
session->join(std::move(clientFd));
}
diff --git a/libs/binder/RpcSession.cpp b/libs/binder/RpcSession.cpp
index 05fa49e..ea82f36 100644
--- a/libs/binder/RpcSession.cpp
+++ b/libs/binder/RpcSession.cpp
@@ -19,10 +19,12 @@
#include <binder/RpcSession.h>
#include <inttypes.h>
+#include <poll.h>
#include <unistd.h>
#include <string_view>
+#include <android-base/macros.h>
#include <binder/Parcel.h>
#include <binder/RpcServer.h>
#include <binder/Stability.h>
@@ -113,6 +115,35 @@
return state()->sendDecStrong(connection.fd(), address);
}
+std::unique_ptr<RpcSession::FdTrigger> RpcSession::FdTrigger::make() {
+ auto ret = std::make_unique<RpcSession::FdTrigger>();
+ if (!android::base::Pipe(&ret->mRead, &ret->mWrite)) return nullptr;
+ return ret;
+}
+
+void RpcSession::FdTrigger::trigger() {
+ mWrite.reset();
+}
+
+bool RpcSession::FdTrigger::triggerablePollRead(base::borrowed_fd fd) {
+ while (true) {
+ pollfd pfd[]{{.fd = fd.get(), .events = POLLIN, .revents = 0},
+ {.fd = mRead.get(), .events = POLLHUP, .revents = 0}};
+ int ret = TEMP_FAILURE_RETRY(poll(pfd, arraysize(pfd), -1));
+ if (ret < 0) {
+ ALOGE("Could not poll: %s", strerror(errno));
+ continue;
+ }
+ if (ret == 0) {
+ continue;
+ }
+ if (pfd[1].revents & POLLHUP) {
+ return false;
+ }
+ return true;
+ }
+}
+
status_t RpcSession::readId() {
{
std::lock_guard<std::mutex> _l(mMutex);
diff --git a/libs/binder/RpcState.cpp b/libs/binder/RpcState.cpp
index 2ba9fa2..230de6f 100644
--- a/libs/binder/RpcState.cpp
+++ b/libs/binder/RpcState.cpp
@@ -18,7 +18,9 @@
#include "RpcState.h"
+#include <android-base/scopeguard.h>
#include <binder/BpBinder.h>
+#include <binder/IPCThreadState.h>
#include <binder/RpcServer.h>
#include "Debug.h"
@@ -28,6 +30,8 @@
namespace android {
+using base::ScopeGuard;
+
RpcState::RpcState() {}
RpcState::~RpcState() {}
@@ -470,6 +474,21 @@
status_t RpcState::processServerCommand(const base::unique_fd& fd, const sp<RpcSession>& session,
const RpcWireHeader& command) {
+ IPCThreadState* kernelBinderState = IPCThreadState::selfOrNull();
+ IPCThreadState::SpGuard spGuard{
+ .address = __builtin_frame_address(0),
+ .context = "processing binder RPC command",
+ };
+ const 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/ProcessState.h b/libs/binder/include/binder/ProcessState.h
index b9db5d7..b992c04 100644
--- a/libs/binder/include/binder/ProcessState.h
+++ b/libs/binder/include/binder/ProcessState.h
@@ -18,8 +18,8 @@
#include <binder/IBinder.h>
#include <utils/KeyedVector.h>
-#include <utils/String8.h>
#include <utils/String16.h>
+#include <utils/String8.h>
#include <utils/threads.h>
@@ -30,11 +30,10 @@
class IPCThreadState;
-class ProcessState : public virtual RefBase
-{
+class ProcessState : public virtual RefBase {
public:
- static sp<ProcessState> self();
- static sp<ProcessState> selfOrNull();
+ static sp<ProcessState> self();
+ static sp<ProcessState> selfOrNull();
/* initWithDriver() can be used to configure libbinder to use
* a different binder driver dev node. It must be called *before*
@@ -44,94 +43,94 @@
*
* If this is called with nullptr, the behavior is the same as selfOrNull.
*/
- static sp<ProcessState> initWithDriver(const char *driver);
+ static sp<ProcessState> initWithDriver(const char* driver);
- sp<IBinder> getContextObject(const sp<IBinder>& caller);
+ sp<IBinder> getContextObject(const sp<IBinder>& caller);
- void startThreadPool();
+ void startThreadPool();
- bool becomeContextManager();
+ bool becomeContextManager();
- sp<IBinder> getStrongProxyForHandle(int32_t handle);
- void expungeHandle(int32_t handle, IBinder* binder);
+ sp<IBinder> getStrongProxyForHandle(int32_t handle);
+ void expungeHandle(int32_t handle, IBinder* binder);
- void spawnPooledThread(bool isMain);
-
- status_t setThreadPoolMaxThreadCount(size_t maxThreads);
- status_t enableOnewaySpamDetection(bool enable);
- void giveThreadPoolName();
+ void spawnPooledThread(bool isMain);
- String8 getDriverName();
+ status_t setThreadPoolMaxThreadCount(size_t maxThreads);
+ status_t enableOnewaySpamDetection(bool enable);
+ void giveThreadPoolName();
- ssize_t getKernelReferences(size_t count, uintptr_t* buf);
+ String8 getDriverName();
- // Only usable by the context manager.
- // This refcount includes:
- // 1. Strong references to the node by this and other processes
- // 2. Temporary strong references held by the kernel during a
- // transaction on the node.
- // It does NOT include local strong references to the node
- ssize_t getStrongRefCountForNode(const sp<BpBinder>& binder);
+ ssize_t getKernelReferences(size_t count, uintptr_t* buf);
- enum class CallRestriction {
- // all calls okay
- NONE,
- // log when calls are blocking
- ERROR_IF_NOT_ONEWAY,
- // abort process on blocking calls
- FATAL_IF_NOT_ONEWAY,
- };
- // Sets calling restrictions for all transactions in this process. This must be called
- // before any threads are spawned.
- void setCallRestriction(CallRestriction restriction);
+ // Only usable by the context manager.
+ // This refcount includes:
+ // 1. Strong references to the node by this and other processes
+ // 2. Temporary strong references held by the kernel during a
+ // transaction on the node.
+ // It does NOT include local strong references to the node
+ ssize_t getStrongRefCountForNode(const sp<BpBinder>& binder);
+
+ enum class CallRestriction {
+ // all calls okay
+ NONE,
+ // log when calls are blocking
+ ERROR_IF_NOT_ONEWAY,
+ // abort process on blocking calls
+ FATAL_IF_NOT_ONEWAY,
+ };
+ // Sets calling restrictions for all transactions in this process. This must be called
+ // before any threads are spawned.
+ void setCallRestriction(CallRestriction restriction);
private:
- static sp<ProcessState> init(const char *defaultDriver, bool requireDefault);
+ static sp<ProcessState> init(const char* defaultDriver, bool requireDefault);
friend class IPCThreadState;
friend class sp<ProcessState>;
- explicit ProcessState(const char* driver);
- ~ProcessState();
+ explicit ProcessState(const char* driver);
+ ~ProcessState();
- ProcessState(const ProcessState& o);
- ProcessState& operator=(const ProcessState& o);
- String8 makeBinderThreadName();
+ ProcessState(const ProcessState& o);
+ ProcessState& operator=(const ProcessState& o);
+ String8 makeBinderThreadName();
- struct handle_entry {
- IBinder* binder;
- RefBase::weakref_type* refs;
- };
+ struct handle_entry {
+ IBinder* binder;
+ RefBase::weakref_type* refs;
+ };
- handle_entry* lookupHandleLocked(int32_t handle);
+ handle_entry* lookupHandleLocked(int32_t handle);
- String8 mDriverName;
- int mDriverFD;
- void* mVMStart;
+ String8 mDriverName;
+ int mDriverFD;
+ void* mVMStart;
- // Protects thread count and wait variables below.
- pthread_mutex_t mThreadCountLock;
- // Broadcast whenever mWaitingForThreads > 0
- pthread_cond_t mThreadCountDecrement;
- // Number of binder threads current executing a command.
- size_t mExecutingThreadsCount;
- // Number of threads calling IPCThreadState::blockUntilThreadAvailable()
- size_t mWaitingForThreads;
- // Maximum number for binder threads allowed for this process.
- size_t mMaxThreads;
- // Time when thread pool was emptied
- int64_t mStarvationStartTimeMs;
+ // Protects thread count and wait variables below.
+ pthread_mutex_t mThreadCountLock;
+ // Broadcast whenever mWaitingForThreads > 0
+ pthread_cond_t mThreadCountDecrement;
+ // Number of binder threads current executing a command.
+ size_t mExecutingThreadsCount;
+ // Number of threads calling IPCThreadState::blockUntilThreadAvailable()
+ size_t mWaitingForThreads;
+ // Maximum number for binder threads allowed for this process.
+ size_t mMaxThreads;
+ // Time when thread pool was emptied
+ int64_t mStarvationStartTimeMs;
- mutable Mutex mLock; // protects everything below.
+ mutable Mutex mLock; // protects everything below.
- Vector<handle_entry>mHandleToObject;
+ Vector<handle_entry> mHandleToObject;
- bool mThreadPoolStarted;
- volatile int32_t mThreadPoolSeq;
+ bool mThreadPoolStarted;
+ volatile int32_t mThreadPoolSeq;
- CallRestriction mCallRestriction;
+ CallRestriction mCallRestriction;
};
-
+
} // namespace android
// ---------------------------------------------------------------------------
diff --git a/libs/binder/include/binder/RpcServer.h b/libs/binder/include/binder/RpcServer.h
index 4973400..f76ecc2 100644
--- a/libs/binder/include/binder/RpcServer.h
+++ b/libs/binder/include/binder/RpcServer.h
@@ -135,12 +135,6 @@
[[nodiscard]] bool shutdown();
/**
- * Accept one connection on this server. You must have at least one client
- * session before calling this.
- */
- [[nodiscard]] bool acceptOne();
-
- /**
* For debugging!
*/
std::vector<sp<RpcSession>> listSessions();
@@ -153,29 +147,12 @@
void onSessionTerminating(const sp<RpcSession>& session);
private:
- /** This is not a pipe. */
- struct FdTrigger {
- static std::unique_ptr<FdTrigger> make();
- /**
- * poll() on this fd for POLLHUP to get notification when trigger is called
- */
- base::borrowed_fd readFd() const { return mRead; }
- /**
- * Close the write end of the pipe so that the read end receives POLLHUP.
- */
- void trigger();
-
- private:
- base::unique_fd mWrite;
- base::unique_fd mRead;
- };
-
friend sp<RpcServer>;
RpcServer();
- void establishConnection(sp<RpcServer>&& session, base::unique_fd clientFd);
+ static void establishConnection(sp<RpcServer>&& server, base::unique_fd clientFd);
bool setupSocketServer(const RpcSocketAddress& address);
- [[nodiscard]] bool acceptOneNoCheck();
+ [[nodiscard]] bool acceptOne();
bool mAgreedExperimental = false;
size_t mMaxThreads = 1;
@@ -188,7 +165,7 @@
std::map<int32_t, sp<RpcSession>> mSessions;
int32_t mSessionIdCounter = 0;
bool mJoinThreadRunning = false;
- std::unique_ptr<FdTrigger> mShutdownTrigger;
+ std::unique_ptr<RpcSession::FdTrigger> mShutdownTrigger;
std::condition_variable mShutdownCv;
};
diff --git a/libs/binder/include/binder/RpcSession.h b/libs/binder/include/binder/RpcSession.h
index bcc213c..bcef8dc 100644
--- a/libs/binder/include/binder/RpcSession.h
+++ b/libs/binder/include/binder/RpcSession.h
@@ -112,6 +112,33 @@
friend RpcServer;
RpcSession();
+ /** This is not a pipe. */
+ struct FdTrigger {
+ static std::unique_ptr<FdTrigger> make();
+ /**
+ * poll() on this fd for POLLHUP to get notification when trigger is called
+ */
+ base::borrowed_fd readFd() const { return mRead; }
+
+ /**
+ * Close the write end of the pipe so that the read end receives POLLHUP.
+ */
+ void trigger();
+
+ /**
+ * Poll for a read event.
+ *
+ * Return:
+ * true - time to read!
+ * false - trigger happened
+ */
+ bool triggerablePollRead(base::borrowed_fd fd);
+
+ private:
+ base::unique_fd mWrite;
+ base::unique_fd mRead;
+ };
+
status_t readId();
// transfer ownership of thread
diff --git a/libs/binder/tests/IBinderRpcTest.aidl b/libs/binder/tests/IBinderRpcTest.aidl
index ef4198d..41daccc 100644
--- a/libs/binder/tests/IBinderRpcTest.aidl
+++ b/libs/binder/tests/IBinderRpcTest.aidl
@@ -55,4 +55,6 @@
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 7679b46..963d7b3 100644
--- a/libs/binder/tests/binderLibTest.cpp
+++ b/libs/binder/tests/binderLibTest.cpp
@@ -1173,39 +1173,30 @@
EXPECT_THAT(server->transact(BINDER_LIB_TEST_CAN_GET_SID, data, nullptr), StatusEq(OK));
}
-class BinderLibTestService : public BBinder
-{
- public:
- explicit BinderLibTestService(int32_t id)
- : m_id(id)
- , m_nextServerId(id + 1)
- , m_serverStartRequested(false)
- , m_callback(nullptr)
- {
- pthread_mutex_init(&m_serverWaitMutex, nullptr);
- pthread_cond_init(&m_serverWaitCond, nullptr);
- }
- ~BinderLibTestService()
- {
- exit(EXIT_SUCCESS);
- }
+class BinderLibTestService : public BBinder {
+public:
+ explicit BinderLibTestService(int32_t id)
+ : m_id(id), m_nextServerId(id + 1), m_serverStartRequested(false), m_callback(nullptr) {
+ pthread_mutex_init(&m_serverWaitMutex, nullptr);
+ pthread_cond_init(&m_serverWaitCond, nullptr);
+ }
+ ~BinderLibTestService() { exit(EXIT_SUCCESS); }
- void processPendingCall() {
- if (m_callback != nullptr) {
- Parcel data;
- data.writeInt32(NO_ERROR);
- m_callback->transact(BINDER_LIB_TEST_CALL_BACK, data, nullptr, TF_ONE_WAY);
- m_callback = nullptr;
- }
+ void processPendingCall() {
+ if (m_callback != nullptr) {
+ Parcel data;
+ data.writeInt32(NO_ERROR);
+ m_callback->transact(BINDER_LIB_TEST_CALL_BACK, data, nullptr, TF_ONE_WAY);
+ m_callback = nullptr;
}
+ }
- virtual status_t onTransact(uint32_t code,
- const Parcel& data, Parcel* reply,
- uint32_t flags = 0) {
- if (getuid() != (uid_t)IPCThreadState::self()->getCallingUid()) {
- return PERMISSION_DENIED;
- }
- switch (code) {
+ virtual status_t onTransact(uint32_t code, const Parcel &data, Parcel *reply,
+ uint32_t flags = 0) {
+ if (getuid() != (uid_t)IPCThreadState::self()->getCallingUid()) {
+ return PERMISSION_DENIED;
+ }
+ switch (code) {
case BINDER_LIB_TEST_REGISTER_SERVER: {
int32_t id;
sp<IBinder> binder;
@@ -1215,8 +1206,7 @@
return BAD_VALUE;
}
- if (m_id != 0)
- return INVALID_OPERATION;
+ if (m_id != 0) return INVALID_OPERATION;
pthread_mutex_lock(&m_serverWaitMutex);
if (m_serverStartRequested) {
@@ -1401,8 +1391,7 @@
return BAD_VALUE;
}
ret = target->linkToDeath(testDeathRecipient);
- if (ret == NO_ERROR)
- ret = testDeathRecipient->waitEvent(5);
+ if (ret == NO_ERROR) ret = testDeathRecipient->waitEvent(5);
data2.writeInt32(ret);
callback->transact(BINDER_LIB_TEST_CALL_BACK, data2, &reply2);
return NO_ERROR;
@@ -1426,8 +1415,7 @@
return BAD_VALUE;
}
ret = write(fd, buf, size);
- if (ret != size)
- return UNKNOWN_ERROR;
+ if (ret != size) return UNKNOWN_ERROR;
return NO_ERROR;
}
case BINDER_LIB_TEST_WRITE_PARCEL_FILE_DESCRIPTOR_TRANSACTION: {
@@ -1482,8 +1470,7 @@
case BINDER_LIB_TEST_ECHO_VECTOR: {
std::vector<uint64_t> vector;
auto err = data.readUint64Vector(&vector);
- if (err != NO_ERROR)
- return err;
+ if (err != NO_ERROR) return err;
reply->writeUint64Vector(vector);
return NO_ERROR;
}
@@ -1495,17 +1482,18 @@
}
default:
return UNKNOWN_TRANSACTION;
- };
- }
- private:
- int32_t m_id;
- int32_t m_nextServerId;
- pthread_mutex_t m_serverWaitMutex;
- pthread_cond_t m_serverWaitCond;
- bool m_serverStartRequested;
- sp<IBinder> m_serverStarted;
- sp<IBinder> m_strongRef;
- sp<IBinder> m_callback;
+ };
+ }
+
+private:
+ int32_t m_id;
+ int32_t m_nextServerId;
+ pthread_mutex_t m_serverWaitMutex;
+ pthread_cond_t m_serverWaitCond;
+ bool m_serverStartRequested;
+ sp<IBinder> m_serverStarted;
+ sp<IBinder> m_strongRef;
+ sp<IBinder> m_callback;
};
int run_server(int index, int readypipefd, bool usePoll)
diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp
index fb0ffdb..e10fe2f 100644
--- a/libs/binder/tests/binderRpcTest.cpp
+++ b/libs/binder/tests/binderRpcTest.cpp
@@ -23,6 +23,7 @@
#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>
@@ -193,6 +194,13 @@
_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;
@@ -889,6 +897,19 @@
}
}
+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);
diff --git a/libs/binder/tests/parcel_fuzzer/main.cpp b/libs/binder/tests/parcel_fuzzer/main.cpp
index f426fd3..2a79e85 100644
--- a/libs/binder/tests/parcel_fuzzer/main.cpp
+++ b/libs/binder/tests/parcel_fuzzer/main.cpp
@@ -23,7 +23,8 @@
#include <iostream>
#include <android-base/logging.h>
-#include <binder/RpcSession.h>
+#include <android/binder_auto_utils.h>
+#include <android/binder_libbinder.h>
#include <fuzzbinder/random_parcel.h>
#include <fuzzer/FuzzedDataProvider.h>
@@ -33,7 +34,6 @@
#include <sys/time.h>
using android::fillRandomParcel;
-using android::RpcSession;
using android::sp;
void fillRandomParcel(::android::hardware::Parcel* p, FuzzedDataProvider&& provider) {
@@ -46,9 +46,22 @@
fillRandomParcel(p->parcel(), std::move(provider));
}
+template <typename P, typename B>
+void doTransactFuzz(const char* backend, const sp<B>& binder, FuzzedDataProvider&& provider) {
+ uint32_t code = provider.ConsumeIntegral<uint32_t>();
+ uint32_t flag = provider.ConsumeIntegral<uint32_t>();
+
+ FUZZ_LOG() << "backend: " << backend;
+
+ P reply;
+ P data;
+ fillRandomParcel(&data, std::move(provider));
+ (void)binder->transact(code, data, &reply, flag);
+}
+
template <typename P>
-void doFuzz(const char* backend, const std::vector<ParcelRead<P>>& reads,
- FuzzedDataProvider&& provider) {
+void doReadFuzz(const char* backend, const std::vector<ParcelRead<P>>& reads,
+ FuzzedDataProvider&& provider) {
// Allow some majority of the bytes to be dedicated to telling us what to
// do. The fixed value added here represents that we want to test doing a
// lot of 'instructions' even on really short parcels.
@@ -59,18 +72,7 @@
provider.ConsumeIntegralInRange<size_t>(0, maxInstructions));
P p;
- if constexpr (std::is_same_v<P, android::Parcel>) {
- if (provider.ConsumeBool()) {
- auto session = sp<RpcSession>::make();
- CHECK(session->addNullDebuggingClient());
- p.markForRpc(session);
- fillRandomParcelData(&p, std::move(provider));
- } else {
- fillRandomParcel(&p, std::move(provider));
- }
- } else {
- fillRandomParcel(&p, std::move(provider));
- }
+ fillRandomParcel(&p, std::move(provider));
// since we are only using a byte to index
CHECK(reads.size() <= 255) << reads.size();
@@ -95,6 +97,17 @@
}
}
+void* NothingClass_onCreate(void* args) {
+ return args;
+}
+void NothingClass_onDestroy(void* /*userData*/) {}
+binder_status_t NothingClass_onTransact(AIBinder*, transaction_code_t, const AParcel*, AParcel*) {
+ return STATUS_UNKNOWN_ERROR;
+}
+static AIBinder_Class* kNothingClass =
+ AIBinder_Class_define("nothing", NothingClass_onCreate, NothingClass_onDestroy,
+ NothingClass_onTransact);
+
extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
if (size <= 1) return 0; // no use
@@ -103,18 +116,35 @@
FuzzedDataProvider provider = FuzzedDataProvider(data, size);
- const std::function<void(FuzzedDataProvider &&)> fuzzBackend[3] = {
+ const std::function<void(FuzzedDataProvider &&)> fuzzBackend[] = {
[](FuzzedDataProvider&& provider) {
- doFuzz<::android::hardware::Parcel>("hwbinder", HWBINDER_PARCEL_READ_FUNCTIONS,
- std::move(provider));
+ doTransactFuzz<
+ ::android::hardware::Parcel>("hwbinder",
+ sp<::android::hardware::BHwBinder>::make(),
+ std::move(provider));
},
[](FuzzedDataProvider&& provider) {
- doFuzz<::android::Parcel>("binder", BINDER_PARCEL_READ_FUNCTIONS,
- std::move(provider));
+ doTransactFuzz<::android::Parcel>("binder", sp<::android::BBinder>::make(),
+ std::move(provider));
},
[](FuzzedDataProvider&& provider) {
- doFuzz<NdkParcelAdapter>("binder_ndk", BINDER_NDK_PARCEL_READ_FUNCTIONS,
- std::move(provider));
+ // fuzz from the libbinder layer since it's a superset of the
+ // interface you get at the libbinder_ndk layer
+ auto ndkBinder = ndk::SpAIBinder(AIBinder_new(kNothingClass, nullptr));
+ auto binder = AIBinder_toPlatformBinder(ndkBinder.get());
+ doTransactFuzz<::android::Parcel>("binder_ndk", binder, std::move(provider));
+ },
+ [](FuzzedDataProvider&& provider) {
+ doReadFuzz<::android::hardware::Parcel>("hwbinder", HWBINDER_PARCEL_READ_FUNCTIONS,
+ std::move(provider));
+ },
+ [](FuzzedDataProvider&& provider) {
+ doReadFuzz<::android::Parcel>("binder", BINDER_PARCEL_READ_FUNCTIONS,
+ std::move(provider));
+ },
+ [](FuzzedDataProvider&& provider) {
+ doReadFuzz<NdkParcelAdapter>("binder_ndk", BINDER_NDK_PARCEL_READ_FUNCTIONS,
+ std::move(provider));
},
};
diff --git a/libs/binder/tests/parcel_fuzzer/random_parcel.cpp b/libs/binder/tests/parcel_fuzzer/random_parcel.cpp
index b045a22..92fdc72 100644
--- a/libs/binder/tests/parcel_fuzzer/random_parcel.cpp
+++ b/libs/binder/tests/parcel_fuzzer/random_parcel.cpp
@@ -18,6 +18,7 @@
#include <android-base/logging.h>
#include <binder/IServiceManager.h>
+#include <binder/RpcSession.h>
#include <fuzzbinder/random_fd.h>
#include <utils/String16.h>
@@ -33,6 +34,14 @@
};
void fillRandomParcel(Parcel* p, FuzzedDataProvider&& provider) {
+ if (provider.ConsumeBool()) {
+ auto session = sp<RpcSession>::make();
+ CHECK(session->addNullDebuggingClient());
+ p->markForRpc(session);
+ fillRandomParcelData(p, std::move(provider));
+ return;
+ }
+
while (provider.remaining_bytes() > 0) {
auto fillFunc = provider.PickValueInArray<const std::function<void()>>({
// write data
diff --git a/libs/binder/tests/rpc_fuzzer/main.cpp b/libs/binder/tests/rpc_fuzzer/main.cpp
index 8a12aea..84f5974 100644
--- a/libs/binder/tests/rpc_fuzzer/main.cpp
+++ b/libs/binder/tests/rpc_fuzzer/main.cpp
@@ -61,7 +61,7 @@
server->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction();
CHECK(server->setupUnixDomainServer(kSock.c_str()));
- std::thread serverThread([=] { (void)server->acceptOne(); });
+ std::thread serverThread([=] { (void)server->join(); });
sockaddr_un addr{
.sun_family = AF_UNIX,
@@ -76,8 +76,6 @@
connect(clientFd.get(), reinterpret_cast<sockaddr*>(&addr), sizeof(addr))))
<< strerror(errno);
- serverThread.join();
-
// TODO(b/182938024): fuzz multiple sessions, instead of just one
#if 0
@@ -90,6 +88,12 @@
clientFd.reset();
+ // TODO(185167543): currently this is okay because we only shutdown the one
+ // thread, but once we can shutdown other sessions, we'll need to change
+ // this behavior in order to make sure all of the input is actually read.
+ while (!server->shutdown()) usleep(100);
+ serverThread.join();
+
// TODO(b/185167543): better way to force a server to shutdown
while (!server->listSessions().empty() && server->numUninitializedSessions()) {
usleep(1);