Remove unused createEventFlag function am: 1390abc07c am: 651a52f908
Original change: https://android-review.googlesource.com/c/platform/system/libfmq/+/1792489
Change-Id: Ie68818396c3020a1c027e8a997365fcea0ac3334
diff --git a/FmqInternal.cpp b/FmqInternal.cpp
index eb8ae2d..6aad3aa 100644
--- a/FmqInternal.cpp
+++ b/FmqInternal.cpp
@@ -16,6 +16,7 @@
#define LOG_TAG "FMQ"
#include <android-base/logging.h>
+#include <utils/Log.h>
namespace android {
namespace hardware {
@@ -33,6 +34,10 @@
LOG(ERROR) << message;
}
+void errorWriteLog(int tag, const char* info) {
+ android_errorWriteLog(tag, info);
+}
+
} // namespace details
} // namespace hardware
} // namespace android
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/base/fmq/MQDescriptorBase.h b/base/fmq/MQDescriptorBase.h
index c70fe57..aef698b 100644
--- a/base/fmq/MQDescriptorBase.h
+++ b/base/fmq/MQDescriptorBase.h
@@ -54,6 +54,7 @@
namespace details {
void logError(const std::string& message);
+void errorWriteLog(int tag, const char* message);
void check(bool exp, const char* message);
typedef uint64_t RingBufferPosition;
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 d24dc7c..c34a4ff 100644
--- a/include/fmq/MessageQueueBase.h
+++ b/include/fmq/MessageQueueBase.h
@@ -1051,6 +1051,13 @@
}
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::errorWriteLog(0x534e4554, "184963385");
+ return false;
+ }
size_t writeOffset = writePtr % mDesc->getSize();
/*
@@ -1136,6 +1143,13 @@
* stores to mReadPtr from a different thread.
*/
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::errorWriteLog(0x534e4554, "184963385");
+ return false;
+ }
if (writePtr - readPtr > mDesc->getSize()) {
mReadPtr->store(writePtr, std::memory_order_release);
diff --git a/tests/aidl/Android.bp b/tests/aidl/Android.bp
index c5010ce..e210fda 100644
--- a/tests/aidl/Android.bp
+++ b/tests/aidl/Android.bp
@@ -11,8 +11,8 @@
"android/fmq/test/*.aidl",
],
imports: [
- "android.hardware.common",
- "android.hardware.common.fmq",
+ "android.hardware.common-V2",
+ "android.hardware.common.fmq-V1",
],
gen_trace: true,
backend: {
diff --git a/tests/msgq_test_client.cpp b/tests/msgq_test_client.cpp
index a971f7d..a6f1ccc 100644
--- a/tests/msgq_test_client.cpp
+++ b/tests/msgq_test_client.cpp
@@ -59,7 +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 = 1024;
+static constexpr size_t kNumElementsInSyncQueue = (PAGE_SIZE - 16) / sizeof(int32_t);
enum class SetupType {
SINGLE_FD,
@@ -643,6 +643,47 @@
}
/*
+ * Request mService to write a message to the queue followed by a beginRead().
+ * Get a pointer to the memory region for the that first message. Set the write
+ * counter to the last byte in the ring buffer. Request another write from
+ * mService. The write should fail because the write address is misaligned.
+ */
+TYPED_TEST(SynchronizedReadWriteClient, MisalignedWriteCounter) {
+ if (TypeParam::UserFd) {
+ // When using the second FD for the ring buffer, we can't get to the read/write
+ // counters from a pointer to the ring buffer, so no sense in testing.
+ 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
+ typename TypeParam::MQType::MemTransaction tx;
+ ASSERT_TRUE(this->mQueue->beginRead(dataLen, &tx));
+ // get a pointer to the beginning of the ring buffer
+ 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);
+
+ // set it to point to the very last byte in the ring buffer
+ *(writeCntr) = this->mQueue->getQuantumCount() * this->mQueue->getQuantumSize() - 1;
+ ASSERT_TRUE(*writeCntr % sizeof(int32_t) != 0);
+
+ // 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.
+ ret = this->requestWriteFmqSync(dataLen);
+ EXPECT_FALSE(ret);
+}
+
+/*
* Request mService to write a small number of messages
* to the FMQ. Read and verify each message using
* beginRead/Commit read APIs.