libbinder: Add binder already sent checks (d67c8e89) · Commits · e / os / platform_frameworks_native

libs/binder/Binder.cpp

+25 −3

Original line number	Original line	Diff line number	Diff line
	@@ -197,9 +197,7 @@ public:

	// ---------------------------------------------------------------------------		// ---------------------------------------------------------------------------

	BBinder::BBinder() : mExtras(nullptr), mStability(0)		BBinder::BBinder() : mExtras(nullptr), mStability(0), mParceled(false) {}
	{
	}

	bool BBinder::isBinderAlive() const		bool BBinder::isBinderAlive() const
	{		{
	@@ -322,6 +320,10 @@ bool BBinder::isRequestingSid()

	void BBinder::setRequestingSid(bool requestingSid)		void BBinder::setRequestingSid(bool requestingSid)
	{		{
			ALOGW_IF(mParceled,
			"setRequestingSid() should not be called after a binder object "
			"is parceled/sent to another process");

	Extras* e = mExtras.load(std::memory_order_acquire);		Extras* e = mExtras.load(std::memory_order_acquire);

	if (!e) {		if (!e) {
	@@ -344,6 +346,10 @@ sp<IBinder> BBinder::getExtension() {
	}		}

	void BBinder::setMinSchedulerPolicy(int policy, int priority) {		void BBinder::setMinSchedulerPolicy(int policy, int priority) {
			ALOGW_IF(mParceled,
			"setMinSchedulerPolicy() should not be called after a binder object "
			"is parceled/sent to another process");

	switch (policy) {		switch (policy) {
	case SCHED_NORMAL:		case SCHED_NORMAL:
	LOG_ALWAYS_FATAL_IF(priority < -20 \|\| priority > 19, "Invalid priority for SCHED_NORMAL: %d", priority);		LOG_ALWAYS_FATAL_IF(priority < -20 \|\| priority > 19, "Invalid priority for SCHED_NORMAL: %d", priority);
	@@ -391,6 +397,10 @@ bool BBinder::isInheritRt() {
	}		}

	void BBinder::setInheritRt(bool inheritRt) {		void BBinder::setInheritRt(bool inheritRt) {
			ALOGW_IF(mParceled,
			"setInheritRt() should not be called after a binder object "
			"is parceled/sent to another process");

	Extras* e = mExtras.load(std::memory_order_acquire);		Extras* e = mExtras.load(std::memory_order_acquire);

	if (!e) {		if (!e) {
	@@ -410,10 +420,22 @@ pid_t BBinder::getDebugPid() {
	}		}

	void BBinder::setExtension(const sp<IBinder>& extension) {		void BBinder::setExtension(const sp<IBinder>& extension) {
			ALOGW_IF(mParceled,
			"setExtension() should not be called after a binder object "
			"is parceled/sent to another process");

	Extras* e = getOrCreateExtras();		Extras* e = getOrCreateExtras();
	e->mExtension = extension;		e->mExtension = extension;
	}		}

			bool BBinder::wasParceled() {
			return mParceled;
			}

			void BBinder::setParceled() {
			mParceled = true;
			}

	status_t BBinder::setRpcClientDebug(const Parcel& data) {		status_t BBinder::setRpcClientDebug(const Parcel& data) {
	if constexpr (!kEnableRpcDevServers) {		if constexpr (!kEnableRpcDevServers) {
	ALOGW("%s: disallowed because RPC is not enabled", __PRETTY_FUNCTION__);		ALOGW("%s: disallowed because RPC is not enabled", __PRETTY_FUNCTION__);

libs/binder/Parcel.cpp

+5 −3

Original line number	Original line	Diff line number	Diff line
	@@ -196,8 +196,11 @@ static constexpr inline int schedPolicyMask(int policy, int priority) {
	return (priority & FLAT_BINDER_FLAG_PRIORITY_MASK) \| ((policy & 3) << FLAT_BINDER_FLAG_SCHED_POLICY_SHIFT);		return (priority & FLAT_BINDER_FLAG_PRIORITY_MASK) \| ((policy & 3) << FLAT_BINDER_FLAG_SCHED_POLICY_SHIFT);
	}		}

	status_t Parcel::flattenBinder(const sp<IBinder>& binder)		status_t Parcel::flattenBinder(const sp<IBinder>& binder) {
	{		BBinder* local = nullptr;
			if (binder) local = binder->localBinder();
			if (local) local->setParceled();

	if (isForRpc()) {		if (isForRpc()) {
	if (binder) {		if (binder) {
	status_t status = writeInt32(1); // non-null		status_t status = writeInt32(1); // non-null
	@@ -223,7 +226,6 @@ status_t Parcel::flattenBinder(const sp<IBinder>& binder)
	}		}

	if (binder != nullptr) {		if (binder != nullptr) {
	BBinder *local = binder->localBinder();
	if (!local) {		if (!local) {
	BpBinder *proxy = binder->remoteBinder();		BpBinder *proxy = binder->remoteBinder();
	if (proxy == nullptr) {		if (proxy == nullptr) {

libs/binder/include/binder/Binder.h

+9 −1

Original line number	Original line	Diff line number	Diff line
	@@ -94,6 +94,13 @@ public:

	pid_t getDebugPid();		pid_t getDebugPid();

			// Whether this binder has been sent to another process.
			bool wasParceled();
			// Consider this binder as parceled (setup/init-related calls should no
			// longer by called. This is automatically set by when this binder is sent
			// to another process.
			void setParceled();

	[[nodiscard]] status_t setRpcClientDebug(android::base::unique_fd clientFd,		[[nodiscard]] status_t setRpcClientDebug(android::base::unique_fd clientFd,
	uint32_t maxRpcThreads);		uint32_t maxRpcThreads);

	@@ -120,7 +127,8 @@ private:

	friend ::android::internal::Stability;		friend ::android::internal::Stability;
	int16_t mStability;		int16_t mStability;
	int16_t mReserved0;		bool mParceled;
			uint8_t mReserved0;

	#ifdef __LP64__		#ifdef __LP64__
	int32_t mReserved1;		int32_t mReserved1;

libs/binder/tests/binderLibTest.cpp

+8 −0

Original line number	Original line	Diff line number	Diff line
	@@ -443,6 +443,14 @@ class TestDeathRecipient : public IBinder::DeathRecipient, public BinderLibTestE
	};		};
	};		};

			TEST_F(BinderLibTest, WasParceled) {
			auto binder = sp<BBinder>::make();
			EXPECT_FALSE(binder->wasParceled());
			Parcel data;
			data.writeStrongBinder(binder);
			EXPECT_TRUE(binder->wasParceled());
			}

	TEST_F(BinderLibTest, NopTransaction) {		TEST_F(BinderLibTest, NopTransaction) {
	Parcel data, reply;		Parcel data, reply;
	EXPECT_THAT(m_server->transact(BINDER_LIB_TEST_NOP_TRANSACTION, data, &reply),		EXPECT_THAT(m_server->transact(BINDER_LIB_TEST_NOP_TRANSACTION, data, &reply),