summaryrefslogtreecommitdiff
path: root/libs/binder/Parcel.cpp
diff options
context:
space:
mode:
author Frederick Mayle <fmayle@google.com> 2022-07-15 20:14:01 +0000
committer Frederick Mayle <fmayle@google.com> 2022-07-15 21:10:12 +0000
commit53b6ffe5af3951e8784c451ef8c4ff19f3d6b196 (patch)
tree8ab28ff57d9c124871514dd5aca03ce39eebf022 /libs/binder/Parcel.cpp
parentec602d42c63c9ace5c429db98bbf5974a76cac78 (diff)
libbinder: Remove Parcel argument from Parcel::release_func
This enforces a clearer separation of concerns between the data owner and the parcel code. It happened to reveal an edge case where FDs are prematurely closed (tracking a fix in b/239222407). Test: TH Bug: 239222407 Change-Id: I54ff0c8e4a8f64afd529092d2038dac40c853371
Diffstat (limited to 'libs/binder/Parcel.cpp')
-rw-r--r--libs/binder/Parcel.cpp13
1 files changed, 10 insertions, 3 deletions
diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp
index 537527e2c1..72c1a1a820 100644
--- a/libs/binder/Parcel.cpp
+++ b/libs/binder/Parcel.cpp
@@ -2706,7 +2706,9 @@ void Parcel::freeDataNoInit()
LOG_ALLOC("Parcel %p: freeing other owner data", this);
//ALOGI("Freeing data ref of %p (pid=%d)", this, getpid());
auto* kernelFields = maybeKernelFields();
- mOwner(this, mData, mDataSize, kernelFields ? kernelFields->mObjects : nullptr,
+ // Close FDs before freeing, otherwise they will leak for kernel binder.
+ closeFileDescriptors();
+ mOwner(mData, mDataSize, kernelFields ? kernelFields->mObjects : nullptr,
kernelFields ? kernelFields->mObjectsSize : 0);
} else {
LOG_ALLOC("Parcel %p: freeing allocated data", this);
@@ -2891,8 +2893,13 @@ status_t Parcel::continueWrite(size_t desired)
if (objects && kernelFields && kernelFields->mObjects) {
memcpy(objects, kernelFields->mObjects, objectsSize * sizeof(binder_size_t));
}
- //ALOGI("Freeing data ref of %p (pid=%d)", this, getpid());
- mOwner(this, mData, mDataSize, kernelFields ? kernelFields->mObjects : nullptr,
+ // ALOGI("Freeing data ref of %p (pid=%d)", this, getpid());
+ if (kernelFields) {
+ // TODO(b/239222407): This seems wrong. We should only free FDs when
+ // they are in a truncated section of the parcel.
+ closeFileDescriptors();
+ }
+ mOwner(mData, mDataSize, kernelFields ? kernelFields->mObjects : nullptr,
kernelFields ? kernelFields->mObjectsSize : 0);
mOwner = nullptr;