summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--libs/binder/Android.bp1
-rw-r--r--libs/binder/Binder.cpp4
-rw-r--r--libs/binder/IPCThreadState.cpp4
-rw-r--r--libs/binder/Parcel.cpp29
-rw-r--r--libs/binder/Utils.cpp27
-rw-r--r--libs/binder/Utils.h25
-rw-r--r--libs/binder/include/binder/IBinder.h4
-rw-r--r--libs/binder/include/binder/Parcel.h11
-rw-r--r--libs/binder/include/private/binder/binder_module.h4
-rw-r--r--libs/binder/ndk/ibinder.cpp2
-rw-r--r--libs/binder/ndk/include_ndk/android/binder_ibinder.h1
-rw-r--r--libs/binder/ndk/include_platform/android/binder_ibinder_platform.h11
-rw-r--r--libs/binder/ndk/include_platform/android/binder_parcel_platform.h11
-rw-r--r--libs/binder/ndk/libbinder_ndk.map.txt5
-rw-r--r--libs/binder/ndk/parcel.cpp4
-rw-r--r--libs/binder/rust/src/binder.rs5
-rw-r--r--libs/binder/rust/src/parcel.rs8
-rw-r--r--libs/binder/rust/sys/BinderBindings.hpp2
-rw-r--r--libs/binder/tests/binderLibTest.cpp8
19 files changed, 156 insertions, 10 deletions
diff --git a/libs/binder/Android.bp b/libs/binder/Android.bp
index e6071a06df..e0d5378b19 100644
--- a/libs/binder/Android.bp
+++ b/libs/binder/Android.bp
@@ -112,6 +112,7 @@ cc_library {
"Stability.cpp",
"Status.cpp",
"TextOutput.cpp",
+ "Utils.cpp",
":libbinder_aidl",
],
diff --git a/libs/binder/Binder.cpp b/libs/binder/Binder.cpp
index 6ca3b16324..f2d223dad3 100644
--- a/libs/binder/Binder.cpp
+++ b/libs/binder/Binder.cpp
@@ -173,6 +173,10 @@ status_t BBinder::transact(
{
data.setDataPosition(0);
+ if (reply != nullptr && (flags & FLAG_CLEAR_BUF)) {
+ reply->markSensitive();
+ }
+
status_t err = NO_ERROR;
switch (code) {
case PING_TRANSACTION:
diff --git a/libs/binder/IPCThreadState.cpp b/libs/binder/IPCThreadState.cpp
index 05fcc2b878..a3a2f871e6 100644
--- a/libs/binder/IPCThreadState.cpp
+++ b/libs/binder/IPCThreadState.cpp
@@ -1244,7 +1244,9 @@ status_t IPCThreadState::executeCommand(int32_t cmd)
if ((tr.flags & TF_ONE_WAY) == 0) {
LOG_ONEWAY("Sending reply to %d!", mCallingPid);
if (error < NO_ERROR) reply.setError(error);
- sendReply(reply, 0);
+
+ constexpr uint32_t kForwardReplyFlags = TF_CLEAR_BUF;
+ sendReply(reply, (tr.flags & kForwardReplyFlags));
} else {
if (error != OK || reply.dataSize() != 0) {
alog << "oneway function results will be dropped but finished with status "
diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp
index 27cf97c945..feeb819a8a 100644
--- a/libs/binder/Parcel.cpp
+++ b/libs/binder/Parcel.cpp
@@ -49,6 +49,7 @@
#include <private/binder/binder_module.h>
#include "Static.h"
+#include "Utils.h"
#define LOG_REFS(...)
//#define LOG_REFS(...) ALOG(LOG_DEBUG, LOG_TAG, __VA_ARGS__)
@@ -502,6 +503,11 @@ bool Parcel::hasFileDescriptors() const
return mHasFds;
}
+void Parcel::markSensitive() const
+{
+ mDeallocZero = true;
+}
+
void Parcel::updateWorkSourceRequestHeaderPosition() const {
// Only update the request headers once. We only want to point
// to the first headers read/written.
@@ -2627,6 +2633,9 @@ void Parcel::freeDataNoInit()
LOG_ALLOC("Parcel %p: freeing with %zu capacity", this, mDataCapacity);
gParcelGlobalAllocSize -= mDataCapacity;
gParcelGlobalAllocCount--;
+ if (mDeallocZero) {
+ zeroMemory(mData, mDataSize);
+ }
free(mData);
}
if (mObjects) free(mObjects);
@@ -2649,6 +2658,21 @@ status_t Parcel::growData(size_t len)
: continueWrite(std::max(newSize, (size_t) 128));
}
+static uint8_t* reallocZeroFree(uint8_t* data, size_t oldCapacity, size_t newCapacity, bool zero) {
+ if (!zero) {
+ return (uint8_t*)realloc(data, newCapacity);
+ }
+ uint8_t* newData = (uint8_t*)malloc(newCapacity);
+ if (!newData) {
+ return nullptr;
+ }
+
+ memcpy(newData, data, std::min(oldCapacity, newCapacity));
+ zeroMemory(data, oldCapacity);
+ free(data);
+ return newData;
+}
+
status_t Parcel::restartWrite(size_t desired)
{
if (desired > INT32_MAX) {
@@ -2662,7 +2686,7 @@ status_t Parcel::restartWrite(size_t desired)
return continueWrite(desired);
}
- uint8_t* data = (uint8_t*)realloc(mData, desired);
+ uint8_t* data = reallocZeroFree(mData, mDataCapacity, desired, mDeallocZero);
if (!data && desired > mDataCapacity) {
mError = NO_MEMORY;
return NO_MEMORY;
@@ -2813,7 +2837,7 @@ status_t Parcel::continueWrite(size_t desired)
// We own the data, so we can just do a realloc().
if (desired > mDataCapacity) {
- uint8_t* data = (uint8_t*)realloc(mData, desired);
+ uint8_t* data = reallocZeroFree(mData, mDataCapacity, desired, mDeallocZero);
if (data) {
LOG_ALLOC("Parcel %p: continue from %zu to %zu capacity", this, mDataCapacity,
desired);
@@ -2881,6 +2905,7 @@ void Parcel::initState()
mHasFds = false;
mFdsKnown = true;
mAllowFds = true;
+ mDeallocZero = false;
mOwner = nullptr;
mOpenAshmemSize = 0;
mWorkSourceRequestHeaderPosition = 0;
diff --git a/libs/binder/Utils.cpp b/libs/binder/Utils.cpp
new file mode 100644
index 0000000000..90a4502ec5
--- /dev/null
+++ b/libs/binder/Utils.cpp
@@ -0,0 +1,27 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "Utils.h"
+
+#include <string.h>
+
+namespace android {
+
+void zeroMemory(uint8_t* data, size_t size) {
+ memset(data, 0, size);
+}
+
+} // namespace android
diff --git a/libs/binder/Utils.h b/libs/binder/Utils.h
new file mode 100644
index 0000000000..f94b158404
--- /dev/null
+++ b/libs/binder/Utils.h
@@ -0,0 +1,25 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <cstdint>
+#include <stddef.h>
+
+namespace android {
+
+// avoid optimizations
+void zeroMemory(uint8_t* data, size_t size);
+
+} // namespace android
diff --git a/libs/binder/include/binder/IBinder.h b/libs/binder/include/binder/IBinder.h
index eea0e89738..7b37e8de0c 100644
--- a/libs/binder/include/binder/IBinder.h
+++ b/libs/binder/include/binder/IBinder.h
@@ -64,6 +64,10 @@ public:
// Corresponds to TF_ONE_WAY -- an asynchronous call.
FLAG_ONEWAY = 0x00000001,
+ // Corresponds to TF_CLEAR_BUF -- clear transaction buffers after call
+ // is made
+ FLAG_CLEAR_BUF = 0x00000020,
+
// Private userspace flag for transaction which is being requested from
// a vendor context.
FLAG_PRIVATE_VENDOR = 0x10000000,
diff --git a/libs/binder/include/binder/Parcel.h b/libs/binder/include/binder/Parcel.h
index fbfd6c5d71..84bfe38032 100644
--- a/libs/binder/include/binder/Parcel.h
+++ b/libs/binder/include/binder/Parcel.h
@@ -84,6 +84,13 @@ public:
bool hasFileDescriptors() const;
+ // Zeros data when reallocating. Other mitigations may be added
+ // in the future.
+ //
+ // WARNING: some read methods may make additional copies of data.
+ // In order to verify this, heap dumps should be used.
+ void markSensitive() const;
+
// Writes the RPC header.
status_t writeInterfaceToken(const String16& interface);
status_t writeInterfaceToken(const char16_t* str, size_t len);
@@ -600,6 +607,10 @@ private:
mutable bool mHasFds;
bool mAllowFds;
+ // if this parcelable is involved in a secure transaction, force the
+ // data to be overridden with zero when deallocated
+ mutable bool mDeallocZero;
+
release_func mOwner;
void* mOwnerCookie;
diff --git a/libs/binder/include/private/binder/binder_module.h b/libs/binder/include/private/binder/binder_module.h
index 7be8f7b2d9..0fe7f5b088 100644
--- a/libs/binder/include/private/binder/binder_module.h
+++ b/libs/binder/include/private/binder/binder_module.h
@@ -88,7 +88,9 @@ struct binder_frozen_status_info {
};
#endif //BINDER_GET_FROZEN_INFO
-
+enum transaction_flags_ext {
+ TF_CLEAR_BUF = 0x20, /* clear buffer on txn complete */
+};
#ifdef __cplusplus
} // namespace android
diff --git a/libs/binder/ndk/ibinder.cpp b/libs/binder/ndk/ibinder.cpp
index d35debc88c..b927f6f390 100644
--- a/libs/binder/ndk/ibinder.cpp
+++ b/libs/binder/ndk/ibinder.cpp
@@ -611,7 +611,7 @@ binder_status_t AIBinder_transact(AIBinder* binder, transaction_code_t code, APa
return STATUS_UNKNOWN_TRANSACTION;
}
- constexpr binder_flags_t kAllFlags = FLAG_PRIVATE_VENDOR | FLAG_ONEWAY;
+ constexpr binder_flags_t kAllFlags = FLAG_PRIVATE_VENDOR | FLAG_ONEWAY | FLAG_CLEAR_BUF;
if ((flags & ~kAllFlags) != 0) {
LOG(ERROR) << __func__ << ": Unrecognized flags sent: " << flags;
return STATUS_BAD_VALUE;
diff --git a/libs/binder/ndk/include_ndk/android/binder_ibinder.h b/libs/binder/ndk/include_ndk/android/binder_ibinder.h
index 33763d58be..ce3d1db62d 100644
--- a/libs/binder/ndk/include_ndk/android/binder_ibinder.h
+++ b/libs/binder/ndk/include_ndk/android/binder_ibinder.h
@@ -43,7 +43,6 @@ __BEGIN_DECLS
#if __ANDROID_API__ >= 29
-// Also see TF_* in kernel's binder.h
typedef uint32_t binder_flags_t;
enum {
/**
diff --git a/libs/binder/ndk/include_platform/android/binder_ibinder_platform.h b/libs/binder/ndk/include_platform/android/binder_ibinder_platform.h
index a99d5559ee..e315c798ee 100644
--- a/libs/binder/ndk/include_platform/android/binder_ibinder_platform.h
+++ b/libs/binder/ndk/include_platform/android/binder_ibinder_platform.h
@@ -20,6 +20,17 @@
__BEGIN_DECLS
+// platform values for binder_flags_t
+enum {
+ /**
+ * The transaction and reply will be cleared by the kernel in read-only
+ * binder buffers storing transactions.
+ *
+ * Introduced in API level 31.
+ */
+ FLAG_CLEAR_BUF = 0x20,
+};
+
/**
* Makes calls to AIBinder_getCallingSid work if the kernel supports it. This
* must be called on a local binder server before it is sent out to any othe
diff --git a/libs/binder/ndk/include_platform/android/binder_parcel_platform.h b/libs/binder/ndk/include_platform/android/binder_parcel_platform.h
index 114a781232..d54c1a18ba 100644
--- a/libs/binder/ndk/include_platform/android/binder_parcel_platform.h
+++ b/libs/binder/ndk/include_platform/android/binder_parcel_platform.h
@@ -33,4 +33,15 @@ __BEGIN_DECLS
*/
bool AParcel_getAllowFds(const AParcel*);
+/**
+ * Data written to the parcel will be zero'd before being deleted or realloced.
+ *
+ * The main use of this is marking a parcel that will be used in a transaction
+ * with FLAG_CLEAR_BUF. When FLAG_CLEAR_BUF is used, the reply parcel will
+ * automatically be marked as sensitive when it is created.
+ *
+ * \param parcel The parcel to clear associated data from.
+ */
+void AParcel_markSensitive(const AParcel* parcel);
+
__END_DECLS
diff --git a/libs/binder/ndk/libbinder_ndk.map.txt b/libs/binder/ndk/libbinder_ndk.map.txt
index 947cc98d55..6962f86dd4 100644
--- a/libs/binder/ndk/libbinder_ndk.map.txt
+++ b/libs/binder/ndk/libbinder_ndk.map.txt
@@ -121,15 +121,16 @@ LIBBINDER_NDK31 { # introduced=31
AServiceManager_registerLazyService; # llndk
AServiceManager_waitForService; # apex llndk
- AParcel_reset;
- AParcel_getDataSize;
AParcel_appendFrom;
AParcel_create;
+ AParcel_getDataSize;
+ AParcel_reset;
};
LIBBINDER_NDK_PLATFORM {
global:
AParcel_getAllowFds;
+ AParcel_markSensitive;
extern "C++" {
AIBinder_fromPlatformBinder*;
AIBinder_toPlatformBinder*;
diff --git a/libs/binder/ndk/parcel.cpp b/libs/binder/ndk/parcel.cpp
index 2f95318874..3e3eda11aa 100644
--- a/libs/binder/ndk/parcel.cpp
+++ b/libs/binder/ndk/parcel.cpp
@@ -226,6 +226,10 @@ int32_t AParcel_getDataPosition(const AParcel* parcel) {
return parcel->get()->dataPosition();
}
+void AParcel_markSensitive(const AParcel* parcel) {
+ return parcel->get()->markSensitive();
+}
+
binder_status_t AParcel_writeStrongBinder(AParcel* parcel, AIBinder* binder) {
sp<IBinder> writeBinder = binder != nullptr ? binder->getBinder() : nullptr;
return parcel->get()->writeStrongBinder(writeBinder);
diff --git a/libs/binder/rust/src/binder.rs b/libs/binder/rust/src/binder.rs
index 6d0a369b7a..037ee95683 100644
--- a/libs/binder/rust/src/binder.rs
+++ b/libs/binder/rust/src/binder.rs
@@ -33,8 +33,7 @@ pub type TransactionCode = u32;
/// Additional operation flags.
///
-/// Can be either 0 for a normal RPC, or [`IBinder::FLAG_ONEWAY`] for a
-/// one-way RPC.
+/// `IBinder::FLAG_*` values.
pub type TransactionFlags = u32;
/// Super-trait for Binder interfaces.
@@ -91,6 +90,8 @@ pub trait IBinder {
/// Corresponds to TF_ONE_WAY -- an asynchronous call.
const FLAG_ONEWAY: TransactionFlags = sys::FLAG_ONEWAY;
+ /// Corresponds to TF_CLEAR_BUF -- clear transaction buffers after call is made.
+ const FLAG_CLEAR_BUF: TransactionFlags = sys::FLAG_CLEAR_BUF;
/// Is this object still alive?
fn is_binder_alive(&self) -> bool;
diff --git a/libs/binder/rust/src/parcel.rs b/libs/binder/rust/src/parcel.rs
index 2c1e5a4b75..6c34824a5e 100644
--- a/libs/binder/rust/src/parcel.rs
+++ b/libs/binder/rust/src/parcel.rs
@@ -100,6 +100,14 @@ impl Parcel {
// Data serialization methods
impl Parcel {
+ /// Data written to parcelable is zero'd before being deleted or reallocated.
+ pub fn mark_sensitive(&mut self) {
+ unsafe {
+ // Safety: guaranteed to have a parcel object, and this method never fails
+ sys::AParcel_markSensitive(self.as_native())
+ }
+ }
+
/// Write a type that implements [`Serialize`] to the `Parcel`.
pub fn write<S: Serialize + ?Sized>(&mut self, parcelable: &S) -> Result<()> {
parcelable.serialize(self)
diff --git a/libs/binder/rust/sys/BinderBindings.hpp b/libs/binder/rust/sys/BinderBindings.hpp
index 3f20a4ff09..ef142b5cf8 100644
--- a/libs/binder/rust/sys/BinderBindings.hpp
+++ b/libs/binder/rust/sys/BinderBindings.hpp
@@ -18,6 +18,7 @@
#include <android/binder_ibinder_platform.h>
#include <android/binder_manager.h>
#include <android/binder_parcel.h>
+#include <android/binder_parcel_platform.h>
#include <android/binder_process.h>
#include <android/binder_shell.h>
#include <android/binder_status.h>
@@ -78,6 +79,7 @@ enum {
enum {
FLAG_ONEWAY = FLAG_ONEWAY,
+ FLAG_CLEAR_BUF = FLAG_CLEAR_BUF,
};
} // namespace consts
diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp
index 98f0868bca..ad4729d127 100644
--- a/libs/binder/tests/binderLibTest.cpp
+++ b/libs/binder/tests/binderLibTest.cpp
@@ -402,6 +402,14 @@ TEST_F(BinderLibTest, NopTransaction) {
EXPECT_EQ(NO_ERROR, ret);
}
+TEST_F(BinderLibTest, NopTransactionClear) {
+ status_t ret;
+ Parcel data, reply;
+ // make sure it accepts the transaction flag
+ ret = m_server->transact(BINDER_LIB_TEST_NOP_TRANSACTION, data, &reply, TF_CLEAR_BUF);
+ EXPECT_EQ(NO_ERROR, ret);
+}
+
TEST_F(BinderLibTest, Freeze) {
status_t ret;
Parcel data, reply, replypid;
generated by cgit v1.2.3-59-g8ed1b (git 2.39.0) at 2025-10-19 16:41:17 +0000