commit 438cce8a7489d4da97ed6fa58e3e8f3d71e5021e
author Steven Moreland <smoreland@google.com> Fri Apr 02 18:04:08 2021 +0000
committer Steven Moreland <smoreland@google.com> Sat Apr 03 00:04:32 2021 +0000
tree ae7a1ab2742d5775f55a0a06c629b002e2e9e3d9
parent e8fa8e3458982b2a6cb7a7c24fa1fb831be47cfe

libbinder: RPC avoid copy of transaction data

The socket code here needs a lot more optimization/analysis, but using
setIpcDataReference here is important so that we know internally whether
a Parcel is owned or not. This will allow for stronger checks in markFor*
functions which control the Parcel format.

Bug: 167966510
Test: binderRpcTest
Change-Id: Id2b0fc02972936f3e3b7382e7b56f7631a621e97

libs/binder/Parcel.cpp

diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp
index 34a474b..96d12ca 100644
--- a/libs/binder/Parcel.cpp
+++ b/libs/binder/Parcel.cpp
@@ -2100,6 +2100,9 @@
 void Parcel::ipcSetDataReference(const uint8_t* data, size_t dataSize,
     const binder_size_t* objects, size_t objectsCount, release_func relFunc)
 {
+    // this code uses 'mOwner == nullptr' to understand whether it owns memory
+    LOG_ALWAYS_FATAL_IF(relFunc == nullptr, "must provide cleanup function");
+
     freeData();
 
     mData = const_cast<uint8_t*>(data);

libs/binder/RpcState.cpp

diff --git a/libs/binder/RpcState.cpp b/libs/binder/RpcState.cpp
index 3b3adca..755ff35 100644
--- a/libs/binder/RpcState.cpp
+++ b/libs/binder/RpcState.cpp
@@ -312,8 +312,8 @@
     return waitForReply(fd, connection, reply);
 }
 
-static void cleanup_data(Parcel* p, const uint8_t* data, size_t dataSize,
-                         const binder_size_t* objects, size_t objectsCount) {
+static void cleanup_reply_data(Parcel* p, const uint8_t* data, size_t dataSize,
+                               const binder_size_t* objects, size_t objectsCount) {
     (void)p;
     delete[] const_cast<uint8_t*>(data - offsetof(RpcWireReply, data));
     (void)dataSize;
@@ -351,7 +351,7 @@
     if (rpcReply->status != OK) return rpcReply->status;
 
     reply->ipcSetDataReference(rpcReply->data, command.bodySize - offsetof(RpcWireReply, data),
-                               nullptr, 0, cleanup_data);
+                               nullptr, 0, cleanup_reply_data);
 
     reply->markForRpc(connection);
 
@@ -427,6 +427,15 @@
     return processTransactInternal(fd, connection, std::move(transactionData));
 }
 
+static void do_nothing_to_transact_data(Parcel* p, const uint8_t* data, size_t dataSize,
+                                        const binder_size_t* objects, size_t objectsCount) {
+    (void)p;
+    (void)data;
+    (void)dataSize;
+    (void)objects;
+    (void)objectsCount;
+}
+
 status_t RpcState::processTransactInternal(const base::unique_fd& fd,
                                            const sp<RpcConnection>& connection,
                                            std::vector<uint8_t>&& transactionData) {
@@ -490,7 +499,12 @@
     }
 
     Parcel data;
-    data.setData(transaction->data, transactionData.size() - offsetof(RpcWireTransaction, data));
+    // transaction->data is owned by this function. Parcel borrows this data and
+    // only holds onto it for the duration of this function call. Parcel will be
+    // deleted before the 'transactionData' object.
+    data.ipcSetDataReference(transaction->data,
+                             transactionData.size() - offsetof(RpcWireTransaction, data),
+                             nullptr /*object*/, 0 /*objectCount*/, do_nothing_to_transact_data);
     data.markForRpc(connection);
 
     Parcel reply;