commit c7d40135ed5fb30434c95f3058324b0af6916711
author Steven Moreland <smoreland@google.com> Thu Jun 10 03:42:11 2021 +0000
committer Steven Moreland <smoreland@google.com> Sat Jun 12 00:22:10 2021 +0000
tree 41e639c646d22fe949ad824aae132ead462ffffb
parent 5ae62560923bb111d3092d96a4ceb8cf3b8965d3

libbinder: RPC disallow nested oneway transactions

Previously, nested transactions were accidentally allowed while
processing oneway transactions. This changes things so that nested
transactions are only explicitly allowed when a synchronous transaction
is being processed (like how kernel binder is).

Future considerations: this CL makes it more explicit that we allow
refcount transactions as part of nested transactions. This is okay
because 'drainCommands' will process these, but there might be some
delay. We could make refcount behavior nicer if we always preferred
using an active threadpool (if one is available) to process them.

Bug: 167966510
Test: binderRpcTest
Change-Id: Iaeb472896654ff4bcd75b20394f8f3230febaabf

libs/binder/RpcState.cpp

diff --git a/libs/binder/RpcState.cpp b/libs/binder/RpcState.cpp
index 5a20156..050f4fb 100644
--- a/libs/binder/RpcState.cpp
+++ b/libs/binder/RpcState.cpp
@@ -633,6 +633,7 @@
     // TODO(b/182939933): heap allocation just for lookup in mNodeForAddress,
     // maybe add an RpcAddress 'view' if the type remains 'heavy'
     auto addr = RpcAddress::fromRawEmbedded(&transaction->address);
+    bool oneway = transaction->flags & IBinder::FLAG_ONEWAY;
 
     status_t replyStatus = OK;
     sp<IBinder> target;
@@ -661,7 +662,7 @@
                   addr.toString().c_str());
             (void)session->shutdownAndWait(false);
             replyStatus = BAD_VALUE;
-        } else if (transaction->flags & IBinder::FLAG_ONEWAY) {
+        } else if (oneway) {
             std::unique_lock<std::mutex> _l(mNodeMutex);
             auto it = mNodeForAddress.find(addr);
             if (it->second.binder.promote() != target) {
@@ -718,7 +719,12 @@
         data.markForRpc(session);
 
         if (target) {
+            bool origAllowNested = connection->allowNested;
+            connection->allowNested = !oneway;
+
             replyStatus = target->transact(transaction->code, data, &reply, transaction->flags);
+
+            connection->allowNested = origAllowNested;
         } else {
             LOG_RPC_DETAIL("Got special transaction %u", transaction->code);
 
@@ -754,7 +760,7 @@
         }
     }
 
-    if (transaction->flags & IBinder::FLAG_ONEWAY) {
+    if (oneway) {
         if (replyStatus != OK) {
             ALOGW("Oneway call failed with error: %d", replyStatus);
         }