Snap for 10453563 from 90f8ec85746d01057064e7c670e23c1cf66e73fe to mainline-documentsui-release

Change-Id: Ic3eb47095caacffc5b5c53ec0c5f88bec6cdba38
diff --git a/Android.bp b/Android.bp
index f37f1c6..c42772b 100644
--- a/Android.bp
+++ b/Android.bp
@@ -30,7 +30,7 @@
     ],
     apex_available: [
         "//apex_available:platform",
-        "com.android.bluetooth",
+        "com.android.btservices",
         "com.android.media.swcodec",
         "com.android.neuralnetworks",
         "test_com.android.media.swcodec",
diff --git a/OWNERS b/OWNERS
index cc4d814..8b7f0e5 100644
--- a/OWNERS
+++ b/OWNERS
@@ -1,3 +1,5 @@
+# Bug component: 655781
+
 smoreland@google.com
 elsk@google.com
 malchev@google.com
diff --git a/TEST_MAPPING b/TEST_MAPPING
index 9f1aa6b..51c6c97 100644
--- a/TEST_MAPPING
+++ b/TEST_MAPPING
@@ -7,7 +7,7 @@
       "name": "fmq_test"
     }
   ],
-  "hwasan-postsubmit": [
+  "hwasan-presubmit": [
     {
       "name": "fmq_unit_tests"
     },
diff --git a/fuzzer/Android.bp b/fuzzer/Android.bp
index 0926c09..72d99d4 100644
--- a/fuzzer/Android.bp
+++ b/fuzzer/Android.bp
@@ -54,15 +54,4 @@
     },
 
     host_supported: true,
-
-    sanitize: {
-        scs: true,
-        cfi: true,
-        address: true,
-        memtag_heap: true,
-        // undefined behavior is expected
-        all_undefined: false,
-        // integer overflow is expected
-        integer_overflow: false,
-    },
 }
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/AidlMQDescriptorShim.h b/include/fmq/AidlMQDescriptorShim.h
index e3d3cb9..de175da 100644
--- a/include/fmq/AidlMQDescriptorShim.h
+++ b/include/fmq/AidlMQDescriptorShim.h
@@ -230,7 +230,11 @@
 
 template <typename T, MQFlavor flavor>
 size_t AidlMQDescriptorShim<T, flavor>::getSize() const {
-    return mGrantors[hardware::details::DATAPTRPOS].extent;
+    if (mGrantors.size() > hardware::details::DATAPTRPOS) {
+        return mGrantors[hardware::details::DATAPTRPOS].extent;
+    } else {
+        return 0;
+    }
 }
 
 template <typename T, MQFlavor flavor>
diff --git a/include/fmq/MessageQueueBase.h b/include/fmq/MessageQueueBase.h
index c34a4ff..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;
@@ -586,12 +591,6 @@
         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 +601,11 @@
          */
         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,21 +616,40 @@
     }
 
     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>
 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;
     }
 
@@ -650,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(" +
@@ -755,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);
@@ -1234,7 +1256,7 @@
 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 +1269,55 @@
     }
 
     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;
+    }
+
+    /*
+     * 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) +
+                                                ") 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/Android.bp b/tests/Android.bp
index 40148b1..55eb1c7 100644
--- a/tests/Android.bp
+++ b/tests/Android.bp
@@ -37,7 +37,8 @@
     name: "fmq_test_client",
     tidy_timeout_srcs: ["msgq_test_client.cpp"],
     srcs: ["msgq_test_client.cpp"],
-
+    // This cc_test is used through the python test and won't support isolated
+    isolated: false,
     cflags: [
         "-Wall",
         "-Werror",
diff --git a/tests/fmq_unit_tests.cpp b/tests/fmq_unit_tests.cpp
index d3fdfbc..03db6ec 100644
--- a/tests/fmq_unit_tests.cpp
+++ b/tests/fmq_unit_tests.cpp
@@ -230,6 +230,7 @@
 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 {};
 
@@ -301,13 +302,76 @@
 
 TYPED_TEST(BadQueueConfig, QueueSizeTooLarge) {
     size_t numElementsInQueue = SIZE_MAX / sizeof(uint16_t) + 1;
-    typename TypeParam::MQType* fmq =
-            new (std::nothrow) typename TypeParam::MQType(numElementsInQueue);
-    ASSERT_NE(nullptr, fmq);
+    typename TypeParam::MQType fmq(numElementsInQueue);
     /*
      * Should fail due to size being too large to fit into size_t.
      */
-    ASSERT_FALSE(fmq->isValid());
+    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
@@ -329,10 +393,9 @@
     size_t numElementsInQueue = 64;
 
     // Create HIDL side and get MQDescriptor
-    MessageQueueSync8* fmq = new (std::nothrow) MessageQueueSync8(numElementsInQueue);
-    ASSERT_NE(nullptr, fmq);
-    ASSERT_TRUE(fmq->isValid());
-    const HidlMQDescSync8* hidlDesc = fmq->getDesc();
+    MessageQueueSync8 fmq(numElementsInQueue);
+    ASSERT_TRUE(fmq.isValid());
+    const HidlMQDescSync8* hidlDesc = fmq.getDesc();
     ASSERT_NE(nullptr, hidlDesc);
 
     // Create AIDL MQDescriptor to send to another process based off the HIDL MQDescriptor
@@ -341,17 +404,16 @@
                                                                                   &aidlDesc);
 
     // Other process will create the other side of the queue using the AIDL MQDescriptor
-    AidlMessageQueueSync8* aidlFmq = new (std::nothrow) AidlMessageQueueSync8(aidlDesc);
-    ASSERT_NE(nullptr, aidlFmq);
-    ASSERT_TRUE(aidlFmq->isValid());
+    AidlMessageQueueSync8 aidlFmq(aidlDesc);
+    ASSERT_TRUE(aidlFmq.isValid());
 
     // Make sure a write to the HIDL side, will show up for the AIDL side
     constexpr size_t dataLen = 4;
     uint8_t data[dataLen] = {12, 11, 10, 9};
-    fmq->write(data, dataLen);
+    fmq.write(data, dataLen);
 
     int8_t readData[dataLen];
-    ASSERT_TRUE(aidlFmq->read(readData, dataLen));
+    ASSERT_TRUE(aidlFmq.read(readData, dataLen));
 
     ASSERT_EQ(data[0], readData[0]);
     ASSERT_EQ(data[1], readData[1]);
@@ -363,10 +425,9 @@
     size_t numElementsInQueue = 64;
 
     // Create HIDL side and get MQDescriptor
-    MessageQueueUnsync8* fmq = new (std::nothrow) MessageQueueUnsync8(numElementsInQueue);
-    ASSERT_NE(nullptr, fmq);
-    ASSERT_TRUE(fmq->isValid());
-    const HidlMQDescUnsync8* hidlDesc = fmq->getDesc();
+    MessageQueueUnsync8 fmq(numElementsInQueue);
+    ASSERT_TRUE(fmq.isValid());
+    const HidlMQDescUnsync8* hidlDesc = fmq.getDesc();
     ASSERT_NE(nullptr, hidlDesc);
 
     // Create AIDL MQDescriptor to send to another process based off the HIDL MQDescriptor
@@ -375,24 +436,22 @@
                                                                                 &aidlDesc);
 
     // Other process will create the other side of the queue using the AIDL MQDescriptor
-    AidlMessageQueueUnsync8* aidlFmq = new (std::nothrow) AidlMessageQueueUnsync8(aidlDesc);
-    ASSERT_NE(nullptr, aidlFmq);
-    ASSERT_TRUE(aidlFmq->isValid());
+    AidlMessageQueueUnsync8 aidlFmq(aidlDesc);
+    ASSERT_TRUE(aidlFmq.isValid());
 
     // Can have multiple readers with unsync flavor
-    AidlMessageQueueUnsync8* aidlFmq2 = new (std::nothrow) AidlMessageQueueUnsync8(aidlDesc);
-    ASSERT_NE(nullptr, aidlFmq2);
-    ASSERT_TRUE(aidlFmq2->isValid());
+    AidlMessageQueueUnsync8 aidlFmq2(aidlDesc);
+    ASSERT_TRUE(aidlFmq2.isValid());
 
     // Make sure a write to the HIDL side, will show up for the AIDL side
     constexpr size_t dataLen = 4;
     uint8_t data[dataLen] = {12, 11, 10, 9};
-    fmq->write(data, dataLen);
+    fmq.write(data, dataLen);
 
     int8_t readData[dataLen];
-    ASSERT_TRUE(aidlFmq->read(readData, dataLen));
+    ASSERT_TRUE(aidlFmq.read(readData, dataLen));
     int8_t readData2[dataLen];
-    ASSERT_TRUE(aidlFmq2->read(readData2, dataLen));
+    ASSERT_TRUE(aidlFmq2.read(readData2, dataLen));
 
     ASSERT_EQ(data[0], readData[0]);
     ASSERT_EQ(data[1], readData[1]);
@@ -417,12 +476,12 @@
     grantors[1] = {0, 1 /* fdIndex */, 16, 16};
     grantors[2] = {0, 1 /* fdIndex */, 16, 16};
 
-    HidlMQDescUnsync8* hidlDesc = new (std::nothrow) HidlMQDescUnsync8(grantors, mqHandle, 10);
-    ASSERT_TRUE(hidlDesc->isHandleValid());
+    HidlMQDescUnsync8 hidlDesc(grantors, mqHandle, 10);
+    ASSERT_TRUE(hidlDesc.isHandleValid());
 
     AidlMQDescUnsync8 aidlDesc;
     bool ret = android::unsafeHidlToAidlMQDescriptor<uint8_t, int8_t, UnsynchronizedWrite>(
-            *hidlDesc, &aidlDesc);
+            hidlDesc, &aidlDesc);
     ASSERT_TRUE(ret);
 }
 
@@ -439,16 +498,14 @@
     grantors[1] = {0, 1 /* fdIndex */, 16, 16};
     grantors[2] = {0, 0 /* fdIndex */, 16, 16};
 
-    HidlMQDescUnsync8* hidlDesc = new (std::nothrow) HidlMQDescUnsync8(grantors, mqHandle, 10);
-    ASSERT_TRUE(hidlDesc->isHandleValid());
+    HidlMQDescUnsync8 hidlDesc(grantors, mqHandle, 10);
+    ASSERT_TRUE(hidlDesc.isHandleValid());
 
     AidlMQDescUnsync8 aidlDesc;
     bool ret = android::unsafeHidlToAidlMQDescriptor<uint8_t, int8_t, UnsynchronizedWrite>(
-            *hidlDesc, &aidlDesc);
+            hidlDesc, &aidlDesc);
     ASSERT_TRUE(ret);
     EXPECT_EQ(aidlDesc.handle.fds.size(), 2);
-    native_handle_close(mqHandle);
-    native_handle_delete(mqHandle);
 }
 
 // TODO(b/165674950) Since AIDL does not support unsigned integers, it can only support
@@ -456,23 +513,21 @@
 // lifted. Until then, check against SSIZE_MAX instead of SIZE_MAX.
 TEST_F(AidlOnlyBadQueueConfig, QueueSizeTooLargeForAidl) {
     size_t numElementsInQueue = SSIZE_MAX / sizeof(uint16_t) + 1;
-    AidlMessageQueueSync16* fmq = new (std::nothrow) AidlMessageQueueSync16(numElementsInQueue);
-    ASSERT_NE(nullptr, fmq);
+    AidlMessageQueueSync16 fmq(numElementsInQueue);
     /*
      * Should fail due to size being too large to fit into size_t.
      */
-    ASSERT_FALSE(fmq->isValid());
+    ASSERT_FALSE(fmq.isValid());
 }
 
 TEST_F(AidlOnlyBadQueueConfig, NegativeAidlDescriptor) {
     aidl::android::hardware::common::fmq::MQDescriptor<uint16_t, SynchronizedReadWrite> desc;
     desc.quantum = -10;
-    AidlMessageQueueSync16* fmq = new (std::nothrow) AidlMessageQueueSync16(desc);
-    ASSERT_NE(nullptr, fmq);
+    AidlMessageQueueSync16 fmq(desc);
     /*
      * Should fail due to quantum being negative.
      */
-    ASSERT_FALSE(fmq->isValid());
+    ASSERT_FALSE(fmq.isValid());
 }
 
 TEST_F(AidlOnlyBadQueueConfig, NegativeAidlDescriptorGrantor) {
@@ -481,12 +536,11 @@
     desc.flags = 0;
     desc.grantors.push_back(
             aidl::android::hardware::common::fmq::GrantorDescriptor{.offset = 12, .extent = -10});
-    AidlMessageQueueSync16* fmq = new (std::nothrow) AidlMessageQueueSync16(desc);
-    ASSERT_NE(nullptr, fmq);
+    AidlMessageQueueSync16 fmq(desc);
     /*
      * Should fail due to grantor having negative extent.
      */
-    ASSERT_FALSE(fmq->isValid());
+    ASSERT_FALSE(fmq.isValid());
 }
 
 /*
@@ -516,8 +570,8 @@
  */
 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());
 }
 
 /*