Merge cherrypicks of [14554379, 14554380, 14554562, 14554565, 14554567, 14554524, 14554569, 14554525, 14554572, 14554574, 14554575, 14554577, 14554622, 14554628, 14554630, 14554631, 14554503, 14554632, 14554536, 14554633] into security-aosp-qt-release
Change-Id: I46b4d048fe99a135930307571550b94e2fe55264
diff --git a/include/fmq/MessageQueue.h b/include/fmq/MessageQueue.h
index 848b77f..7be7151 100644
--- a/include/fmq/MessageQueue.h
+++ b/include/fmq/MessageQueue.h
@@ -95,7 +95,7 @@
*
* @return Whether the write was successful.
*/
- bool write(const T* data);
+ __attribute__((always_inline)) bool write(const T* data);
/**
* Non-blocking read from FMQ.
@@ -115,7 +115,7 @@
*
* @return Whether the write was successful.
*/
- bool write(const T* data, size_t count);
+ __attribute__((always_inline)) bool write(const T* data, size_t count);
/**
* Perform a blocking write of 'count' items into the FMQ using EventFlags.
@@ -170,7 +170,7 @@
*
* @return Whether the read was successful.
*/
- bool read(T* data, size_t count);
+ __attribute__((noinline)) bool read(T* data, size_t count);
/**
* Perform a blocking read operation of 'count' items from the FMQ. Does not
@@ -378,7 +378,7 @@
* @return Whether it is possible to write 'nMessages' items of type T
* into the FMQ.
*/
- bool beginWrite(size_t nMessages, MemTransaction* memTx) const;
+ __attribute__((always_inline)) bool beginWrite(size_t nMessages, MemTransaction* memTx) const;
/**
* Commit a write of size 'nMessages'. To be only used after a call to beginWrite().
@@ -402,7 +402,7 @@
* @return bool Whether it is possible to read 'nMessages' items of type T
* from the FMQ.
*/
- bool beginRead(size_t nMessages, MemTransaction* memTx) const;
+ __attribute__((always_inline)) bool beginRead(size_t nMessages, MemTransaction* memTx) const;
/**
* Commit a read of size 'nMessages'. To be only used after a call to beginRead().
@@ -996,6 +996,12 @@
}
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.");
+ return false;
+ }
size_t writeOffset = writePtr % mDesc->getSize();
/*
@@ -1083,6 +1089,12 @@
* 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.");
+ return false;
+ }
if (writePtr - readPtr > mDesc->getSize()) {
mReadPtr->store(writePtr, std::memory_order_release);
diff --git a/tests/msgq_test_client.cpp b/tests/msgq_test_client.cpp
index ce7e340..271586a 100644
--- a/tests/msgq_test_client.cpp
+++ b/tests/msgq_test_client.cpp
@@ -72,7 +72,7 @@
}
virtual void SetUp() {
- static constexpr size_t kNumElementsInQueue = 1024;
+ static constexpr size_t kNumElementsInQueue = (PAGE_SIZE - 16) / sizeof(uint16_t);
mService = waitGetTestService();
ASSERT_NE(mService, nullptr);
ASSERT_TRUE(mService->isRemote());
@@ -424,6 +424,41 @@
}
/*
+ * 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.
+ */
+TEST_F(SynchronizedReadWriteClient, MisalignedWriteCounter) {
+ const size_t dataLen = 1;
+ bool ret = mService->requestWriteFmqSync(dataLen);
+ ASSERT_TRUE(ret);
+ // begin read and get a MemTransaction object for the first object in the queue
+ MessageQueueSync::MemTransaction tx;
+ ASSERT_TRUE(mQueue->beginRead(dataLen, &tx));
+ // get a pointer to the beginning of the ring buffer
+ const auto& region = tx.getFirstRegion();
+ uint16_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) = mQueue->getQuantumCount() * mQueue->getQuantumSize() - 1;
+ ASSERT_TRUE(*writeCntr % sizeof(uint16_t) != 0);
+
+ // this is not actually necessary, but it's the expected the pattern.
+ mQueue->commitRead(dataLen);
+
+ // This next write will be misaligned and will overlap outside of the ring buffer.
+ // The write should fail.
+ ret = mService->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.