OWNERS += bug component am: f0933bad94 am: 3188ea20f0 am: 3ad01f0e6f
Original change: https://android-review.googlesource.com/c/platform/system/libfmq/+/2541031
Change-Id: I5c82d328ac89cb9317bdfda41198401f46042d15
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
diff --git a/fuzzer/fmq_fuzzer.cpp b/fuzzer/fmq_fuzzer.cpp
index 8c8a78e..1c92814 100644
--- a/fuzzer/fmq_fuzzer.cpp
+++ b/fuzzer/fmq_fuzzer.cpp
@@ -36,8 +36,8 @@
typedef int32_t payload_t;
-// The reader will wait for 10 ms
-static constexpr int kBlockingTimeoutNs = 10000000;
+// The reader/writers will wait during blocking calls
+static constexpr int kBlockingTimeoutNs = 100000;
/*
* MessageQueueBase.h contains asserts when memory allocation fails. So we need
@@ -45,6 +45,13 @@
*/
static constexpr size_t kAlignment = 8;
static constexpr size_t kMaxNumElements = PAGE_SIZE * 10 / sizeof(payload_t) - kAlignment + 1;
+/*
+ * limit the custom grantor case to one page of memory.
+ * If we want to increase this, we need to make sure that all of grantors offset
+ * plus extent are less than the size of the page aligned ashmem region that is
+ * created
+ */
+static constexpr size_t kMaxCustomGrantorMemoryBytes = PAGE_SIZE;
/*
* The read counter can be found in the shared memory 16 bytes before the start
@@ -72,8 +79,40 @@
typedef android::hardware::MQDescriptorSync<payload_t> MQDescSync;
typedef android::hardware::MQDescriptorUnsync<payload_t> MQDescUnsync;
-static inline uint64_t* getCounterPtr(payload_t* start, int byteOffset) {
- return reinterpret_cast<uint64_t*>(reinterpret_cast<uint8_t*>(start) - byteOffset);
+// AIDL and HIDL have different ways of accessing the grantors
+template <typename Desc>
+uint64_t* getCounterPtr(payload_t* start, const Desc& desc, int grantorIndx);
+
+uint64_t* createCounterPtr(payload_t* start, uint32_t offset, uint32_t data_offset) {
+ // start is the address of the beginning of the FMQ data section in memory
+ // offset is overall offset of the counter in the FMQ memory
+ // data_offset is the overall offset of the data section in the FMQ memory
+ // start - (data_offset) = beginning address of the FMQ memory
+ return reinterpret_cast<uint64_t*>(reinterpret_cast<uint8_t*>(start) - data_offset + offset);
+}
+
+uint64_t* getCounterPtr(payload_t* start, const MQDescSync& desc, int grantorIndx) {
+ uint32_t offset = desc.grantors()[grantorIndx].offset;
+ uint32_t data_offset = desc.grantors()[android::hardware::details::DATAPTRPOS].offset;
+ return createCounterPtr(start, offset, data_offset);
+}
+
+uint64_t* getCounterPtr(payload_t* start, const MQDescUnsync& desc, int grantorIndx) {
+ uint32_t offset = desc.grantors()[grantorIndx].offset;
+ uint32_t data_offset = desc.grantors()[android::hardware::details::DATAPTRPOS].offset;
+ return createCounterPtr(start, offset, data_offset);
+}
+
+uint64_t* getCounterPtr(payload_t* start, const AidlMQDescSync& desc, int grantorIndx) {
+ uint32_t offset = desc.grantors[grantorIndx].offset;
+ uint32_t data_offset = desc.grantors[android::hardware::details::DATAPTRPOS].offset;
+ return createCounterPtr(start, offset, data_offset);
+}
+
+uint64_t* getCounterPtr(payload_t* start, const AidlMQDescUnsync& desc, int grantorIndx) {
+ uint32_t offset = desc.grantors[grantorIndx].offset;
+ uint32_t data_offset = desc.grantors[android::hardware::details::DATAPTRPOS].offset;
+ return createCounterPtr(start, offset, data_offset);
}
template <typename Queue, typename Desc>
@@ -84,7 +123,7 @@
return;
}
FuzzedDataProvider fdp(&readerData[0], readerData.size());
- payload_t* ring = nullptr;
+ payload_t* ring = reinterpret_cast<payload_t*>(readMq.getRingBufferPtr());
while (fdp.remaining_bytes()) {
typename Queue::MemTransaction tx;
size_t numElements = fdp.ConsumeIntegralInRange<size_t>(0, kMaxNumElements);
@@ -97,11 +136,9 @@
// the ring buffer is only next to the read/write counters when there is
// no user supplied fd
if (!userFd) {
- if (ring == nullptr) {
- ring = firstStart;
- }
if (fdp.ConsumeIntegral<uint8_t>() == 1) {
- uint64_t* writeCounter = getCounterPtr(ring, kWriteCounterOffsetBytes);
+ uint64_t* writeCounter =
+ getCounterPtr(ring, desc, android::hardware::details::WRITEPTRPOS);
*writeCounter = fdp.ConsumeIntegral<uint64_t>();
}
}
@@ -124,7 +161,7 @@
FuzzedDataProvider fdp(&readerData[0], readerData.size());
do {
size_t count = fdp.remaining_bytes()
- ? fdp.ConsumeIntegralInRange<size_t>(1, readMq.getQuantumCount())
+ ? fdp.ConsumeIntegralInRange<size_t>(0, readMq.getQuantumCount() + 1)
: 1;
std::vector<payload_t> data;
data.resize(count);
@@ -142,9 +179,9 @@
void readerBlocking<MessageQueueUnsync, MQDescUnsync>(const MQDescUnsync&, std::vector<uint8_t>&,
std::atomic<size_t>&, std::atomic<size_t>&) {}
-template <typename Queue>
-void writer(Queue& writeMq, FuzzedDataProvider& fdp, bool userFd) {
- payload_t* ring = nullptr;
+template <typename Queue, typename Desc>
+void writer(const Desc& desc, Queue& writeMq, FuzzedDataProvider& fdp, bool userFd) {
+ payload_t* ring = reinterpret_cast<payload_t*>(writeMq.getRingBufferPtr());
while (fdp.remaining_bytes()) {
typename Queue::MemTransaction tx;
size_t numElements = 1;
@@ -159,11 +196,9 @@
// the ring buffer is only next to the read/write counters when there is
// no user supplied fd
if (!userFd) {
- if (ring == nullptr) {
- ring = firstStart;
- }
if (fdp.ConsumeIntegral<uint8_t>() == 1) {
- uint64_t* readCounter = getCounterPtr(ring, kReadCounterOffsetBytes);
+ uint64_t* readCounter =
+ getCounterPtr(ring, desc, android::hardware::details::READPTRPOS);
*readCounter = fdp.ConsumeIntegral<uint64_t>();
}
}
@@ -179,7 +214,7 @@
std::atomic<size_t>& readersNotFinished) {
android::base::ScopeGuard guard([&writersNotFinished]() { writersNotFinished--; });
while (fdp.remaining_bytes() > sizeof(size_t) && readersNotFinished > 0) {
- size_t count = fdp.ConsumeIntegralInRange<size_t>(1, writeMq.getQuantumCount());
+ size_t count = fdp.ConsumeIntegralInRange<size_t>(0, writeMq.getQuantumCount() + 1);
std::vector<payload_t> data;
for (int i = 0; i < count; i++) {
data.push_back(fdp.ConsumeIntegral<payload_t>());
@@ -197,74 +232,132 @@
std::atomic<size_t>&, std::atomic<size_t>&) {}
template <typename Queue, typename Desc>
-void fuzzAidlWithReaders(std::vector<uint8_t>& writerData,
- std::vector<std::vector<uint8_t>>& readerData, bool blocking) {
- FuzzedDataProvider fdp(&writerData[0], writerData.size());
- bool evFlag = blocking || fdp.ConsumeBool();
- android::base::unique_fd dataFd;
- size_t bufferSize = 0;
- size_t numElements = fdp.ConsumeIntegralInRange<size_t>(1, kMaxNumElements);
- bool userFd = fdp.ConsumeBool();
- if (userFd) {
- // run test with our own data region
- bufferSize = numElements * sizeof(payload_t);
- dataFd.reset(::ashmem_create_region("SyncReadWrite", bufferSize));
- }
- Queue writeMq(numElements, evFlag, std::move(dataFd), bufferSize);
- if (!writeMq.isValid()) {
- LOG(ERROR) << "AIDL write mq invalid";
- return;
- }
- const auto desc = writeMq.dupeDesc();
- CHECK(desc.handle.fds[0].get() != -1);
+inline std::optional<Desc> getDesc(std::unique_ptr<Queue>& queue, FuzzedDataProvider& fdp);
- std::atomic<size_t> readersNotFinished = readerData.size();
- std::atomic<size_t> writersNotFinished = 1;
- std::vector<std::thread> readers;
- for (int i = 0; i < readerData.size(); i++) {
- if (blocking) {
- readers.emplace_back(readerBlocking<Queue, Desc>, std::ref(desc),
- std::ref(readerData[i]), std::ref(readersNotFinished),
- std::ref(writersNotFinished));
-
+template <typename Queue, typename Desc>
+inline std::optional<Desc> getAidlDesc(std::unique_ptr<Queue>& queue, FuzzedDataProvider& fdp) {
+ if (queue) {
+ // get the existing descriptor from the queue
+ Desc desc = queue->dupeDesc();
+ if (desc.handle.fds[0].get() == -1) {
+ return std::nullopt;
} else {
- readers.emplace_back(reader<Queue, Desc>, std::ref(desc), std::ref(readerData[i]),
- userFd);
+ return std::make_optional(std::move(desc));
}
- }
-
- if (blocking) {
- writerBlocking<Queue>(writeMq, fdp, writersNotFinished, readersNotFinished);
} else {
- writer<Queue>(writeMq, fdp, userFd);
- }
+ // create a custom descriptor
+ std::vector<aidl::android::hardware::common::fmq::GrantorDescriptor> grantors;
+ size_t numGrantors = fdp.ConsumeIntegralInRange<size_t>(0, 4);
+ for (int i = 0; i < numGrantors; i++) {
+ grantors.push_back({fdp.ConsumeIntegralInRange<int32_t>(-2, 2) /* fdIndex */,
+ fdp.ConsumeIntegralInRange<int32_t>(
+ 0, kMaxCustomGrantorMemoryBytes) /* offset */,
+ fdp.ConsumeIntegralInRange<int64_t>(
+ 0, kMaxCustomGrantorMemoryBytes) /* extent */});
+ // ashmem region is PAGE_SIZE and we need to make sure all of the
+ // pointers and data region fit inside
+ if (grantors.back().offset + grantors.back().extent > PAGE_SIZE) return std::nullopt;
+ }
- for (auto& reader : readers) {
- reader.join();
+ android::base::unique_fd fd(
+ ashmem_create_region("AidlCustomGrantors", kMaxCustomGrantorMemoryBytes));
+ ashmem_set_prot_region(fd, PROT_READ | PROT_WRITE);
+ aidl::android::hardware::common::NativeHandle handle;
+ handle.fds.emplace_back(fd.get());
+
+ return std::make_optional<Desc>(
+ {grantors, std::move(handle), sizeof(payload_t), fdp.ConsumeBool()});
}
}
+template <>
+inline std::optional<AidlMQDescSync> getDesc(std::unique_ptr<AidlMessageQueueSync>& queue,
+ FuzzedDataProvider& fdp) {
+ return getAidlDesc<AidlMessageQueueSync, AidlMQDescSync>(queue, fdp);
+}
+
+template <>
+inline std::optional<AidlMQDescUnsync> getDesc(std::unique_ptr<AidlMessageQueueUnsync>& queue,
+ FuzzedDataProvider& fdp) {
+ return getAidlDesc<AidlMessageQueueUnsync, AidlMQDescUnsync>(queue, fdp);
+}
+
template <typename Queue, typename Desc>
-void fuzzHidlWithReaders(std::vector<uint8_t>& writerData,
- std::vector<std::vector<uint8_t>>& readerData, bool blocking) {
+inline std::optional<Desc> getHidlDesc(std::unique_ptr<Queue>& queue, FuzzedDataProvider& fdp) {
+ if (queue) {
+ auto desc = queue->getDesc();
+ if (!desc->isHandleValid()) {
+ return std::nullopt;
+ } else {
+ return std::make_optional(std::move(*desc));
+ }
+ } else {
+ // create a custom descriptor
+ std::vector<android::hardware::GrantorDescriptor> grantors;
+ size_t numGrantors = fdp.ConsumeIntegralInRange<size_t>(0, 4);
+ for (int i = 0; i < numGrantors; i++) {
+ grantors.push_back({fdp.ConsumeIntegral<uint32_t>() /* flags */,
+ fdp.ConsumeIntegralInRange<uint32_t>(0, 2) /* fdIndex */,
+ fdp.ConsumeIntegralInRange<uint32_t>(
+ 0, kMaxCustomGrantorMemoryBytes) /* offset */,
+ fdp.ConsumeIntegralInRange<uint64_t>(
+ 0, kMaxCustomGrantorMemoryBytes) /* extent */});
+ // ashmem region is PAGE_SIZE and we need to make sure all of the
+ // pointers and data region fit inside
+ if (grantors.back().offset + grantors.back().extent > PAGE_SIZE) return std::nullopt;
+ }
+
+ native_handle_t* handle = native_handle_create(1, 0);
+ int ashmemFd = ashmem_create_region("HidlCustomGrantors", kMaxCustomGrantorMemoryBytes);
+ ashmem_set_prot_region(ashmemFd, PROT_READ | PROT_WRITE);
+ handle->data[0] = ashmemFd;
+
+ return std::make_optional<Desc>(grantors, handle, sizeof(payload_t));
+ }
+}
+
+template <>
+inline std::optional<MQDescSync> getDesc(std::unique_ptr<MessageQueueSync>& queue,
+ FuzzedDataProvider& fdp) {
+ return getHidlDesc<MessageQueueSync, MQDescSync>(queue, fdp);
+}
+
+template <>
+inline std::optional<MQDescUnsync> getDesc(std::unique_ptr<MessageQueueUnsync>& queue,
+ FuzzedDataProvider& fdp) {
+ return getHidlDesc<MessageQueueUnsync, MQDescUnsync>(queue, fdp);
+}
+
+template <typename Queue, typename Desc>
+void fuzzWithReaders(std::vector<uint8_t>& writerData,
+ std::vector<std::vector<uint8_t>>& readerData, bool blocking) {
FuzzedDataProvider fdp(&writerData[0], writerData.size());
bool evFlag = blocking || fdp.ConsumeBool();
- android::base::unique_fd dataFd;
- size_t bufferSize = 0;
size_t numElements = fdp.ConsumeIntegralInRange<size_t>(1, kMaxNumElements);
+ size_t bufferSize = numElements * sizeof(payload_t);
bool userFd = fdp.ConsumeBool();
- if (userFd) {
- // run test with our own data region
- bufferSize = numElements * sizeof(payload_t);
- dataFd.reset(::ashmem_create_region("SyncReadWrite", bufferSize));
+ bool manualGrantors = fdp.ConsumeBool();
+ std::unique_ptr<Queue> writeMq = nullptr;
+ if (manualGrantors) {
+ std::optional<Desc> customDesc(getDesc<Queue, Desc>(writeMq, fdp));
+ if (customDesc) {
+ writeMq = std::make_unique<Queue>(*customDesc);
+ }
+ } else {
+ android::base::unique_fd dataFd;
+ if (userFd) {
+ // run test with our own data region
+ dataFd.reset(::ashmem_create_region("CustomData", bufferSize));
+ }
+ writeMq = std::make_unique<Queue>(numElements, evFlag, std::move(dataFd), bufferSize);
}
- Queue writeMq(numElements, evFlag, std::move(dataFd), bufferSize);
- if (!writeMq.isValid()) {
- LOG(ERROR) << "HIDL write mq invalid";
+
+ if (writeMq == nullptr || !writeMq->isValid()) {
return;
}
- const auto desc = writeMq.getDesc();
- CHECK(desc->isHandleValid());
+ // get optional desc
+ const std::optional<Desc> desc(std::move(getDesc<Queue, Desc>(writeMq, fdp)));
+ CHECK(desc != std::nullopt);
std::atomic<size_t> readersNotFinished = readerData.size();
std::atomic<size_t> writersNotFinished = 1;
@@ -281,9 +374,9 @@
}
if (blocking) {
- writerBlocking<Queue>(writeMq, fdp, writersNotFinished, readersNotFinished);
+ writerBlocking<Queue>(*writeMq, fdp, writersNotFinished, readersNotFinished);
} else {
- writer<Queue>(writeMq, fdp, userFd);
+ writer<Queue>(*desc, *writeMq, fdp, userFd);
}
for (auto& reader : readers) {
@@ -307,13 +400,11 @@
bool fuzzBlocking = fdp.ConsumeBool();
std::vector<uint8_t> writerData = fdp.ConsumeRemainingBytes<uint8_t>();
if (fuzzSync) {
- fuzzHidlWithReaders<MessageQueueSync, MQDescSync>(writerData, readerData, fuzzBlocking);
- fuzzAidlWithReaders<AidlMessageQueueSync, AidlMQDescSync>(writerData, readerData,
- fuzzBlocking);
+ fuzzWithReaders<MessageQueueSync, MQDescSync>(writerData, readerData, fuzzBlocking);
+ fuzzWithReaders<AidlMessageQueueSync, AidlMQDescSync>(writerData, readerData, fuzzBlocking);
} else {
- fuzzHidlWithReaders<MessageQueueUnsync, MQDescUnsync>(writerData, readerData, false);
- fuzzAidlWithReaders<AidlMessageQueueUnsync, AidlMQDescUnsync>(writerData, readerData,
- false);
+ fuzzWithReaders<MessageQueueUnsync, MQDescUnsync>(writerData, readerData, false);
+ fuzzWithReaders<AidlMessageQueueUnsync, AidlMQDescUnsync>(writerData, readerData, false);
}
return 0;
diff --git a/include/fmq/MessageQueueBase.h b/include/fmq/MessageQueueBase.h
index f4bf7e2..f99e335 100644
--- a/include/fmq/MessageQueueBase.h
+++ b/include/fmq/MessageQueueBase.h
@@ -421,6 +421,11 @@
*/
bool commitRead(size_t nMessages);
+ /**
+ * Get the pointer to the ring buffer. Useful for debugging and fuzzing.
+ */
+ uint8_t* getRingBufferPtr() const { return mRing; }
+
private:
size_t availableToWriteBytes() const;
size_t availableToReadBytes() const;
@@ -643,7 +648,8 @@
MessageQueueBase<MQDescriptorType, T, flavor>::MessageQueueBase(const Descriptor& Desc,
bool resetPointers) {
mDesc = std::unique_ptr<Descriptor>(new (std::nothrow) Descriptor(Desc));
- if (mDesc == nullptr) {
+ if (mDesc == nullptr || mDesc->getSize() == 0) {
+ hardware::details::logError("MQDescriptor is invalid or queue size is 0.");
return;
}
@@ -662,6 +668,10 @@
". Number of elements: " + std::to_string(numElementsInQueue));
return;
}
+ if (numElementsInQueue == 0) {
+ hardware::details::logError("Requested queue size of 0.");
+ return;
+ }
if (bufferFd != -1 && numElementsInQueue * sizeof(T) > bufferSize) {
hardware::details::logError("The supplied buffer size(" + std::to_string(bufferSize) +
") is smaller than the required size(" +
@@ -767,10 +777,10 @@
template <template <typename, MQFlavor> typename MQDescriptorType, typename T, MQFlavor flavor>
MessageQueueBase<MQDescriptorType, T, flavor>::~MessageQueueBase() {
- if (flavor == kUnsynchronizedWrite && mReadPtr != nullptr) {
- delete mReadPtr;
- } else if (mReadPtr != nullptr) {
+ if (flavor == kSynchronizedReadWrite && mReadPtr != nullptr) {
unmapGrantorDescr(mReadPtr, hardware::details::READPTRPOS);
+ } else if (mReadPtr != nullptr) {
+ delete mReadPtr;
}
if (mWritePtr != nullptr) {
unmapGrantorDescr(mWritePtr, hardware::details::WRITEPTRPOS);
@@ -1278,6 +1288,29 @@
return nullptr;
}
+ /*
+ * Expect some grantors to be at least a min size
+ */
+ for (uint32_t i = 0; i < grantors.size(); i++) {
+ switch (i) {
+ case hardware::details::READPTRPOS:
+ if (grantors[i].extent < sizeof(uint64_t)) return nullptr;
+ break;
+ case hardware::details::WRITEPTRPOS:
+ if (grantors[i].extent < sizeof(uint64_t)) return nullptr;
+ break;
+ case hardware::details::DATAPTRPOS:
+ // We don't expect specific data size
+ break;
+ case hardware::details::EVFLAGWORDPOS:
+ if (grantors[i].extent < sizeof(uint32_t)) return nullptr;
+ break;
+ default:
+ // We don't care about unknown grantors
+ break;
+ }
+ }
+
int mapOffset = (grantors[grantorIdx].offset / PAGE_SIZE) * PAGE_SIZE;
if (grantors[grantorIdx].extent < 0 || grantors[grantorIdx].extent > INT_MAX - PAGE_SIZE) {
hardware::details::logError(std::string("Grantor (index " + std::to_string(grantorIdx) +
