[automerger skipped] Check for misaligned read and write pointers am: 4ed31d5a6c am: 63f0ea080c -s ours am: 9034ba8849 -s ours
am skip reason: Merged-In Iaf33d30b5601e838f8899e1dceb65c86a10566b0 with SHA-1 4ed31d5a6c is already in history
Original change: https://googleplex-android-review.googlesource.com/c/platform/system/libfmq/+/14238137
Change-Id: Id9317e40c2118c90deb47407511923e7570b5ddd
diff --git a/PREUPLOAD.cfg b/PREUPLOAD.cfg
index 213c93a..18b0a1b 100644
--- a/PREUPLOAD.cfg
+++ b/PREUPLOAD.cfg
@@ -3,3 +3,4 @@
[Builtin Hooks]
clang_format = true
+bpfmt = true
diff --git a/fuzzer/Android.bp b/fuzzer/Android.bp
new file mode 100644
index 0000000..6b41e3f
--- /dev/null
+++ b/fuzzer/Android.bp
@@ -0,0 +1,66 @@
+// Copyright (C) 2021 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.
+
+package {
+ default_applicable_licenses: ["Android-Apache-2.0"],
+}
+
+cc_fuzz {
+ name: "fmq_fuzzer",
+
+ defaults: [
+ "libbinder_ndk_host_user",
+ ],
+
+ srcs: [
+ "fmq_fuzzer.cpp",
+ ],
+
+ static_libs: [
+ "libfmq",
+ "android.hardware.common.fmq-V1-ndk_platform",
+ ],
+
+ shared_libs: [
+ "libbase",
+ "libcutils",
+ "libhidlbase",
+ "liblog",
+ "libutils",
+ ],
+
+ fuzz_config: {
+ cc: [
+ "devinmoore@google.com",
+ "smoreland@google.com",
+ ],
+ componentid: 655781,
+ libfuzzer_options: [
+ "max_len=50000",
+ ],
+ },
+
+ host_supported: true,
+
+ sanitize: {
+ hwaddress: true,
+ scs: true,
+ cfi: true,
+ memtag_heap: true,
+ // undefined behavior is expected
+ all_undefined: false,
+ // integer overflow is expected
+ integer_overflow: false,
+ },
+}
diff --git a/fuzzer/fmq_fuzzer.cpp b/fuzzer/fmq_fuzzer.cpp
new file mode 100644
index 0000000..844188f
--- /dev/null
+++ b/fuzzer/fmq_fuzzer.cpp
@@ -0,0 +1,195 @@
+/*
+ * Copyright (C) 2021 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 <stddef.h>
+#include <stdint.h>
+#include <iostream>
+#include <limits>
+#include <thread>
+
+#include <android-base/logging.h>
+#include <fmq/AidlMessageQueue.h>
+#include <fmq/ConvertMQDescriptors.h>
+#include <fmq/EventFlag.h>
+#include <fmq/MessageQueue.h>
+
+#include "fuzzer/FuzzedDataProvider.h"
+
+using aidl::android::hardware::common::fmq::SynchronizedReadWrite;
+using aidl::android::hardware::common::fmq::UnsynchronizedWrite;
+using android::hardware::kSynchronizedReadWrite;
+using android::hardware::kUnsynchronizedWrite;
+
+typedef int32_t payload_t;
+
+/*
+ * MessageQueueBase.h contains asserts when memory allocation fails. So we need
+ * to set a reasonable limit if we want to avoid those asserts.
+ */
+static constexpr size_t kAlignment = 8;
+static constexpr size_t kMaxNumElements = PAGE_SIZE * 10 / sizeof(payload_t) - kAlignment + 1;
+
+/*
+ * The read counter can be found in the shared memory 16 bytes before the start
+ * of the ring buffer.
+ */
+static constexpr int kReadCounterOffsetBytes = 16;
+/*
+ * The write counter can be found in the shared memory 8 bytes before the start
+ * of the ring buffer.
+ */
+static constexpr int kWriteCounterOffsetBytes = 8;
+
+static constexpr int kMaxNumSyncReaders = 1;
+static constexpr int kMaxNumUnsyncReaders = 5;
+
+typedef android::AidlMessageQueue<payload_t, SynchronizedReadWrite> AidlMessageQueueSync;
+typedef android::AidlMessageQueue<payload_t, UnsynchronizedWrite> AidlMessageQueueUnsync;
+typedef android::hardware::MessageQueue<payload_t, kSynchronizedReadWrite> MessageQueueSync;
+typedef android::hardware::MessageQueue<payload_t, kUnsynchronizedWrite> MessageQueueUnsync;
+typedef aidl::android::hardware::common::fmq::MQDescriptor<payload_t, SynchronizedReadWrite>
+ AidlMQDescSync;
+typedef aidl::android::hardware::common::fmq::MQDescriptor<payload_t, UnsynchronizedWrite>
+ AidlMQDescUnsync;
+typedef android::hardware::MQDescriptorSync<payload_t> MQDescSync;
+typedef android::hardware::MQDescriptorUnsync<payload_t> MQDescUnsync;
+
+template <typename Queue, typename Desc>
+void reader(const Desc& desc, std::vector<uint8_t> readerData) {
+ Queue readMq(desc);
+ if (!readMq.isValid()) {
+ LOG(ERROR) << "read mq invalid";
+ return;
+ }
+ FuzzedDataProvider fdp(&readerData[0], readerData.size());
+ while (fdp.remaining_bytes()) {
+ typename Queue::MemTransaction tx;
+ size_t numElements = fdp.ConsumeIntegralInRange<size_t>(0, kMaxNumElements);
+ if (!readMq.beginRead(numElements, &tx)) {
+ continue;
+ }
+ const auto& region = tx.getFirstRegion();
+ payload_t* firstStart = region.getAddress();
+
+ // TODO add the debug function to get pointer to the ring buffer
+ uint64_t* writeCounter = reinterpret_cast<uint64_t*>(
+ reinterpret_cast<uint8_t*>(firstStart) - kWriteCounterOffsetBytes);
+ *writeCounter = fdp.ConsumeIntegral<uint64_t>();
+
+ (void)std::to_string(*firstStart);
+
+ readMq.commitRead(numElements);
+ }
+}
+
+template <typename Queue>
+void writer(Queue& writeMq, FuzzedDataProvider& fdp) {
+ while (fdp.remaining_bytes()) {
+ typename Queue::MemTransaction tx;
+ size_t numElements = 1;
+ if (!writeMq.beginWrite(numElements, &tx)) {
+ // need to consume something so we don't end up looping forever
+ fdp.ConsumeIntegral<uint8_t>();
+ continue;
+ }
+
+ const auto& region = tx.getFirstRegion();
+ payload_t* firstStart = region.getAddress();
+
+ // TODO add the debug function to get pointer to the ring buffer
+ uint64_t* readCounter = reinterpret_cast<uint64_t*>(reinterpret_cast<uint8_t*>(firstStart) -
+ kReadCounterOffsetBytes);
+ *readCounter = fdp.ConsumeIntegral<uint64_t>();
+
+ *firstStart = fdp.ConsumeIntegral<payload_t>();
+
+ writeMq.commitWrite(numElements);
+ }
+}
+
+template <typename Queue, typename Desc>
+void fuzzAidlWithReaders(std::vector<uint8_t>& writerData,
+ std::vector<std::vector<uint8_t>>& readerData) {
+ FuzzedDataProvider fdp(&writerData[0], writerData.size());
+ Queue writeMq(fdp.ConsumeIntegralInRange<size_t>(1, kMaxNumElements), fdp.ConsumeBool());
+ if (!writeMq.isValid()) {
+ LOG(ERROR) << "AIDL write mq invalid";
+ return;
+ }
+ const auto desc = writeMq.dupeDesc();
+ CHECK(desc.handle.fds[0].get() != -1);
+
+ std::vector<std::thread> clients;
+ for (int i = 0; i < readerData.size(); i++) {
+ clients.emplace_back(reader<Queue, Desc>, std::ref(desc), std::ref(readerData[i]));
+ }
+
+ writer<Queue>(writeMq, fdp);
+
+ for (auto& client : clients) {
+ client.join();
+ }
+}
+
+template <typename Queue, typename Desc>
+void fuzzHidlWithReaders(std::vector<uint8_t>& writerData,
+ std::vector<std::vector<uint8_t>>& readerData) {
+ FuzzedDataProvider fdp(&writerData[0], writerData.size());
+ Queue writeMq(fdp.ConsumeIntegralInRange<size_t>(1, kMaxNumElements), fdp.ConsumeBool());
+ if (!writeMq.isValid()) {
+ LOG(ERROR) << "HIDL write mq invalid";
+ return;
+ }
+ const auto desc = writeMq.getDesc();
+ CHECK(desc->isHandleValid());
+
+ std::vector<std::thread> clients;
+ for (int i = 0; i < readerData.size(); i++) {
+ clients.emplace_back(reader<Queue, Desc>, std::ref(*desc), std::ref(readerData[i]));
+ }
+
+ writer<Queue>(writeMq, fdp);
+
+ for (auto& client : clients) {
+ client.join();
+ }
+}
+
+extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
+ if (size < 1 || size > 50000) {
+ return 0;
+ }
+ FuzzedDataProvider fdp(data, size);
+
+ bool fuzzSync = fdp.ConsumeBool();
+ std::vector<std::vector<uint8_t>> readerData;
+ uint8_t numReaders = fuzzSync ? fdp.ConsumeIntegralInRange<uint8_t>(0, kMaxNumSyncReaders)
+ : fdp.ConsumeIntegralInRange<uint8_t>(0, kMaxNumUnsyncReaders);
+ for (int i = 0; i < numReaders; i++) {
+ readerData.emplace_back(fdp.ConsumeBytes<uint8_t>(5));
+ }
+ std::vector<uint8_t> writerData = fdp.ConsumeRemainingBytes<uint8_t>();
+
+ if (fuzzSync) {
+ fuzzHidlWithReaders<MessageQueueSync, MQDescSync>(writerData, readerData);
+ fuzzAidlWithReaders<AidlMessageQueueSync, AidlMQDescSync>(writerData, readerData);
+ } else {
+ fuzzHidlWithReaders<MessageQueueUnsync, MQDescUnsync>(writerData, readerData);
+ fuzzAidlWithReaders<AidlMessageQueueUnsync, AidlMQDescUnsync>(writerData, readerData);
+ }
+
+ return 0;
+}
diff --git a/include/fmq/MessageQueueBase.h b/include/fmq/MessageQueueBase.h
index 7d628a3..9bf20e4 100644
--- a/include/fmq/MessageQueueBase.h
+++ b/include/fmq/MessageQueueBase.h
@@ -125,7 +125,7 @@
*
* @return Whether the write was successful.
*/
- __attribute__((noinline)) bool write(const T* data, size_t count);
+ bool write(const T* data, size_t count);
/**
* Perform a blocking write of 'count' items into the FMQ using EventFlags.
@@ -180,7 +180,7 @@
*
* @return Whether the read was successful.
*/
- __attribute__((noinline)) bool read(T* data, size_t count);
+ bool read(T* data, size_t count);
/**
* Perform a blocking read operation of 'count' items from the FMQ. Does not
@@ -384,7 +384,7 @@
* @return Whether it is possible to write 'nMessages' items of type T
* into the FMQ.
*/
- __attribute__((always_inline)) bool beginWrite(size_t nMessages, MemTransaction* memTx) const;
+ bool beginWrite(size_t nMessages, MemTransaction* memTx) const;
/**
* Commit a write of size 'nMessages'. To be only used after a call to beginWrite().
@@ -408,7 +408,7 @@
* @return bool Whether it is possible to read 'nMessages' items of type T
* from the FMQ.
*/
- __attribute__((always_inline)) bool beginRead(size_t nMessages, MemTransaction* memTx) const;
+ bool beginRead(size_t nMessages, MemTransaction* memTx) const;
/**
* Commit a read of size 'nMessages'. To be only used after a call to beginRead().
@@ -1070,9 +1070,9 @@
auto writePtr = mWritePtr->load(std::memory_order_relaxed);
if (writePtr % sizeof(T) != 0) {
- hardware::details::logError("The write pointer has become misaligned. "
- "Writing to the queue is no longer "
- "possible.");
+ hardware::details::logError(
+ "The write pointer has become misaligned. Writing to the queue is no longer "
+ "possible.");
return false;
}
size_t writeOffset = writePtr % mDesc->getSize();
@@ -1161,9 +1161,9 @@
*/
auto readPtr = mReadPtr->load(std::memory_order_relaxed);
if (writePtr % sizeof(T) != 0 || readPtr % sizeof(T) != 0) {
- hardware::details::logError("The write or read pointer has become "
- "misaligned. Reading from the queue is no "
- "longer possible.");
+ hardware::details::logError(
+ "The write or read pointer has become misaligned. Reading from the queue is no "
+ "longer possible.");
return false;
}
diff --git a/tests/msgq_test_client.cpp b/tests/msgq_test_client.cpp
index 5ff0f00..a6f1ccc 100644
--- a/tests/msgq_test_client.cpp
+++ b/tests/msgq_test_client.cpp
@@ -59,8 +59,7 @@
typedef android::hardware::MessageQueue<int32_t, kSynchronizedReadWrite> MessageQueueSync;
typedef android::hardware::MessageQueue<int32_t, kUnsynchronizedWrite> MessageQueueUnsync;
static const std::string kServiceName = "BnTestAidlMsgQ";
-static constexpr size_t kNumElementsInSyncQueue =
- (PAGE_SIZE - 16) / sizeof(uint16_t);
+static constexpr size_t kNumElementsInSyncQueue = (PAGE_SIZE - 16) / sizeof(int32_t);
enum class SetupType {
SINGLE_FD,
@@ -656,20 +655,20 @@
GTEST_SKIP();
}
const size_t dataLen = 1;
+ ASSERT_LE(dataLen, kNumElementsInSyncQueue);
bool ret = this->requestWriteFmqSync(dataLen);
ASSERT_TRUE(ret);
- // begin read and get a MemTransaction object for the first object in the
- // queue
+ // begin read and get a MemTransaction object for the first object in the queue
typename TypeParam::MQType::MemTransaction tx;
ASSERT_TRUE(this->mQueue->beginRead(dataLen, &tx));
// get a pointer to the beginning of the ring buffer
- const auto ®ion = tx.getFirstRegion();
+ const auto& region = tx.getFirstRegion();
int32_t* firstStart = region.getAddress();
// because this is the first location in the ring buffer, we can get
// access to the read and write pointer stored in the fd. 8 bytes back for the
// write counter and 16 bytes back for the read counter
- uint64_t *writeCntr = (uint64_t *)((uint8_t *)firstStart - 8);
+ uint64_t* writeCntr = (uint64_t*)((uint8_t*)firstStart - 8);
// set it to point to the very last byte in the ring buffer
*(writeCntr) = this->mQueue->getQuantumCount() * this->mQueue->getQuantumSize() - 1;
@@ -678,8 +677,8 @@
// this is not actually necessary, but it's the expected the pattern.
this->mQueue->commitRead(dataLen);
- // This next write will be misaligned and will overlap outside of the ring
- // buffer. The write should fail.
+ // This next write will be misaligned and will overlap outside of the ring buffer.
+ // The write should fail.
ret = this->requestWriteFmqSync(dataLen);
EXPECT_FALSE(ret);
}