libhwbinder: support TF_CLEAR_BUF
This flag instructs the kernel to clear transactions from send/reply
buffers for certain transactions which may contain sensitive data, as a
security precaution.
Bug: 171501998
Test: hidl_test + verify by reading memory bits w/ updated kernel
Change-Id: I7dda8f8d24091f77bdaf99a7de446875356c601c
diff --git a/Android.bp b/Android.bp
index a768012..c75a350 100644
--- a/Android.bp
+++ b/Android.bp
@@ -48,6 +48,7 @@
"ProcessState.cpp",
"Static.cpp",
"TextOutput.cpp",
+ "Utils.cpp",
],
product_variables: {
diff --git a/Binder.cpp b/Binder.cpp
index 9edd27b..d86795c 100644
--- a/Binder.cpp
+++ b/Binder.cpp
@@ -110,6 +110,10 @@
{
data.setDataPosition(0);
+ if (reply != nullptr && (flags & FLAG_CLEAR_BUF)) {
+ reply->markSensitive();
+ }
+
status_t err = NO_ERROR;
switch (code) {
default:
diff --git a/IPCThreadState.cpp b/IPCThreadState.cpp
index fbb1705..6801490 100644
--- a/IPCThreadState.cpp
+++ b/IPCThreadState.cpp
@@ -1170,6 +1170,8 @@
<< reinterpret_cast<const size_t*>(tr.data.ptr.offsets) << endl;
}
+ constexpr size_t kForwardReplyFlags = TF_CLEAR_BUF;
+
auto reply_callback = [&] (auto &replyParcel) {
if (reply_sent) {
// Reply was sent earlier, ignore it.
@@ -1179,7 +1181,7 @@
reply_sent = true;
if ((tr.flags & TF_ONE_WAY) == 0) {
replyParcel.setError(NO_ERROR);
- sendReply(replyParcel, 0);
+ sendReply(replyParcel, (tr.flags & kForwardReplyFlags));
} else {
ALOGE("Not sending reply in one-way transaction");
}
@@ -1206,7 +1208,7 @@
// Should have been a reply but there wasn't, so there
// must have been an error instead.
reply.setError(error);
- sendReply(reply, 0);
+ sendReply(reply, (tr.flags & kForwardReplyFlags));
} else {
if (error != NO_ERROR) {
ALOGE("transact() returned error after sending reply.");
diff --git a/Parcel.cpp b/Parcel.cpp
index 8831604..57a99a1 100644
--- a/Parcel.cpp
+++ b/Parcel.cpp
@@ -45,6 +45,7 @@
#include "binder_kernel.h"
#include <hwbinder/Static.h>
#include "TextOutput.h"
+#include "Utils.h"
#include <atomic>
@@ -355,6 +356,11 @@
return err;
}
+void Parcel::markSensitive() const
+{
+ mDeallocZero = true;
+}
+
// Write RPC headers. (previously just the interface token)
status_t Parcel::writeInterfaceToken(const char* interface)
{
@@ -1689,6 +1695,9 @@
LOG_ALLOC("Parcel %p: freeing with %zu capacity", this, mDataCapacity);
gParcelGlobalAllocSize -= mDataCapacity;
gParcelGlobalAllocCount--;
+ if (mDeallocZero) {
+ zeroMemory(mData, mDataSize);
+ }
free(mData);
}
if (mObjects) free(mObjects);
@@ -1708,6 +1717,21 @@
return continueWrite(newSize);
}
+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) {
@@ -1721,7 +1745,7 @@
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;
@@ -1871,7 +1895,7 @@
// 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);
@@ -1938,6 +1962,7 @@
mHasFds = false;
mFdsKnown = true;
mAllowFds = true;
+ mDeallocZero = false;
mOwner = nullptr;
clearCache();
diff --git a/Utils.cpp b/Utils.cpp
new file mode 100644
index 0000000..5a29d6b
--- /dev/null
+++ b/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::hardware {
+
+void zeroMemory(uint8_t* data, size_t size) {
+ memset(data, 0, size);
+}
+
+} // namespace android::hardware
diff --git a/Utils.h b/Utils.h
new file mode 100644
index 0000000..07a5e69
--- /dev/null
+++ b/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::hardware {
+
+// avoid optimizations
+void zeroMemory(uint8_t* data, size_t size);
+
+} // namespace android::hardware
diff --git a/binder_kernel.h b/binder_kernel.h
index fdf5b1e..b8e00d3 100644
--- a/binder_kernel.h
+++ b/binder_kernel.h
@@ -17,10 +17,6 @@
#ifndef ANDROID_HARDWARE_BINDER_KERNEL_H
#define ANDROID_HARDWARE_BINDER_KERNEL_H
-/**
- * Only need this file to fix the __packed__ keyword.
- */
-
// TODO(b/31559095): bionic on host
#ifndef __ANDROID__
#define __packed __attribute__((__packed__))
@@ -28,4 +24,8 @@
#include <linux/android/binder.h>
+enum transaction_flags_ext {
+ TF_CLEAR_BUF = 0x20, /* clear buffer on txn complete */
+};
+
#endif // ANDROID_HARDWARE_BINDER_KERNEL_H
diff --git a/include/hwbinder/IBinder.h b/include/hwbinder/IBinder.h
index c54bef9..8340ca0 100644
--- a/include/hwbinder/IBinder.h
+++ b/include/hwbinder/IBinder.h
@@ -74,7 +74,11 @@
LAST_HIDL_TRANSACTION = 0x0fffffff,
// Corresponds to TF_ONE_WAY -- an asynchronous call.
- FLAG_ONEWAY = 0x00000001
+ FLAG_ONEWAY = 0x00000001,
+
+ // Corresponds to TF_CLEAR_BUF -- clear transaction buffers after call
+ // is made
+ FLAG_CLEAR_BUF = 0x00000020,
};
IBinder();
diff --git a/include/hwbinder/Parcel.h b/include/hwbinder/Parcel.h
index 8f1ae78..01b9b3f 100644
--- a/include/hwbinder/Parcel.h
+++ b/include/hwbinder/Parcel.h
@@ -66,6 +66,13 @@
status_t setData(const uint8_t* buffer, size_t len);
+ // 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 char* interface);
@@ -292,6 +299,10 @@
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;
};