Protect against malformed grantorsandroid13-gsi

Grantors might have invalid fdIndex and extent values.

Test: atest fmq_unit_tests
Test: running POC from bug on device
Bug: 244713317
Change-Id: I17bd7da9bd47fe7b89ddb890d476d2f66f4c0d0d
(cherry picked from commit d931906af5e245eeea4a2749e9153be0180d1bae)
Merged-In: I17bd7da9bd47fe7b89ddb890d476d2f66f4c0d0d
(cherry picked from commit 98f3a08b2fb44e7cff31c26007fbe9feaac11750)

2 files changed, 113 insertions, 13 deletions

diff --git a/include/fmq/MessageQueueBase.h b/include/fmq/MessageQueueBase.h
index c34a4ff..f4bf7e2 100644
--- a/include/fmq/MessageQueueBase.h
+++ b/include/fmq/MessageQueueBase.h
@@ -586,12 +586,6 @@ void MessageQueueBase<MQDescriptorType, T, flavor>::initMemory(bool resetPointer
         return;
     }
 
-    const auto& grantors = mDesc->grantors();
-    for (const auto& grantor : grantors) {
-        hardware::details::check(hardware::details::isAlignedToWordBoundary(grantor.offset) == true,
-                                 "Grantor offsets need to be aligned");
-    }
-
     if (flavor == kSynchronizedReadWrite) {
         mReadPtr = reinterpret_cast<std::atomic<uint64_t>*>(
                 mapGrantorDescr(hardware::details::READPTRPOS));
@@ -602,11 +596,11 @@ void MessageQueueBase<MQDescriptorType, T, flavor>::initMemory(bool resetPointer
          */
         mReadPtr = new (std::nothrow) std::atomic<uint64_t>;
     }
-    hardware::details::check(mReadPtr != nullptr, "mReadPtr is null");
+    if (mReadPtr == nullptr) goto error;
 
     mWritePtr = reinterpret_cast<std::atomic<uint64_t>*>(
             mapGrantorDescr(hardware::details::WRITEPTRPOS));
-    hardware::details::check(mWritePtr != nullptr, "mWritePtr is null");
+    if (mWritePtr == nullptr) goto error;
 
     if (resetPointers) {
         mReadPtr->store(0, std::memory_order_release);
@@ -617,14 +611,32 @@ void MessageQueueBase<MQDescriptorType, T, flavor>::initMemory(bool resetPointer
     }
 
     mRing = reinterpret_cast<uint8_t*>(mapGrantorDescr(hardware::details::DATAPTRPOS));
-    hardware::details::check(mRing != nullptr, "mRing is null");
+    if (mRing == nullptr) goto error;
 
     if (mDesc->countGrantors() > hardware::details::EVFLAGWORDPOS) {
         mEvFlagWord = static_cast<std::atomic<uint32_t>*>(
                 mapGrantorDescr(hardware::details::EVFLAGWORDPOS));
-        hardware::details::check(mEvFlagWord != nullptr, "mEvFlagWord is null");
+        if (mEvFlagWord == nullptr) goto error;
         android::hardware::EventFlag::createEventFlag(mEvFlagWord, &mEventFlag);
     }
+    return;
+error:
+    if (mReadPtr) {
+        if (flavor == kSynchronizedReadWrite) {
+            unmapGrantorDescr(mReadPtr, hardware::details::READPTRPOS);
+        } else {
+            delete mReadPtr;
+        }
+        mReadPtr = nullptr;
+    }
+    if (mWritePtr) {
+        unmapGrantorDescr(mWritePtr, hardware::details::WRITEPTRPOS);
+        mWritePtr = nullptr;
+    }
+    if (mRing) {
+        unmapGrantorDescr(mRing, hardware::details::EVFLAGWORDPOS);
+        mRing = nullptr;
+    }
 }
 
 template <template <typename, MQFlavor> typename MQDescriptorType, typename T, MQFlavor flavor>
@@ -1234,7 +1246,7 @@ bool MessageQueueBase<MQDescriptorType, T, flavor>::isValid() const {
 template <template <typename, MQFlavor> typename MQDescriptorType, typename T, MQFlavor flavor>
 void* MessageQueueBase<MQDescriptorType, T, flavor>::mapGrantorDescr(uint32_t grantorIdx) {
     const native_handle_t* handle = mDesc->handle();
-    auto grantors = mDesc->grantors();
+    const std::vector<android::hardware::GrantorDescriptor> grantors = mDesc->grantors();
     if (handle == nullptr) {
         hardware::details::logError("mDesc->handle is null");
         return nullptr;
@@ -1247,10 +1259,32 @@ void* MessageQueueBase<MQDescriptorType, T, flavor>::mapGrantorDescr(uint32_t gr
     }
 
     int fdIndex = grantors[grantorIdx].fdIndex;
+    if (fdIndex < 0 || fdIndex >= handle->numFds) {
+        hardware::details::logError(
+                std::string("fdIndex (" + std::to_string(fdIndex) + ") from grantor (index " +
+                            std::to_string(grantorIdx) +
+                            ") must be smaller than the number of fds in the handle: " +
+                            std::to_string(handle->numFds)));
+        return nullptr;
+    }
+
     /*
      * Offset for mmap must be a multiple of PAGE_SIZE.
      */
+    if (!hardware::details::isAlignedToWordBoundary(grantors[grantorIdx].offset)) {
+        hardware::details::logError("Grantor (index " + std::to_string(grantorIdx) +
+                                    ") offset needs to be aligned to word boundary but is: " +
+                                    std::to_string(grantors[grantorIdx].offset));
+        return nullptr;
+    }
+
     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) +
+                                                ") extent value is too large or negative: " +
+                                                std::to_string(grantors[grantorIdx].extent)));
+        return nullptr;
+    }
     int mapLength = grantors[grantorIdx].offset - mapOffset + grantors[grantorIdx].extent;
 
     void* address = mmap(0, mapLength, PROT_READ | PROT_WRITE, MAP_SHARED, handle->data[fdIndex],
diff --git a/tests/fmq_unit_tests.cpp b/tests/fmq_unit_tests.cpp
index d3fdfbc..3d70439 100644
--- a/tests/fmq_unit_tests.cpp
+++ b/tests/fmq_unit_tests.cpp
@@ -230,6 +230,7 @@ template <typename T>
 class BadQueueConfig : public TestBase<T> {};
 
 class AidlOnlyBadQueueConfig : public ::testing::Test {};
+class HidlOnlyBadQueueConfig : public ::testing::Test {};
 class Hidl2AidlOperation : public ::testing::Test {};
 class DoubleFdFailures : public ::testing::Test {};
 
@@ -310,6 +311,71 @@ TYPED_TEST(BadQueueConfig, QueueSizeTooLarge) {
     ASSERT_FALSE(fmq->isValid());
 }
 
+// {flags, fdIndex, offset, extent}
+static const std::vector<android::hardware::GrantorDescriptor> kGrantors = {
+        {0, 0, 0, 4096},
+        {0, 0, 0, 4096},
+        {0, 0, 0, 4096},
+};
+
+// Make sure this passes without invalid index/extent for the next two test
+// cases
+TEST_F(HidlOnlyBadQueueConfig, SanityCheck) {
+    std::vector<android::hardware::GrantorDescriptor> grantors = kGrantors;
+
+    native_handle_t* handle = native_handle_create(1, 0);
+    int ashmemFd = ashmem_create_region("QueueHidlOnlyBad", 4096);
+    ashmem_set_prot_region(ashmemFd, PROT_READ | PROT_WRITE);
+    handle->data[0] = ashmemFd;
+
+    android::hardware::MQDescriptor<uint16_t, kSynchronizedReadWrite> desc(grantors, handle,
+                                                                           sizeof(uint16_t));
+    android::hardware::MessageQueue<uint16_t, kSynchronizedReadWrite> fmq(desc);
+    EXPECT_TRUE(fmq.isValid());
+
+    close(ashmemFd);
+}
+
+TEST_F(HidlOnlyBadQueueConfig, BadFdIndex) {
+    std::vector<android::hardware::GrantorDescriptor> grantors = kGrantors;
+    grantors[0].fdIndex = 5;
+
+    native_handle_t* handle = native_handle_create(1, 0);
+    int ashmemFd = ashmem_create_region("QueueHidlOnlyBad", 4096);
+    ashmem_set_prot_region(ashmemFd, PROT_READ | PROT_WRITE);
+    handle->data[0] = ashmemFd;
+
+    android::hardware::MQDescriptor<uint16_t, kSynchronizedReadWrite> desc(grantors, handle,
+                                                                           sizeof(uint16_t));
+    android::hardware::MessageQueue<uint16_t, kSynchronizedReadWrite> fmq(desc);
+    /*
+     * Should fail due fdIndex being out of range of the native_handle.
+     */
+    EXPECT_FALSE(fmq.isValid());
+
+    close(ashmemFd);
+}
+
+TEST_F(HidlOnlyBadQueueConfig, ExtentTooLarge) {
+    std::vector<android::hardware::GrantorDescriptor> grantors = kGrantors;
+    grantors[0].extent = 0xfffff041;
+
+    native_handle_t* handle = native_handle_create(1, 0);
+    int ashmemFd = ashmem_create_region("QueueHidlOnlyBad", 4096);
+    ashmem_set_prot_region(ashmemFd, PROT_READ | PROT_WRITE);
+    handle->data[0] = ashmemFd;
+
+    android::hardware::MQDescriptor<uint16_t, kSynchronizedReadWrite> desc(grantors, handle,
+                                                                           sizeof(uint16_t));
+    android::hardware::MessageQueue<uint16_t, kSynchronizedReadWrite> fmq(desc);
+    /*
+     * Should fail due to extent being too large.
+     */
+    EXPECT_FALSE(fmq.isValid());
+
+    close(ashmemFd);
+}
+
 // If this test fails and we do leak FDs, the next test will cause a crash
 TEST_F(AidlOnlyBadQueueConfig, LookForLeakedFds) {
     size_t numElementsInQueue = SIZE_MAX / sizeof(uint32_t) - PAGE_SIZE - 1;
@@ -516,8 +582,8 @@ TEST_F(AidlOnlyBadQueueConfig, MismatchedPayloadSize) {
  */
 TEST_F(DoubleFdFailures, InvalidFd) {
     android::base::SetLogger(android::base::StdioLogger);
-    EXPECT_DEATH_IF_SUPPORTED(AidlMessageQueueSync(64, false, android::base::unique_fd(3000), 64),
-                              "Check failed: exp mRing is null");
+    auto queue = AidlMessageQueueSync(64, false, android::base::unique_fd(3000), 64);
+    EXPECT_FALSE(queue.isValid());
 }
 
 /*