From 06234b37e3bfb910e7be9b4fdb035c958983a2ae Mon Sep 17 00:00:00 2001 From: Jeff Brown Date: Fri, 4 Nov 2011 19:01:44 -0700 Subject: [PATCH] Fix a leak in Parcel::writeBlob. Was mistakenly assuming that Parcel::writeFileDescriptor took ownership of the fd that was passed in. It does not! Added some comments and a default parameter to allow the caller to specify whether it wishes the Parcel to take ownership. Bug: 5563374 Change-Id: I5a12f51d582bf246ce90133cce7690bb9bca93f6 --- include/binder/Parcel.h | 3 ++- libs/binder/Parcel.cpp | 13 ++++--------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/include/binder/Parcel.h b/include/binder/Parcel.h index 3fa2acbda..33b2f0050 100644 --- a/include/binder/Parcel.h +++ b/include/binder/Parcel.h @@ -110,7 +110,8 @@ public: // Place a file descriptor into the parcel. The given fd must remain // valid for the lifetime of the parcel. - status_t writeFileDescriptor(int fd); + // The Parcel does not take ownership of the given fd unless you ask it to. + status_t writeFileDescriptor(int fd, bool takeOwnership = false); // Place a file descriptor into the parcel. A dup of the fd is made, which // will be closed once the parcel is destroyed. diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index c7180cee0..6b4c1a61e 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -710,24 +710,19 @@ status_t Parcel::writeNativeHandle(const native_handle* handle) return err; } -status_t Parcel::writeFileDescriptor(int fd) +status_t Parcel::writeFileDescriptor(int fd, bool takeOwnership) { flat_binder_object obj; obj.type = BINDER_TYPE_FD; obj.flags = 0x7f | FLAT_BINDER_FLAG_ACCEPTS_FDS; obj.handle = fd; - obj.cookie = (void*)0; + obj.cookie = (void*) (takeOwnership ? 1 : 0); return writeObject(obj, true); } status_t Parcel::writeDupFileDescriptor(int fd) { - flat_binder_object obj; - obj.type = BINDER_TYPE_FD; - obj.flags = 0x7f | FLAT_BINDER_FLAG_ACCEPTS_FDS; - obj.handle = dup(fd); - obj.cookie = (void*)1; - return writeObject(obj, true); + return writeFileDescriptor(dup(fd), true /*takeOwnership*/); } status_t Parcel::writeBlob(size_t len, WritableBlob* outBlob) @@ -764,7 +759,7 @@ status_t Parcel::writeBlob(size_t len, WritableBlob* outBlob) } else { status = writeInt32(1); if (!status) { - status = writeFileDescriptor(fd); + status = writeFileDescriptor(fd, true /*takeOwnership*/); if (!status) { outBlob->init(true /*mapped*/, ptr, len); return NO_ERROR;