avoid extra release of unowned objects in Parcel error path am: d668098e47 (41db1b0e) · Commits · e / os / platform_frameworks_native

libs/binder/Parcel.cpp

+5 −3

Original line number	Diff line number	Diff line
		@@ -2322,12 +2322,14 @@ void Parcel::ipcSetDataReference(const uint8_t* data, size_t dataSize,
		type == BINDER_TYPE_FD)) {
		// We should never receive other types (eg BINDER_TYPE_FDA) as long as we don't support
		// them in libbinder. If we do receive them, it probably means a kernel bug; try to
		// recover gracefully by clearing out the objects, and releasing the objects we do
		// know about.
		// recover gracefully by clearing out the objects.
		android_errorWriteLog(0x534e4554, "135930648");
		android_errorWriteLog(0x534e4554, "203847542");
		ALOGE("%s: unsupported type object (%" PRIu32 ") at offset %" PRIu64 "\n",
		__func__, type, (uint64_t)offset);
		releaseObjects();

		// WARNING: callers of ipcSetDataReference need to make sure they
		// don't rely on mObjectsSize in their release_func.
		mObjectsSize = 0;
		break;
		}

libs/binder/tests/binderLibTest.cpp

+42 −3

Original line number	Diff line number	Diff line
		@@ -76,7 +76,7 @@ enum BinderLibTestTranscationCode {
		BINDER_LIB_TEST_CREATE_BINDER_TRANSACTION,
		BINDER_LIB_TEST_GET_WORK_SOURCE_TRANSACTION,
		BINDER_LIB_TEST_ECHO_VECTOR,
		BINDER_LIB_TEST_REJECT_BUF,
		BINDER_LIB_TEST_REJECT_OBJECTS,
		};

		pid_t start_server_process(int arg2, bool usePoll = false)
		@@ -1050,14 +1050,53 @@ TEST_F(BinderLibTest, BufRejected) {
		// And now, overwrite it with the buffer object
		memcpy(parcelData, &obj, sizeof(obj));
		data.setDataSize(sizeof(obj));
		EXPECT_EQ(data.objectsCount(), 1);

		status_t ret = server->transact(BINDER_LIB_TEST_REJECT_OBJECTS, data, &reply);

		status_t ret = server->transact(BINDER_LIB_TEST_REJECT_BUF, data, &reply);
		// Either the kernel should reject this transaction (if it's correct), but
		// if it's not, the server implementation should return an error if it
		// finds an object in the received Parcel.
		EXPECT_NE(NO_ERROR, ret);
		}

		TEST_F(BinderLibTest, WeakRejected) {
		Parcel data, reply;
		sp<IBinder> server = addServer();
		ASSERT_TRUE(server != nullptr);

		sp<BBinder> binder = new BBinder();
		wp<BBinder> wpBinder(binder);
		flat_binder_object obj{
		.hdr = {.type = BINDER_TYPE_WEAK_BINDER},
		.flags = 0,
		.binder = reinterpret_cast<uintptr_t>(wpBinder.get_refs()),
		.cookie = reinterpret_cast<uintptr_t>(wpBinder.unsafe_get()),
		};
		data.setDataCapacity(1024);
		// Write a bogus object at offset 0 to get an entry in the offset table
		data.writeFileDescriptor(0);
		EXPECT_EQ(data.objectsCount(), 1);
		uint8_t parcelData = const_cast<uint8_t >(data.data());
		// And now, overwrite it with the weak binder
		memcpy(parcelData, &obj, sizeof(obj));
		data.setDataSize(sizeof(obj));

		// a previous bug caused other objects to be released an extra time, so we
		// test with an object that libbinder will actually try to release
		EXPECT_EQ(OK, data.writeStrongBinder(new BBinder()));

		EXPECT_EQ(data.objectsCount(), 2);

		// send it many times, since previous error was memory corruption, make it
		// more likely that the server crashes
		for (size_t i = 0; i < 100; i++) {
		EXPECT_EQ(BAD_VALUE, server->transact(BINDER_LIB_TEST_REJECT_OBJECTS, data, &reply));
		}

		EXPECT_EQ(NO_ERROR, server->pingBinder());
		}

		class BinderLibTestService : public BBinder
		{
		public:
		@@ -1340,7 +1379,7 @@ class BinderLibTestService : public BBinder
		reply->writeUint64Vector(vector);
		return NO_ERROR;
		}
		case BINDER_LIB_TEST_REJECT_BUF: {
		case BINDER_LIB_TEST_REJECT_OBJECTS: {
		return data.objectsCount() == 0 ? BAD_VALUE : NO_ERROR;
		}
		default: