commit 750d69b248d1f7c90c8dbe6f1eee322cf0fc59e7
author Steven Moreland <smoreland@google.com> Fri Dec 13 23:14:50 2024 +0000
committer Steven Moreland <smoreland@google.com> Mon Dec 16 14:19:55 2024 -0800
tree d508411e2b403911c76192df7570f48f1229754f
parent 575fdb3495cba6c3f5f47495b1b2c14f493256fe

libbinder: remove overeager fdsan crash

In the continueWrite case, if a Parcel contians FDs,
we try to tag the FDs we own in Parcel a second time.
This causes a crash.

Bug: 384097481
Test: with fuzzer, on device and host
Ignore-AOSP-First: fuzzing
Change-Id: I01b2312c316fa457781a4a0c1a81bfd845d0e60e

libs/binder/Parcel.cpp

diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp
index d09d5a8..e2eea6c 100644
--- a/libs/binder/Parcel.cpp
+++ b/libs/binder/Parcel.cpp
@@ -156,7 +156,7 @@
 
 #ifdef BINDER_WITH_KERNEL_IPC
 static void acquire_object(const sp<ProcessState>& proc, const flat_binder_object& obj,
-                           const void* who) {
+                           const void* who, bool tagFds) {
     switch (obj.hdr.type) {
         case BINDER_TYPE_BINDER:
             if (obj.binder) {
@@ -173,7 +173,7 @@
             return;
         }
         case BINDER_TYPE_FD: {
-            if (obj.cookie != 0) { // owned
+            if (tagFds && obj.cookie != 0) { // owned
                 FdTag(obj.handle, nullptr, who);
             }
             return;
@@ -611,7 +611,7 @@
                     }
                 }
 
-                acquire_object(proc, *flat, this);
+                acquire_object(proc, *flat, this, true /*tagFds*/);
             }
         }
 #else
@@ -1797,7 +1797,7 @@
         // Need to write meta-data?
         if (nullMetaData || val.binder != 0) {
             kernelFields->mObjects[kernelFields->mObjectsSize] = mDataPos;
-            acquire_object(ProcessState::self(), val, this);
+            acquire_object(ProcessState::self(), val, this, true /*tagFds*/);
             kernelFields->mObjectsSize++;
         }
 
@@ -2883,15 +2883,17 @@
 #endif // BINDER_WITH_KERNEL_IPC
 }
 
-void Parcel::acquireObjects()
-{
+void Parcel::reacquireObjects(size_t objectsSize) {
     auto* kernelFields = maybeKernelFields();
     if (kernelFields == nullptr) {
         return;
     }
 
 #ifdef BINDER_WITH_KERNEL_IPC
-    size_t i = kernelFields->mObjectsSize;
+    LOG_ALWAYS_FATAL_IF(objectsSize > kernelFields->mObjectsSize,
+                        "Object size %zu out of range of %zu", objectsSize,
+                        kernelFields->mObjectsSize);
+    size_t i = objectsSize;
     if (i == 0) {
         return;
     }
@@ -2901,8 +2903,10 @@
     while (i > 0) {
         i--;
         const flat_binder_object* flat = reinterpret_cast<flat_binder_object*>(data + objects[i]);
-        acquire_object(proc, *flat, this);
+        acquire_object(proc, *flat, this, false /*tagFds*/); // they are already tagged
     }
+#else
+    (void) objectsSize;
 #endif // BINDER_WITH_KERNEL_IPC
 }
 
@@ -3119,12 +3123,8 @@
                 return NO_MEMORY;
             }
 
-            // Little hack to only acquire references on objects
-            // we will be keeping.
-            size_t oldObjectsSize = kernelFields->mObjectsSize;
-            kernelFields->mObjectsSize = objectsSize;
-            acquireObjects();
-            kernelFields->mObjectsSize = oldObjectsSize;
+            // only acquire references on objects we are keeping
+            reacquireObjects(objectsSize);
         }
         if (rpcFields) {
             if (status_t status = truncateRpcObjects(objectsSize); status != OK) {