Merge changes I5160e72b,I54ff0c8e am: c59b49c3cd am: 637ee9248f am: 9c9ffe6208 am: 079854e8d1 am: f2e153663c
Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/2153945
Change-Id: Ia93dba9a177c63785a5fcadb4302d63213742ff5
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
diff --git a/libs/binder/IPCThreadState.cpp b/libs/binder/IPCThreadState.cpp
index d536219..b50cfb3 100644
--- a/libs/binder/IPCThreadState.cpp
+++ b/libs/binder/IPCThreadState.cpp
@@ -972,18 +972,15 @@
freeBuffer);
} else {
err = *reinterpret_cast<const status_t*>(tr.data.ptr.buffer);
- freeBuffer(nullptr,
- reinterpret_cast<const uint8_t*>(tr.data.ptr.buffer),
- tr.data_size,
- reinterpret_cast<const binder_size_t*>(tr.data.ptr.offsets),
- tr.offsets_size/sizeof(binder_size_t));
+ freeBuffer(reinterpret_cast<const uint8_t*>(tr.data.ptr.buffer),
+ tr.data_size,
+ reinterpret_cast<const binder_size_t*>(tr.data.ptr.offsets),
+ tr.offsets_size / sizeof(binder_size_t));
}
} else {
- freeBuffer(nullptr,
- reinterpret_cast<const uint8_t*>(tr.data.ptr.buffer),
- tr.data_size,
- reinterpret_cast<const binder_size_t*>(tr.data.ptr.offsets),
- tr.offsets_size/sizeof(binder_size_t));
+ freeBuffer(reinterpret_cast<const uint8_t*>(tr.data.ptr.buffer), tr.data_size,
+ reinterpret_cast<const binder_size_t*>(tr.data.ptr.offsets),
+ tr.offsets_size / sizeof(binder_size_t));
continue;
}
}
@@ -1473,17 +1470,13 @@
ee.id, ee.command, ee.param);
}
-void IPCThreadState::freeBuffer(Parcel* parcel, const uint8_t* data,
- size_t /*dataSize*/,
- const binder_size_t* /*objects*/,
- size_t /*objectsSize*/)
-{
+void IPCThreadState::freeBuffer(const uint8_t* data, size_t /*dataSize*/,
+ const binder_size_t* /*objects*/, size_t /*objectsSize*/) {
//ALOGI("Freeing parcel %p", &parcel);
IF_LOG_COMMANDS() {
alog << "Writing BC_FREE_BUFFER for " << data << endl;
}
ALOG_ASSERT(data != NULL, "Called with NULL data");
- if (parcel != nullptr) parcel->closeFileDescriptors();
IPCThreadState* state = self();
state->mOut.writeInt32(BC_FREE_BUFFER);
state->mOut.writePointer((uintptr_t)data);
diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp
index 537527e..8b5d118 100644
--- a/libs/binder/Parcel.cpp
+++ b/libs/binder/Parcel.cpp
@@ -2590,6 +2590,22 @@
LOG_ALWAYS_FATAL_IF(session == nullptr);
+ if (objectTableSize != ancillaryFds.size()) {
+ ALOGE("objectTableSize=%zu ancillaryFds.size=%zu", objectTableSize, ancillaryFds.size());
+ relFunc(data, dataSize, nullptr, 0);
+ return BAD_VALUE;
+ }
+ for (size_t i = 0; i < objectTableSize; i++) {
+ uint32_t minObjectEnd;
+ if (__builtin_add_overflow(objectTable[i], sizeof(RpcFields::ObjectType), &minObjectEnd) ||
+ minObjectEnd >= dataSize) {
+ ALOGE("received out of range object position: %" PRIu32 " (parcel size is %zu)",
+ objectTable[i], dataSize);
+ relFunc(data, dataSize, nullptr, 0);
+ return BAD_VALUE;
+ }
+ }
+
freeData();
markForRpc(session);
@@ -2600,12 +2616,6 @@
mDataSize = mDataCapacity = dataSize;
mOwner = relFunc;
- if (objectTableSize != ancillaryFds.size()) {
- ALOGE("objectTableSize=%zu ancillaryFds.size=%zu", objectTableSize, ancillaryFds.size());
- freeData(); // don't leak mData
- return BAD_VALUE;
- }
-
rpcFields->mObjectPositions.reserve(objectTableSize);
for (size_t i = 0; i < objectTableSize; i++) {
rpcFields->mObjectPositions.push_back(objectTable[i]);
@@ -2706,7 +2716,9 @@
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 +2903,13 @@
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;
diff --git a/libs/binder/RpcState.cpp b/libs/binder/RpcState.cpp
index 633fea9..1c5654c 100644
--- a/libs/binder/RpcState.cpp
+++ b/libs/binder/RpcState.cpp
@@ -586,13 +586,12 @@
return waitForReply(connection, session, reply);
}
-static void cleanup_reply_data(Parcel* p, const uint8_t* data, size_t dataSize,
- const binder_size_t* objects, size_t objectsCount) {
- (void)p;
+static void cleanup_reply_data(const uint8_t* data, size_t dataSize, const binder_size_t* objects,
+ size_t objectsCount) {
delete[] const_cast<uint8_t*>(data);
(void)dataSize;
LOG_ALWAYS_FATAL_IF(objects != nullptr);
- LOG_ALWAYS_FATAL_IF(objectsCount != 0, "%zu objects remaining", objectsCount);
+ (void)objectsCount;
}
status_t RpcState::waitForReply(const sp<RpcSession::RpcConnection>& connection,
@@ -799,9 +798,8 @@
std::move(ancillaryFds));
}
-static void do_nothing_to_transact_data(Parcel* p, const uint8_t* data, size_t dataSize,
+static void do_nothing_to_transact_data(const uint8_t* data, size_t dataSize,
const binder_size_t* objects, size_t objectsCount) {
- (void)p;
(void)data;
(void)dataSize;
(void)objects;
diff --git a/libs/binder/include/binder/IPCThreadState.h b/libs/binder/include/binder/IPCThreadState.h
index 8ce3bc9..c01e92f 100644
--- a/libs/binder/include/binder/IPCThreadState.h
+++ b/libs/binder/include/binder/IPCThreadState.h
@@ -217,9 +217,8 @@
void clearCaller();
static void threadDestructor(void *st);
- static void freeBuffer(Parcel* parcel,
- const uint8_t* data, size_t dataSize,
- const binder_size_t* objects, size_t objectsSize);
+ static void freeBuffer(const uint8_t* data, size_t dataSize, const binder_size_t* objects,
+ size_t objectsSize);
static void logExtendedError();
const sp<ProcessState> mProcess;
diff --git a/libs/binder/include/binder/Parcel.h b/libs/binder/include/binder/Parcel.h
index 91febbd..5469239 100644
--- a/libs/binder/include/binder/Parcel.h
+++ b/libs/binder/include/binder/Parcel.h
@@ -598,9 +598,9 @@
void print(TextOutput& to, uint32_t flags = 0) const;
private:
- typedef void (*release_func)(Parcel* parcel,
- const uint8_t* data, size_t dataSize,
- const binder_size_t* objects, size_t objectsSize);
+ // `objects` and `objectsSize` always 0 for RPC Parcels.
+ typedef void (*release_func)(const uint8_t* data, size_t dataSize, const binder_size_t* objects,
+ size_t objectsSize);
uintptr_t ipcData() const;
size_t ipcDataSize() const;