Check for misaligned read and write pointers

Do not allow reads or writes when one of the read or write pointers are
misaligned.

Test: adb sync && adb shell /data/nativetest64/fmq_test
Bug: 184963385
Merged-in: Iaf33d30b5601e838f8899e1dceb65c86a10566b0
Change-Id: Iaf33d30b5601e838f8899e1dceb65c86a10566b0

2 files changed, 54 insertions, 5 deletions

diff --git a/include/fmq/MessageQueueBase.h b/include/fmq/MessageQueueBase.h
index 2a29937..7d628a3 100644
--- a/include/fmq/MessageQueueBase.h
+++ b/include/fmq/MessageQueueBase.h
@@ -125,7 +125,7 @@ struct MessageQueueBase {
      *
      * @return Whether the write was successful.
      */
-    bool write(const T* data, size_t count);
+    __attribute__((noinline)) bool write(const T* data, size_t count);
 
     /**
      * Perform a blocking write of 'count' items into the FMQ using EventFlags.
@@ -180,7 +180,7 @@ struct MessageQueueBase {
      *
      * @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
@@ -384,7 +384,7 @@ struct MessageQueueBase {
      * @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().
@@ -408,7 +408,7 @@ struct MessageQueueBase {
      * @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().
@@ -1069,6 +1069,12 @@ bool MessageQueueBase<MQDescriptorType, T, flavor>::beginWrite(size_t nMessages,
     }
 
     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();
 
     /*
@@ -1154,6 +1160,12 @@ MessageQueueBase<MQDescriptorType, T, flavor>::beginRead(size_t nMessages,
      * 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 a971f7d..d8c1e49 100644
--- a/tests/msgq_test_client.cpp
+++ b/tests/msgq_test_client.cpp
@@ -59,7 +59,8 @@ typedef android::AidlMessageQueue<int32_t, UnsynchronizedWrite> AidlMessageQueue
 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(uint16_t);
 
 enum class SetupType {
     SINGLE_FD,
@@ -643,6 +644,42 @@ TYPED_TEST(SynchronizedReadWriteClient, SmallInputReaderTest1) {
 }
 
 /*
+ * 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.