Validate all fds as fd_count cannot be relied upon

Bug: 212803946
Bug: 212804042
Test: Boots to home, VTS and manual check heap_profile
Change-Id: Ie0aed4a66093bd24fce79f821b71de5333f67fb7
Merged-In: I7ab56cd3126f57ae0edebe2707c4e24cdab6196a

3 files changed, 28 insertions, 17 deletions

diff --git a/gralloc4/src/allocator/mali_gralloc_ion.cpp b/gralloc4/src/allocator/mali_gralloc_ion.cpp
index e807651..58790a3 100644
--- a/gralloc4/src/allocator/mali_gralloc_ion.cpp
+++ b/gralloc4/src/allocator/mali_gralloc_ion.cpp
@@ -588,7 +588,8 @@ int mali_gralloc_ion_allocate(const gralloc_buffer_descriptor_t *descriptors,
 	uint32_t i;
 	unsigned int ion_flags = 0;
 	int min_pgsz = 0;
-	int fds[5] = {-1, -1, -1, -1, -1};
+	int fds[MAX_FDS];
+	std::fill(fds, fds + MAX_FDS, -1);
 
 	ion_device *dev = ion_device::get();
 	if (!dev)
diff --git a/gralloc4/src/core/mali_gralloc_reference.cpp b/gralloc4/src/core/mali_gralloc_reference.cpp
index 6dc7ddd..954c2b3 100644
--- a/gralloc4/src/core/mali_gralloc_reference.cpp
+++ b/gralloc4/src/core/mali_gralloc_reference.cpp
@@ -37,8 +37,8 @@ private:
     // should become the only place where address mapping is maintained and can be
     // queried from.
     struct MappedData {
-        void *bases[MAX_FDS] = {};
-        size_t alloc_sizes[MAX_FDS] = {};
+        void *bases[MAX_BUFFER_FDS] = {};
+        size_t alloc_sizes[MAX_BUFFER_FDS] = {};
         uint64_t ref_count = 0;
     };
 
@@ -81,7 +81,7 @@ private:
             return error;
         }
 
-        for (auto i = 0; i < hnd->fd_count; i++) {
+        for (auto i = 0; i < MAX_BUFFER_FDS; i++) {
             data.bases[i] = reinterpret_cast<void *>(hnd->bases[i]);
             data.alloc_sizes[i] = hnd->alloc_sizes[i];
         }
@@ -104,7 +104,7 @@ private:
 
         auto &data = *(it->second.get());
         if (data.bases[0] != nullptr) {
-            for (auto i = 0; i < hnd->fd_count; i++) {
+            for (auto i = 0; i < MAX_BUFFER_FDS; i++) {
                 if (data.bases[i] != reinterpret_cast<void *>(hnd->bases[i]) ||
                     data.alloc_sizes[i] != hnd->alloc_sizes[i]) {
                     MALI_GRALLOC_LOGE(
@@ -113,7 +113,7 @@ private:
                 }
             }
         } else {
-            for (auto i = 0; i < hnd->fd_count; i++) {
+            for (auto i = 0; i < MAX_BUFFER_FDS; i++) {
                 if (hnd->bases[i] != 0 || data.bases[i] != nullptr) {
                     MALI_GRALLOC_LOGE("Validation failed: Expected nullptr for unmaped buffer");
                     return -EINVAL;
@@ -150,7 +150,7 @@ public:
                 return -EINVAL;
             }
 
-            for (int i = 0; i < hnd->fd_count; i++) {
+            for (int i = 0; i < MAX_BUFFER_FDS; i++) {
                 hnd->bases[i] = 0;
             }
         } else if (it->second->ref_count == 0) {
diff --git a/gralloc4/src/mali_gralloc_buffer.h b/gralloc4/src/mali_gralloc_buffer.h
index 908c36c..02172c6 100644
--- a/gralloc4/src/mali_gralloc_buffer.h
+++ b/gralloc4/src/mali_gralloc_buffer.h
@@ -53,7 +53,17 @@
 /*
  * Maximum number of fds in a private_handle_t.
  */
-#define MAX_FDS 5
+#define MAX_FDS 4
+
+/*
+ * One fd is reserved for metadata dmabuf.
+ */
+#define MAX_BUFFER_FDS MAX_FDS - 1
+
+/*
+ * In the worst case, there will be one plane per fd.
+ */
+static_assert(MAX_BUFFER_FDS == MAX_PLANES, "MAX_PLANES and MAX_BUFFER_FDS defines do not match");
 
 #ifdef __cplusplus
 #define DEFAULT_INITIALIZER(x) = x
@@ -222,9 +232,9 @@ struct private_handle_t
 	int cpu_write DEFAULT_INITIALIZER(0);              /**< Buffer is locked for CPU write when non-zero. */
 	// locally mapped shared attribute area
 
-	int ion_handles[3];
-	uint64_t bases[3];
-	uint64_t alloc_sizes[3];
+	int ion_handles[MAX_BUFFER_FDS];
+	uint64_t bases[MAX_BUFFER_FDS];
+	uint64_t alloc_sizes[MAX_BUFFER_FDS];
 
 	void *attr_base __attribute__((aligned (8))) DEFAULT_INITIALIZER(nullptr);
 	off_t offset    __attribute__((aligned (8))) DEFAULT_INITIALIZER(0);
@@ -249,9 +259,9 @@ struct private_handle_t
 
 	private_handle_t(
 		int _flags,
-		uint64_t _alloc_sizes[3],
+		uint64_t _alloc_sizes[MAX_BUFFER_FDS],
 		uint64_t _consumer_usage, uint64_t _producer_usage,
-		int _fds[5], int _fd_count,
+		int _fds[MAX_FDS], int _fd_count,
 		int _req_format, uint64_t _alloc_format,
 		int _width, int _height, int _stride,
 		uint64_t _layer_count, plane_info_t _plane_info[MAX_PLANES])
@@ -329,8 +339,8 @@ struct private_handle_t
 
 	int get_share_attr_fd_index() const
 	{
-		/* share_attr can be at idx 1 to 4 */
-		if (fd_count <= 0 || fd_count > 4)
+		/* share_attr can be at idx 1 to MAX_FDS */
+		if (fd_count <= 0 || fd_count > MAX_FDS)
 			return -1;
 
 		return fd_count;
@@ -370,7 +380,7 @@ struct private_handle_t
 	{
 		ALOGE("[%s] "
 			"numInts(%d) numFds(%d) fd_count(%d) "
-			"fd(%d %d %d %d %d) "
+			"fd(%d %d %d %d) "
 			"flags(%d) "
 			"wh(%d %d) "
 			"req_format(%#x) alloc_format(%#" PRIx64 ") "
@@ -386,7 +396,7 @@ struct private_handle_t
 			"\n",
 			str,
 			numInts, numFds, fd_count,
-			fds[0], fds[1], fds[2], fds[3], fds[4],
+			fds[0], fds[1], fds[2], fds[3],
 			flags,
 			width, height,
 			req_format, alloc_format,