Reject invalid object types for incoming transactions.
TYPE_PTR and TYPE_FDA should only be used for scatter-gather
transactions, which is not supported by libbinder. Because userspace is
responsible for cleaning up the transaction objects, we can't just
reject the transaction outright; instead, free the objects and make sure
the Parcel won't contain them going forward.
While we're at it, also reject weak binders now, since those are no
longer supported.
For Parcels which were expecting valid objects, those will now fail to
unparcel, and return an error to the caller, which seems reasonable.
For Parcels which didn't expect any objects, the Parcel will be read
out successfully, but also no harm should be done, as there's no way to
propagate the bad objects.
Bug: 135930648
Test: atest binderLibTest
Change-Id: I2a4b408d6dcf1a67f3093d40061cb6159a0444f9
diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp
index 495b2f9..5c6cf9d 100644
--- a/libs/binder/tests/binderLibTest.cpp
+++ b/libs/binder/tests/binderLibTest.cpp
@@ -72,6 +72,7 @@
BINDER_LIB_TEST_CREATE_BINDER_TRANSACTION,
BINDER_LIB_TEST_GET_WORK_SOURCE_TRANSACTION,
BINDER_LIB_TEST_ECHO_VECTOR,
+ BINDER_LIB_TEST_REJECT_BUF,
};
pid_t start_server_process(int arg2, bool usePoll = false)
@@ -1024,6 +1025,34 @@
EXPECT_EQ(readValue, testValue);
}
+TEST_F(BinderLibTest, BufRejected) {
+ Parcel data, reply;
+ uint32_t buf;
+ sp<IBinder> server = addServer();
+ ASSERT_TRUE(server != nullptr);
+
+ binder_buffer_object obj {
+ .hdr = { .type = BINDER_TYPE_PTR },
+ .buffer = reinterpret_cast<binder_uintptr_t>((void*)&buf),
+ .length = 4,
+ .flags = 0,
+ };
+ 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 buffer object
+ memcpy(parcelData, &obj, sizeof(obj));
+ data.setDataSize(sizeof(obj));
+
+ 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);
+}
+
class BinderLibTestService : public BBinder
{
public:
@@ -1306,6 +1335,9 @@
reply->writeUint64Vector(vector);
return NO_ERROR;
}
+ case BINDER_LIB_TEST_REJECT_BUF: {
+ return data.objectsCount() == 0 ? BAD_VALUE : NO_ERROR;
+ }
default:
return UNKNOWN_TRANSACTION;
};
@@ -1337,6 +1369,9 @@
*/
testService->setExtension(new BBinder());
+ // Required for test "BufRejected'
+ testService->setRequestingSid(true);
+
/*
* We need this below, but can't hold a sp<> because it prevents the
* node from being cleaned up automatically. It's safe in this case