gralloc4: Defer metadata mmap and remove metadata vaddr from handle am: 1881b96025

Original change: https://googleplex-android-review.googlesource.com/c/platform/hardware/google/gchips/+/24591226

Change-Id: I6e7cfb5cfdb15abb2ef71a21a70284309e123527
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

13 files changed, 178 insertions, 197 deletions

diff --git a/gralloc4/src/Android.bp b/gralloc4/src/Android.bp
index a813fc8..5f89832 100644
--- a/gralloc4/src/Android.bp
+++ b/gralloc4/src/Android.bp
@@ -44,7 +44,6 @@ cc_library_shared {
 	srcs: [
 		"libGralloc4Wrapper/wrapper.cpp",
 		"allocator/mali_gralloc_ion.cpp",
-		"allocator/mali_gralloc_shared_memory.cpp",
 		"core/format_info.cpp",
 		"core/mali_gralloc_formats.cpp",
 		"core/mali_gralloc_bufferallocation.cpp",
diff --git a/gralloc4/src/allocator/Android.bp b/gralloc4/src/allocator/Android.bp
index fe42411..630735b 100644
--- a/gralloc4/src/allocator/Android.bp
+++ b/gralloc4/src/allocator/Android.bp
@@ -76,7 +76,6 @@ arm_gralloc_allocator_cc_defaults {
 	},
 	srcs: [
 		"mali_gralloc_ion.cpp",
-		"mali_gralloc_shared_memory.cpp",
 	],
 	static_libs: [
 		"libarect",
diff --git a/gralloc4/src/allocator/mali_gralloc_shared_memory.cpp b/gralloc4/src/allocator/mali_gralloc_shared_memory.cpp
deleted file mode 100644
index b681cd9..0000000
--- a/gralloc4/src/allocator/mali_gralloc_shared_memory.cpp
+++ /dev/null
@@ -1,37 +0,0 @@
-/*
- * Copyright (C) 2020 Arm Limited. All rights reserved.
- *
- * Copyright (C) 2008 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include "mali_gralloc_shared_memory.h"
-#include "mali_gralloc_log.h"
-#include "mali_gralloc_buffer.h"
-#include "mali_gralloc_ion.h"
-#include "gralloc_helper.h"
-
-/* TODO: move shared attr memory allocation function here */
-
-void gralloc_shared_memory_free(private_handle_t *hnd)
-{
-	if (hnd->attr_base)
-	{
-		munmap(hnd->attr_base, hnd->attr_size);
-		hnd->attr_base = nullptr;
-	}
-
-	hnd->close_share_attr_fd();
-	hnd->set_share_attr_fd(-1);
-}
diff --git a/gralloc4/src/allocator/mali_gralloc_shared_memory.h b/gralloc4/src/allocator/mali_gralloc_shared_memory.h
deleted file mode 100644
index 28e0a62..0000000
--- a/gralloc4/src/allocator/mali_gralloc_shared_memory.h
+++ /dev/null
@@ -1,31 +0,0 @@
-/*
- * Copyright (C) 2020 Arm Limited. All rights reserved.
- *
- * Copyright (C) 2008 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#ifndef GRALLOC_SHARED_MEMORY_H_
-#define GRALLOC_SHARED_MEMORY_H_
-
-#include <stdint.h>
-#include <utility>
-#include "mali_gralloc_buffer.h"
-
-/*
- * Frees resources acquired from gralloc_shared_memory_allocate.
- */
-void gralloc_shared_memory_free(private_handle_t *hnd);
-
-#endif /* GRALLOC_SHARED_MEMORY_H_ */
diff --git a/gralloc4/src/core/mali_gralloc_bufferallocation.cpp b/gralloc4/src/core/mali_gralloc_bufferallocation.cpp
index 27faa51..d67eeb8 100644
--- a/gralloc4/src/core/mali_gralloc_bufferallocation.cpp
+++ b/gralloc4/src/core/mali_gralloc_bufferallocation.cpp
@@ -30,7 +30,6 @@
 
 #include "mali_gralloc_bufferallocation.h"
 #include "allocator/mali_gralloc_ion.h"
-#include "allocator/mali_gralloc_shared_memory.h"
 #include "mali_gralloc_buffer.h"
 #include "mali_gralloc_bufferdescriptor.h"
 #include "mali_gralloc_log.h"
@@ -1216,8 +1215,8 @@ int mali_gralloc_buffer_free(buffer_handle_t pHandle)
 		return -1;
 	}
 
-	gralloc_shared_memory_free(hnd);
-	mali_gralloc_ion_free(hnd);
+	native_handle_close(hnd);
+	native_handle_delete(hnd);
 
 	return 0;
 }
diff --git a/gralloc4/src/core/mali_gralloc_reference.cpp b/gralloc4/src/core/mali_gralloc_reference.cpp
index 84f4c14..6726895 100644
--- a/gralloc4/src/core/mali_gralloc_reference.cpp
+++ b/gralloc4/src/core/mali_gralloc_reference.cpp
@@ -37,6 +37,10 @@ private:
     struct MappedData {
         std::array<void *, MAX_BUFFER_FDS> bases;
         size_t alloc_sizes[MAX_BUFFER_FDS] = {};
+
+        void *metadata_vaddr;
+        size_t metadata_size;
+
         uint64_t ref_count = 0;
     };
 
@@ -52,7 +56,9 @@ private:
         return size;
     }
 
-    static bool dmabuf_sanity_check(buffer_handle_t handle) {
+    // TODO(b/296934447): AION buffers set the size of the buffer themselves. That size exceeds the
+    // size of the actual allocated dmabuf.
+    static bool dmabuf_sanity_check(buffer_handle_t handle, bool skip_buffer_size_check = false) {
         private_handle_t *hnd =
                 static_cast<private_handle_t *>(const_cast<native_handle_t *>(handle));
 
@@ -77,10 +83,13 @@ private:
         };
 
         // Check client facing dmabufs
-        for (auto i = 0; i < hnd->fd_count; i++) {
-            if (!check_pid(hnd->fds[i], hnd->alloc_sizes[i])) {
-                MALI_GRALLOC_LOGE("%s failed: Size check failed for alloc_sizes[%d]", __func__, i);
-                return false;
+        if (!skip_buffer_size_check) {
+            for (auto i = 0; i < hnd->fd_count; i++) {
+                if (!check_pid(hnd->fds[i], hnd->alloc_sizes[i])) {
+                    MALI_GRALLOC_LOGE("%s failed: Size check failed for alloc_sizes[%d]", __func__,
+                                      i);
+                    return false;
+                }
             }
         }
 
@@ -93,53 +102,75 @@ private:
         return true;
     }
 
-    int map_locked(buffer_handle_t handle) REQUIRES(lock) {
-        private_handle_t *hnd =
-                reinterpret_cast<private_handle_t *>(const_cast<native_handle *>(handle));
-        auto it = buffer_map.find(hnd);
-
-        if (it == buffer_map.end()) {
-            MALI_GRALLOC_LOGE("BUG: Map called without importing buffer");
-            return -EINVAL;
-        }
-
-        auto &data = *(it->second.get());
-        if (data.ref_count == 0) {
-            MALI_GRALLOC_LOGE("BUG: Found an imported buffer with ref count 0, expect errors");
+    bool map_buffer_locked(buffer_handle_t handle) REQUIRES(lock) {
+        auto data_oe = get_validated_data_locked(handle);
+        if (!data_oe.has_value()) {
+            return false;
         }
+        MappedData &data = data_oe.value();
 
         // Return early if buffer is already mapped
         if (data.bases[0] != nullptr) {
-            return 0;
+            return true;
         }
 
         if (!dmabuf_sanity_check(handle)) {
-            return -EINVAL;
+            return false;
         }
 
+        private_handle_t *hnd =
+                reinterpret_cast<private_handle_t *>(const_cast<native_handle *>(handle));
         data.bases = mali_gralloc_ion_map(hnd);
         if (data.bases[0] == nullptr) {
-            return -EINVAL;
+            return false;
         }
 
         for (auto i = 0; i < MAX_BUFFER_FDS; i++) {
             data.alloc_sizes[i] = hnd->alloc_sizes[i];
         }
 
-        return 0;
+        return true;
     }
 
-    int validate_locked(buffer_handle_t handle) REQUIRES(lock) {
+    bool map_metadata_locked(buffer_handle_t handle) REQUIRES(lock) {
+        auto data_oe = get_validated_data_locked(handle);
+        if (!data_oe.has_value()) {
+            return false;
+        }
+        MappedData &data = data_oe.value();
+
+        // Return early if buffer is already mapped
+        if (data.metadata_vaddr != nullptr) {
+            return true;
+        }
+
+        if (!dmabuf_sanity_check(handle, /*skip_buffer_size_check=*/true)) {
+            return false;
+        }
+
+        private_handle_t *hnd =
+                reinterpret_cast<private_handle_t *>(const_cast<native_handle *>(handle));
+        data.metadata_vaddr = mmap(nullptr, hnd->attr_size, PROT_READ | PROT_WRITE, MAP_SHARED,
+                                   hnd->get_share_attr_fd(), 0);
+        if (data.metadata_vaddr == nullptr) {
+            return false;
+        }
+
+        data.metadata_size = hnd->attr_size;
+        return true;
+    }
+
+    bool validate_locked(buffer_handle_t handle) REQUIRES(lock) {
         if (private_handle_t::validate(handle) < 0) {
             MALI_GRALLOC_LOGE("Reference invalid buffer %p, returning error", handle);
-            return -EINVAL;
+            return false;
         }
 
         const auto *hnd = reinterpret_cast<private_handle_t *>(const_cast<native_handle *>(handle));
         auto it = buffer_map.find(hnd);
         if (it == buffer_map.end()) {
             MALI_GRALLOC_LOGE("Reference unimported buffer %p, returning error", handle);
-            return -EINVAL;
+            return false;
         }
 
         auto &data = *(it->second.get());
@@ -148,19 +179,41 @@ private:
                 if (data.alloc_sizes[i] != hnd->alloc_sizes[i]) {
                     MALI_GRALLOC_LOGE(
                             "Validation failed: Buffer attributes inconsistent with mapper");
-                    return -EINVAL;
+                    return false;
                 }
             }
         } else {
             for (auto i = 0; i < MAX_BUFFER_FDS; i++) {
                 if (data.bases[i] != nullptr) {
                     MALI_GRALLOC_LOGE("Validation failed: Expected nullptr for unmapped buffer");
-                    return -EINVAL;
+                    return false;
                 }
             }
         }
 
-        return 0;
+        return true;
+    }
+
+    std::optional<std::reference_wrapper<MappedData>> get_validated_data_locked(
+            buffer_handle_t handle) REQUIRES(lock) {
+        if (!validate_locked(handle)) {
+            return {};
+        }
+
+        private_handle_t *hnd =
+                reinterpret_cast<private_handle_t *>(const_cast<native_handle *>(handle));
+        auto it = buffer_map.find(hnd);
+        if (it == buffer_map.end()) {
+            MALI_GRALLOC_LOGE("Trying to release a non-imported buffer");
+            return {};
+        }
+
+        MappedData &data = *(it->second.get());
+        if (data.ref_count == 0) {
+            MALI_GRALLOC_LOGE("BUG: Found an imported buffer with ref count 0, expect errors");
+        }
+
+        return data;
     }
 
 public:
@@ -200,35 +253,22 @@ public:
 
     int map(buffer_handle_t handle) EXCLUDES(lock) {
         std::lock_guard<std::mutex> _l(lock);
-        auto error = validate_locked(handle);
-        if (error != 0) {
-            return error;
+        if (!map_buffer_locked(handle)) {
+            return -EINVAL;
         }
 
-        return map_locked(handle);
+        return 0;
     }
 
     int release(buffer_handle_t handle) EXCLUDES(lock) {
         std::lock_guard<std::mutex> _l(lock);
 
-        // Always call locked variant of validate from this function. On calling
-        // the other validate variant, an attacker might launch a timing attack
-        // where they would try to time their attack between the return of
-        // validate and before taking the lock in this function again.
-        auto error = validate_locked(handle);
-        if (error != 0) {
-            return error;
-        }
-
-        private_handle_t *hnd =
-                reinterpret_cast<private_handle_t *>(const_cast<native_handle *>(handle));
-        auto it = buffer_map.find(hnd);
-        if (it == buffer_map.end()) {
-            MALI_GRALLOC_LOGE("Trying to release a non-imported buffer");
+        auto data_oe = get_validated_data_locked(handle);
+        if (!data_oe.has_value()) {
             return -EINVAL;
         }
+        MappedData &data = data_oe.value();
 
-        auto &data = *(it->second.get());
         if (data.ref_count == 0) {
             MALI_GRALLOC_LOGE("BUG: Reference held for buffer whose counter is 0");
             return -EINVAL;
@@ -236,15 +276,19 @@ public:
 
         data.ref_count--;
         if (data.ref_count == 0) {
+            private_handle_t *hnd =
+                    reinterpret_cast<private_handle_t *>(const_cast<native_handle *>(handle));
+            auto it = buffer_map.find(hnd);
+
             if (data.bases[0] != nullptr) {
                 mali_gralloc_ion_unmap(hnd, data.bases);
             }
 
-            /* TODO: Make this unmapping of shared meta fd into a function? */
-            if (hnd->attr_base) {
-                munmap(hnd->attr_base, hnd->attr_size);
-                hnd->attr_base = nullptr;
+            if (data.metadata_vaddr != nullptr) {
+                munmap(data.metadata_vaddr, data.metadata_size);
+                data.metadata_vaddr = nullptr;
             }
+
             buffer_map.erase(it);
         }
 
@@ -253,26 +297,23 @@ public:
 
     int validate(buffer_handle_t handle) EXCLUDES(lock) {
         std::lock_guard<std::mutex> _l(lock);
-        return validate_locked(handle);
+
+        if (!validate_locked(handle)) {
+            return -EINVAL;
+        }
+
+        return 0;
     }
 
     std::optional<void *> get_buf_addr(buffer_handle_t handle) {
         std::lock_guard<std::mutex> _l(lock);
 
-        auto error = validate_locked(handle);
-        if (error != 0) {
-            return {};
-        }
-
-        private_handle_t *hnd =
-                reinterpret_cast<private_handle_t *>(const_cast<native_handle *>(handle));
-        auto it = buffer_map.find(hnd);
-        if (it == buffer_map.end()) {
-            MALI_GRALLOC_LOGE("BUG: Cannot find a validated buffer");
+        auto data_oe = get_validated_data_locked(handle);
+        if (!data_oe.has_value()) {
             return {};
         }
+        MappedData &data = data_oe.value();
 
-        const auto &data = *(it->second.get());
         if (data.bases[0] == nullptr) {
             MALI_GRALLOC_LOGE("BUG: Called %s for an un-mapped buffer", __FUNCTION__);
             return {};
@@ -280,6 +321,24 @@ public:
 
         return data.bases[0];
     }
+
+    std::optional<void *> get_metadata_addr(buffer_handle_t handle) {
+        std::lock_guard<std::mutex> _l(lock);
+
+        auto data_oe = get_validated_data_locked(handle);
+        if (!data_oe.has_value()) {
+            return {};
+        }
+        MappedData &data = data_oe.value();
+
+        if (data.metadata_vaddr == nullptr) {
+            if (!map_metadata_locked(handle)) {
+                return {};
+            }
+        }
+
+        return data.metadata_vaddr;
+    }
 };
 
 int mali_gralloc_reference_retain(buffer_handle_t handle) {
@@ -301,3 +360,7 @@ int mali_gralloc_reference_validate(buffer_handle_t handle) {
 std::optional<void *> mali_gralloc_reference_get_buf_addr(buffer_handle_t handle) {
     return BufferManager::getInstance().get_buf_addr(handle);
 }
+
+std::optional<void *> mali_gralloc_reference_get_metadata_addr(buffer_handle_t handle) {
+    return BufferManager::getInstance().get_metadata_addr(handle);
+}
diff --git a/gralloc4/src/core/mali_gralloc_reference.h b/gralloc4/src/core/mali_gralloc_reference.h
index 257bb6b..bddcead 100644
--- a/gralloc4/src/core/mali_gralloc_reference.h
+++ b/gralloc4/src/core/mali_gralloc_reference.h
@@ -28,5 +28,6 @@ int mali_gralloc_reference_validate(buffer_handle_t handle);
 int mali_gralloc_reference_map(buffer_handle_t handle);
 
 std::optional<void*> mali_gralloc_reference_get_buf_addr(buffer_handle_t handle);
+std::optional<void*> mali_gralloc_reference_get_metadata_addr(buffer_handle_t handle);
 
 #endif /* MALI_GRALLOC_REFERENCE_H_ */
diff --git a/gralloc4/src/hidl_common/Allocator.cpp b/gralloc4/src/hidl_common/Allocator.cpp
index 3b8e62a..29cf3ca 100644
--- a/gralloc4/src/hidl_common/Allocator.cpp
+++ b/gralloc4/src/hidl_common/Allocator.cpp
@@ -20,14 +20,13 @@
 
 #include <utils/Trace.h>
 
-#include "SharedMetadata.h"
 #include "Allocator.h"
 #include "core/mali_gralloc_bufferallocation.h"
 #include "core/mali_gralloc_bufferdescriptor.h"
 #include "core/format_info.h"
 #include "allocator/mali_gralloc_ion.h"
-#include "allocator/mali_gralloc_shared_memory.h"
 #include "gralloc_priv.h"
+#include "SharedMetadata.h"
 
 namespace arm
 {
@@ -99,29 +98,27 @@ void allocate(const buffer_descriptor_t &bufferDescriptor, uint32_t count, IAllo
 			mali_gralloc_ion_allocate_attr(hnd);
 
 			/* TODO: error check for failure */
-			hnd->attr_base = mmap(nullptr, hnd->attr_size, PROT_READ | PROT_WRITE,
+			void* metadata_vaddr = mmap(nullptr, hnd->attr_size, PROT_READ | PROT_WRITE,
 					MAP_SHARED, hnd->get_share_attr_fd(), 0);
 
-			memset(hnd->attr_base, 0, hnd->attr_size);
+			memset(metadata_vaddr, 0, hnd->attr_size);
 
-			mapper::common::shared_metadata_init(hnd->attr_base, bufferDescriptor.name);
+			mapper::common::shared_metadata_init(metadata_vaddr, bufferDescriptor.name);
 
 			const uint32_t base_format = bufferDescriptor.alloc_format & MALI_GRALLOC_INTFMT_FMT_MASK;
 			const uint64_t usage = bufferDescriptor.consumer_usage | bufferDescriptor.producer_usage;
 			android_dataspace_t dataspace;
 			get_format_dataspace(base_format, usage, hnd->width, hnd->height, &dataspace);
 
-			mapper::common::set_dataspace(hnd, static_cast<mapper::common::Dataspace>(dataspace));
-
-			/*
-			 * We need to set attr_base to MAP_FAILED before the HIDL callback
-			 * to avoid sending an invalid pointer to the client process.
-			 *
-			 * hnd->attr_base = mmap(...);
-			 * hidl_callback(hnd); // client receives hnd->attr_base = <dangling pointer>
-			 */
-			munmap(hnd->attr_base, hnd->attr_size);
-			hnd->attr_base = 0;
+			// TODO: set_dataspace API in mapper expects a buffer to be first imported before it can set the dataspace
+			{
+				using mapper::common::shared_metadata;
+				using mapper::common::aligned_optional;
+				using mapper::common::Dataspace;
+				(static_cast<shared_metadata*>(metadata_vaddr))->dataspace = aligned_optional(static_cast<Dataspace>(dataspace));
+			}
+
+			munmap(metadata_vaddr, hnd->attr_size);
 		}
 
 		int tmpStride = 0;
diff --git a/gralloc4/src/hidl_common/Mapper.cpp b/gralloc4/src/hidl_common/Mapper.cpp
index aa8e6d2..38d5377 100644
--- a/gralloc4/src/hidl_common/Mapper.cpp
+++ b/gralloc4/src/hidl_common/Mapper.cpp
@@ -286,15 +286,6 @@ void importBuffer(const hidl_handle& rawHandle, IMapper::importBuffer_cb hidl_cb
 	}
 
 	auto *private_handle = static_cast<private_handle_t *>(bufferHandle);
-	private_handle->attr_base = mmap(nullptr, private_handle->attr_size, PROT_READ | PROT_WRITE,
-	                                 MAP_SHARED, private_handle->get_share_attr_fd(), 0);
-	if (private_handle->attr_base == MAP_FAILED)
-	{
-		native_handle_close(bufferHandle);
-		native_handle_delete(bufferHandle);
-		hidl_cb(Error::NO_RESOURCES, nullptr);
-		return;
-	}
 
 	if (gRegisteredHandles->add(bufferHandle) == false)
 	{
@@ -324,16 +315,6 @@ Error freeBuffer(void* buffer)
 		return Error::BAD_BUFFER;
 	}
 
-	{
-		auto *private_handle = static_cast<private_handle_t *>(bufferHandle);
-		int ret = munmap(private_handle->attr_base, private_handle->attr_size);
-		if (ret < 0)
-		{
-			MALI_GRALLOC_LOGW("munmap: %s", strerror(errno));
-		}
-		private_handle->attr_base = MAP_FAILED;
-	}
-
 	const Error status = unregisterBuffer(bufferHandle);
 	if (status != Error::NONE)
 	{
@@ -765,7 +746,13 @@ void getReservedRegion(void *buffer, IMapper::getReservedRegion_cb hidl_cb)
 		hidl_cb(Error::BAD_BUFFER, 0, 0);
 		return;
 	}
-	void *reserved_region = static_cast<std::byte *>(handle->attr_base)
+
+	auto metadata_addr_oe = mali_gralloc_reference_get_metadata_addr(handle);
+	if (!metadata_addr_oe.has_value()) {
+		hidl_cb(Error::BAD_BUFFER, 0, 0);
+	}
+
+	void *reserved_region = static_cast<std::byte *>(metadata_addr_oe.value())
 	    + mapper::common::shared_metadata_size();
 	hidl_cb(Error::NONE, reserved_region, handle->reserved_region_size);
 }
diff --git a/gralloc4/src/hidl_common/SharedMetadata.cpp b/gralloc4/src/hidl_common/SharedMetadata.cpp
index 27ed972..6960b08 100644
--- a/gralloc4/src/hidl_common/SharedMetadata.cpp
+++ b/gralloc4/src/hidl_common/SharedMetadata.cpp
@@ -17,6 +17,7 @@
  */
 
 #include "SharedMetadata.h"
+#include "core/mali_gralloc_reference.h"
 #include "mali_gralloc_log.h"
 #include "mali_gralloc_usages.h"
 //#include <VendorVideoAPI.h>
@@ -40,19 +41,19 @@ size_t shared_metadata_size()
 
 void get_name(const private_handle_t *hnd, std::string *name)
 {
-	auto *metadata = reinterpret_cast<const shared_metadata *>(hnd->attr_base);
+	auto *metadata = reinterpret_cast<shared_metadata *>(mali_gralloc_reference_get_metadata_addr(hnd).value());
 	*name = metadata->get_name();
 }
 
 void get_crop_rect(const private_handle_t *hnd, std::optional<Rect> *crop)
 {
-	auto *metadata = reinterpret_cast<const shared_metadata *>(hnd->attr_base);
+	auto *metadata = reinterpret_cast<shared_metadata *>(mali_gralloc_reference_get_metadata_addr(hnd).value());
 	*crop = metadata->crop.to_std_optional();
 }
 
 android::status_t set_crop_rect(const private_handle_t *hnd, const Rect &crop)
 {
-	auto *metadata = reinterpret_cast<shared_metadata *>(hnd->attr_base);
+	auto *metadata = reinterpret_cast<shared_metadata *>(mali_gralloc_reference_get_metadata_addr(hnd).value());
 
 	if (crop.top < 0 || crop.left < 0 ||
 	    crop.left > crop.right || crop.right > hnd->plane_info[0].alloc_width ||
@@ -70,31 +71,31 @@ android::status_t set_crop_rect(const private_handle_t *hnd, const Rect &crop)
 
 void get_dataspace(const private_handle_t *hnd, std::optional<Dataspace> *dataspace)
 {
-	auto *metadata = reinterpret_cast<const shared_metadata *>(hnd->attr_base);
+	auto *metadata = reinterpret_cast<shared_metadata *>(mali_gralloc_reference_get_metadata_addr(hnd).value());
 	*dataspace = metadata->dataspace.to_std_optional();
 }
 
 void set_dataspace(const private_handle_t *hnd, const Dataspace &dataspace)
 {
-	auto *metadata = reinterpret_cast<shared_metadata *>(hnd->attr_base);
+	auto *metadata = reinterpret_cast<shared_metadata *>(mali_gralloc_reference_get_metadata_addr(hnd).value());
 	metadata->dataspace = aligned_optional(dataspace);
 }
 
 void get_blend_mode(const private_handle_t *hnd, std::optional<BlendMode> *blend_mode)
 {
-	auto *metadata = reinterpret_cast<const shared_metadata *>(hnd->attr_base);
+	auto *metadata = reinterpret_cast<shared_metadata *>(mali_gralloc_reference_get_metadata_addr(hnd).value());
 	*blend_mode = metadata->blend_mode.to_std_optional();
 }
 
 void set_blend_mode(const private_handle_t *hnd, const BlendMode &blend_mode)
 {
-	auto *metadata = reinterpret_cast<shared_metadata *>(hnd->attr_base);
+	auto *metadata = reinterpret_cast<shared_metadata *>(mali_gralloc_reference_get_metadata_addr(hnd).value());
 	metadata->blend_mode = aligned_optional(blend_mode);
 }
 
 void get_smpte2086(const private_handle_t *hnd, std::optional<Smpte2086> *smpte2086)
 {
-	auto *metadata = reinterpret_cast<const shared_metadata *>(hnd->attr_base);
+	auto *metadata = reinterpret_cast<shared_metadata *>(mali_gralloc_reference_get_metadata_addr(hnd).value());
 	*smpte2086 = metadata->smpte2086.to_std_optional();
 }
 
@@ -105,7 +106,7 @@ android::status_t set_smpte2086(const private_handle_t *hnd, const std::optional
 		return android::BAD_VALUE;
 	}
 
-	auto *metadata = reinterpret_cast<shared_metadata *>(hnd->attr_base);
+	auto *metadata = reinterpret_cast<shared_metadata *>(mali_gralloc_reference_get_metadata_addr(hnd).value());
 	metadata->smpte2086 = aligned_optional(smpte2086);
 
 	return android::OK;
@@ -113,7 +114,7 @@ android::status_t set_smpte2086(const private_handle_t *hnd, const std::optional
 
 void get_cta861_3(const private_handle_t *hnd, std::optional<Cta861_3> *cta861_3)
 {
-	auto *metadata = reinterpret_cast<const shared_metadata *>(hnd->attr_base);
+	auto *metadata = reinterpret_cast<shared_metadata *>(mali_gralloc_reference_get_metadata_addr(hnd).value());
 	*cta861_3 = metadata->cta861_3.to_std_optional();
 }
 
@@ -124,7 +125,7 @@ android::status_t set_cta861_3(const private_handle_t *hnd, const std::optional<
 		return android::BAD_VALUE;
 	}
 
-	auto *metadata = reinterpret_cast<shared_metadata *>(hnd->attr_base);
+	auto *metadata = reinterpret_cast<shared_metadata *>(mali_gralloc_reference_get_metadata_addr(hnd).value());
 	metadata->cta861_3 = aligned_optional(cta861_3);
 
 	return android::OK;
@@ -132,7 +133,7 @@ android::status_t set_cta861_3(const private_handle_t *hnd, const std::optional<
 
 void get_smpte2094_40(const private_handle_t *hnd, std::optional<std::vector<uint8_t>> *smpte2094_40)
 {
-	auto *metadata = reinterpret_cast<const shared_metadata *>(hnd->attr_base);
+	auto *metadata = reinterpret_cast<shared_metadata *>(mali_gralloc_reference_get_metadata_addr(hnd).value());
 	if (metadata->smpte2094_40.size > 0)
 	{
 		const uint8_t *begin = metadata->smpte2094_40.data();
@@ -147,7 +148,7 @@ void get_smpte2094_40(const private_handle_t *hnd, std::optional<std::vector<uin
 
 android::status_t set_smpte2094_40(const private_handle_t *hnd, const std::optional<std::vector<uint8_t>> &smpte2094_40)
 {
-	auto *metadata = reinterpret_cast<shared_metadata *>(hnd->attr_base);
+	auto *metadata = reinterpret_cast<shared_metadata *>(mali_gralloc_reference_get_metadata_addr(hnd).value());
 	const size_t size = smpte2094_40.has_value() ? smpte2094_40->size() : 0;
 	if (size > metadata->smpte2094_40.capacity())
 	{
@@ -162,7 +163,7 @@ android::status_t set_smpte2094_40(const private_handle_t *hnd, const std::optio
 }
 
 void* get_video_hdr(const private_handle_t *hnd) {
-	auto *metadata = reinterpret_cast<shared_metadata *>(hnd->attr_base);
+	auto *metadata = reinterpret_cast<shared_metadata *>(mali_gralloc_reference_get_metadata_addr(hnd).value());
 	return &(metadata->video_private_data);
 }
 
@@ -170,7 +171,8 @@ void* get_video_roiinfo(const private_handle_t *hnd) {
 	if (!(hnd->get_usage() & GRALLOC_USAGE_ROIINFO))
 		return nullptr;
 
-	return static_cast<char*>(hnd->attr_base) + sizeof(shared_metadata) + hnd->reserved_region_size;
+	auto *metadata = static_cast<char*>(mali_gralloc_reference_get_metadata_addr(hnd).value());
+	return metadata + sizeof(shared_metadata) + hnd->reserved_region_size;
 }
 } // namespace common
 } // namespace mapper
diff --git a/gralloc4/src/hidl_common/SharedMetadata.h b/gralloc4/src/hidl_common/SharedMetadata.h
index 9961733..f3aad7c 100644
--- a/gralloc4/src/hidl_common/SharedMetadata.h
+++ b/gralloc4/src/hidl_common/SharedMetadata.h
@@ -23,7 +23,6 @@
 #include <vector>
 #include "gralloctypes/Gralloc4.h"
 #include "mali_gralloc_buffer.h"
-#include "allocator/mali_gralloc_shared_memory.h"
 #include "core/mali_gralloc_bufferdescriptor.h"
 #include "gralloc_helper.h"
 
diff --git a/gralloc4/src/libGralloc4Wrapper/wrapper.cpp b/gralloc4/src/libGralloc4Wrapper/wrapper.cpp
index 7d286e0..f9b1b9c 100644
--- a/gralloc4/src/libGralloc4Wrapper/wrapper.cpp
+++ b/gralloc4/src/libGralloc4Wrapper/wrapper.cpp
@@ -5,7 +5,6 @@
 #include "core/format_info.h"
 #include "core/mali_gralloc_bufferdescriptor.h"
 #include "core/mali_gralloc_bufferallocation.h"
-#include "core/mali_gralloc_reference.h"
 #include "allocator/mali_gralloc_ion.h"
 #include "hidl_common/SharedMetadata.h"
 #include "gralloc_priv.h"
@@ -158,27 +157,32 @@ buffer_handle_t createNativeHandle(const Descriptor &descriptor) {
     }
 
     {
-        hnd->attr_base = mmap(nullptr, hnd->attr_size, PROT_READ | PROT_WRITE,
+        auto metadata_vaddr = mmap(nullptr, hnd->attr_size, PROT_READ | PROT_WRITE,
                 MAP_SHARED, hnd->get_share_attr_fd(), 0);
-        if (hnd->attr_base == MAP_FAILED) {
+        if (metadata_vaddr == MAP_FAILED) {
             ALOGE("mmap hnd->get_share_attr_fd() failed");
             mali_gralloc_buffer_free(tmp_buffer);
             return nullptr;
         }
 
-        memset(hnd->attr_base, 0, hnd->attr_size);
+        memset(metadata_vaddr, 0, hnd->attr_size);
 
-        arm::mapper::common::shared_metadata_init(hnd->attr_base, buffer_descriptor.name);
+        arm::mapper::common::shared_metadata_init(metadata_vaddr, buffer_descriptor.name);
 
         const uint32_t base_format = buffer_descriptor.alloc_format & MALI_GRALLOC_INTFMT_FMT_MASK;
         const uint64_t usage = buffer_descriptor.consumer_usage | buffer_descriptor.producer_usage;
         android_dataspace_t dataspace;
         get_format_dataspace(base_format, usage, hnd->width, hnd->height, &dataspace);
 
-        arm::mapper::common::set_dataspace(hnd, static_cast<arm::mapper::common::Dataspace>(dataspace));
+        {
+            using arm::mapper::common::aligned_optional;
+            using arm::mapper::common::Dataspace;
+            using arm::mapper::common::shared_metadata;
+            (static_cast<shared_metadata *>(metadata_vaddr))->dataspace =
+                    aligned_optional(static_cast<Dataspace>(dataspace));
+        }
 
-        munmap(hnd->attr_base, hnd->attr_size);
-        hnd->attr_base = 0;
+        munmap(metadata_vaddr, hnd->attr_size);
     }
 
     // TODO(modan@, handle all plane offsets)
diff --git a/gralloc4/src/mali_gralloc_buffer.h b/gralloc4/src/mali_gralloc_buffer.h
index 11c41f5..09461d7 100644
--- a/gralloc4/src/mali_gralloc_buffer.h
+++ b/gralloc4/src/mali_gralloc_buffer.h
@@ -238,7 +238,6 @@ struct private_handle_t
 	int ion_handles[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);
 
 	/* Size of the attribute shared region in bytes. */