Enabled fmq_fuzzer for presubmit am: 4f3d91fa0e am: 0945189cab am: e5fff193d7 am: 3f2d9f8d1e am: 92dbf85b85 Original change: https://android-review.googlesource.com/c/platform/system/libfmq/+/2743942 Change-Id: I9a5f31e563a79cca1d9e5d797ed79b4cc17dc703 Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
diff --git a/include/fmq/MessageQueueBase.h b/include/fmq/MessageQueueBase.h index 8144937..017e7fc 100644 --- a/include/fmq/MessageQueueBase.h +++ b/include/fmq/MessageQueueBase.h
@@ -1324,6 +1324,13 @@ void* address = mmap(0, mapLength, PROT_READ | PROT_WRITE, MAP_SHARED, handle->data[fdIndex], mapOffset); + if (address == MAP_FAILED && errno == EPERM && flavor == kUnsynchronizedWrite) { + // If the supplied memory is read-only, it would fail with EPERM. + // Try again to mmap read-only for the kUnsynchronizedWrite case. + // kSynchronizedReadWrite cannot use read-only memory because the + // read pointer is stored in the shared memory as well. + address = mmap(0, mapLength, PROT_READ, MAP_SHARED, handle->data[fdIndex], mapOffset); + } if (address == MAP_FAILED) { hardware::details::logError(std::string("mmap failed: ") + std::to_string(errno)); return nullptr;
diff --git a/tests/aidl/default/TestAidlMsgQ.cpp b/tests/aidl/default/TestAidlMsgQ.cpp index 485e75b..d230993 100644 --- a/tests/aidl/default/TestAidlMsgQ.cpp +++ b/tests/aidl/default/TestAidlMsgQ.cpp
@@ -60,7 +60,14 @@ *_aidl_return = false; } else { *mqDesc = std::move(mFmqUnsynchronized->dupeDesc()); - *_aidl_return = true; + // set write-protection so readers can't mmap and write + int res = ashmem_set_prot_region(mqDesc->handle.fds[0].get(), PROT_READ); + if (res == -1) { + ALOGE("Failed to set write protection: %s", strerror(errno)); + *_aidl_return = false; + } else { + *_aidl_return = true; + } } return ndk::ScopedAStatus::ok();
diff --git a/tests/msgq_test_client.cpp b/tests/msgq_test_client.cpp index b55c368..8cab77e 100644 --- a/tests/msgq_test_client.cpp +++ b/tests/msgq_test_client.cpp
@@ -163,7 +163,7 @@ bool result = false; aidl::android::hardware::common::fmq::MQDescriptor<int32_t, UnsynchronizedWrite> desc; auto ret = service->getFmqUnsyncWrite(configureFmq, userFd, &desc, &result); - *queue = new (std::nothrow) AidlMessageQueueUnsync(desc); + *queue = new (std::nothrow) AidlMessageQueueUnsync(desc, false); return result && ret.isOk(); } @@ -187,7 +187,7 @@ } AidlMessageQueueUnsync* newQueue() { if (mQueue->isValid()) - return new (std::nothrow) AidlMessageQueueUnsync(mQueue->dupeDesc()); + return new (std::nothrow) AidlMessageQueueUnsync(mQueue->dupeDesc(), false); else return nullptr; } @@ -215,7 +215,7 @@ service->getFmqUnsyncWrite(configureFmq, userFd, [queue](bool ret, const MQDescriptorUnsync<int32_t>& in) { ASSERT_TRUE(ret); - *queue = new (std::nothrow) MessageQueueUnsync(in); + *queue = new (std::nothrow) MessageQueueUnsync(in, false); }); return true; } @@ -237,7 +237,7 @@ } MessageQueueUnsync* newQueue() { - return new (std::nothrow) MessageQueueUnsync(*mQueue->getDesc()); + return new (std::nothrow) MessageQueueUnsync(*mQueue->getDesc(), false); } sp<ITestMsgQ> mService; @@ -961,11 +961,8 @@ TYPED_TEST(UnsynchronizedWriteClient, SmallInputWriterTest1) { const size_t dataLen = 16; ASSERT_LE(dataLen, this->mNumMessagesMax); - int32_t data[dataLen]; - initData(data, dataLen); - ASSERT_TRUE(this->mQueue->write(data, dataLen)); - bool ret = this->requestReadFmqUnsync(dataLen, this->mService); - ASSERT_TRUE(ret); + ASSERT_TRUE(this->requestWriteFmqUnsync(dataLen, this->mService)); + ASSERT_TRUE(this->requestReadFmqUnsync(dataLen, this->mService)); } /* @@ -991,9 +988,9 @@ TYPED_TEST(UnsynchronizedWriteClient, WriteWhenFull) { std::vector<int32_t> data(this->mNumMessagesMax); initData(&data[0], this->mNumMessagesMax); - ASSERT_TRUE(this->mQueue->write(&data[0], this->mNumMessagesMax)); + ASSERT_TRUE(this->requestWriteFmqUnsync(this->mNumMessagesMax, this->mService)); ASSERT_EQ(0UL, this->mQueue->availableToWrite()); - ASSERT_TRUE(this->mQueue->write(&data[0], 1)); + ASSERT_TRUE(this->requestWriteFmqUnsync(1, this->mService)); bool ret = this->requestReadFmqUnsync(this->mNumMessagesMax, this->mService); ASSERT_FALSE(ret); } @@ -1028,23 +1025,20 @@ /* * Write until FMQ is full. - * Verify that the number of messages available to write - * is equal to this->mNumMessagesMax. * Verify that another write attempt is successful. - * Request this->mService to read. Verify that read is unsuccessful. + * Request this->mService to read. Verify that read is unsuccessful + * because of the write rollover. * Perform another write and verify that the read is successful * to check if the reader process can recover from the error condition. */ TYPED_TEST(UnsynchronizedWriteClient, LargeInputTest3) { - std::vector<int32_t> data(this->mNumMessagesMax); - initData(&data[0], this->mNumMessagesMax); - ASSERT_TRUE(this->mQueue->write(&data[0], this->mNumMessagesMax)); + ASSERT_TRUE(this->requestWriteFmqUnsync(this->mNumMessagesMax, this->mService)); ASSERT_EQ(0UL, this->mQueue->availableToWrite()); - ASSERT_TRUE(this->mQueue->write(&data[0], 1)); + ASSERT_TRUE(this->requestWriteFmqUnsync(1, this->mService)); bool ret = this->requestReadFmqUnsync(this->mNumMessagesMax, this->mService); ASSERT_FALSE(ret); - ASSERT_TRUE(this->mQueue->write(&data[0], this->mNumMessagesMax)); + ASSERT_TRUE(this->requestWriteFmqUnsync(this->mNumMessagesMax, this->mService)); ret = this->requestReadFmqUnsync(this->mNumMessagesMax, this->mService); ASSERT_TRUE(ret); @@ -1080,13 +1074,15 @@ const size_t chunkNum = 5; const size_t numMessages = chunkSize * chunkNum; ASSERT_LE(numMessages, this->mNumMessagesMax); - int32_t data[numMessages]; - initData(data, numMessages); for (size_t i = 0; i < chunkNum; i++) { - ASSERT_TRUE(this->mQueue->write(data + i * chunkSize, chunkSize)); + ASSERT_TRUE(this->requestWriteFmqUnsync(chunkSize, this->mService)); } - bool ret = this->requestReadFmqUnsync(numMessages, this->mService); - ASSERT_TRUE(ret); + ASSERT_EQ(numMessages, this->mQueue->availableToRead()); + int32_t readData[numMessages] = {}; + ASSERT_TRUE(this->mQueue->read(readData, numMessages)); + // verify that data is filled by the service - the messages will contiain + // 'chunkSize' because that's the value we passed to the service each write. + ASSERT_TRUE(verifyData(readData, chunkSize)); } /* @@ -1097,14 +1093,10 @@ */ TYPED_TEST(UnsynchronizedWriteClient, ReadWriteWrapAround) { size_t numMessages = this->mNumMessagesMax / 2; - std::vector<int32_t> data(this->mNumMessagesMax); - initData(&data[0], this->mNumMessagesMax); - ASSERT_TRUE(this->mQueue->write(&data[0], numMessages)); - bool ret = this->requestReadFmqUnsync(numMessages, this->mService); - ASSERT_TRUE(ret); - ASSERT_TRUE(this->mQueue->write(&data[0], this->mNumMessagesMax)); - ret = this->requestReadFmqUnsync(this->mNumMessagesMax, this->mService); - ASSERT_TRUE(ret); + ASSERT_TRUE(this->requestWriteFmqUnsync(numMessages, this->mService)); + ASSERT_TRUE(this->requestReadFmqUnsync(numMessages, this->mService)); + ASSERT_TRUE(this->requestWriteFmqUnsync(this->mNumMessagesMax, this->mService)); + ASSERT_TRUE(this->requestReadFmqUnsync(this->mNumMessagesMax, this->mService)); } /*