diff options
author | Ankit Goyal <layog@google.com> | 2022-03-24 07:14:17 +0000 |
---|---|---|
committer | Presubmit Automerger Backend <android-build-presubmit-automerger-backend@system.gserviceaccount.com> | 2022-03-24 07:14:17 +0000 |
commit | 639cd76b264cc13104faf9c2b9428b1a5ee094ef (patch) | |
tree | 3bc89c0eb60c9d66ab219be23f481a18bc54dbf3 | |
parent | 1b834a2d9c41e183cf69194c67f7511aff47942e (diff) | |
parent | d4a075ecad69f9d8abc6a7dc49a0640bd3ac9cd5 (diff) | |
download | gchips-639cd76b264cc13104faf9c2b9428b1a5ee094ef.tar.gz |
[automerge] Validate all fds as fd_count cannot be relied upon 2p: d4a075ecad
Original change: https://googleplex-android-review.googlesource.com/c/platform/hardware/google/gchips/+/17352147
Bug: 212803946
Bug: 212804042
Change-Id: Iee7404e829893517b56f4fc80a2dd97e35a7516e
Merged-In: I7ab56cd3126f57ae0edebe2707c4e24cdab6196a
-rw-r--r-- | gralloc4/Android.bp | 1 | ||||
-rw-r--r-- | gralloc4/src/Android.bp | 2 | ||||
-rw-r--r-- | gralloc4/src/allocator/mali_gralloc_ion.cpp | 3 | ||||
-rw-r--r-- | gralloc4/src/core/Android.bp | 1 | ||||
-rw-r--r-- | gralloc4/src/core/exynos_format_allocation.h | 2 | ||||
-rw-r--r-- | gralloc4/src/core/mali_gralloc_bufferaccess.cpp | 30 | ||||
-rw-r--r-- | gralloc4/src/core/mali_gralloc_bufferallocation.cpp | 1 | ||||
-rw-r--r-- | gralloc4/src/core/mali_gralloc_debug.cpp | 136 | ||||
-rw-r--r-- | gralloc4/src/core/mali_gralloc_debug.h | 33 | ||||
-rw-r--r-- | gralloc4/src/core/mali_gralloc_reference.cpp | 339 | ||||
-rw-r--r-- | gralloc4/src/core/mali_gralloc_reference.h | 2 | ||||
-rw-r--r-- | gralloc4/src/hidl_common/Mapper.cpp | 2 | ||||
-rw-r--r-- | gralloc4/src/mali_gralloc_buffer.h | 40 | ||||
-rw-r--r-- | libvendorgraphicbuffer/gralloc4/vendor_graphicbuffer_meta.cpp | 7 |
14 files changed, 244 insertions, 355 deletions
diff --git a/gralloc4/Android.bp b/gralloc4/Android.bp index c30c359..5d2f785 100644 --- a/gralloc4/Android.bp +++ b/gralloc4/Android.bp @@ -63,6 +63,7 @@ arm_gralloc_cc_defaults { cflags: [ "-Wundef", "-Werror", + "-Wthread-safety", "-DGRALLOC_LIBRARY_BUILD=1", "-DDISABLE_FRAMEBUFFER_HAL=1", "-DGRALLOC_USE_LEGACY_CALCS=0", diff --git a/gralloc4/src/Android.bp b/gralloc4/src/Android.bp index a3a254d..2de8907 100644 --- a/gralloc4/src/Android.bp +++ b/gralloc4/src/Android.bp @@ -47,11 +47,11 @@ cc_library_shared { "core/mali_gralloc_bufferallocation.cpp", "core/mali_gralloc_bufferdescriptor.cpp", "core/mali_gralloc_reference.cpp", - "core/mali_gralloc_debug.cpp", ":libgralloc_hidl_common_shared_metadata", ], cflags: [ "-DGRALLOC_LIBRARY_BUILD=1", + "-Wthread-safety", ], static_libs: [ "libgralloc_capabilities", 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/Android.bp b/gralloc4/src/core/Android.bp index 023962c..b01b0b7 100644 --- a/gralloc4/src/core/Android.bp +++ b/gralloc4/src/core/Android.bp @@ -59,7 +59,6 @@ arm_gralloc_core_cc_defaults { "mali_gralloc_bufferdescriptor.cpp", "mali_gralloc_formats.cpp", "mali_gralloc_reference.cpp", - "mali_gralloc_debug.cpp", "format_info.cpp", ], include_dirs: [ diff --git a/gralloc4/src/core/exynos_format_allocation.h b/gralloc4/src/core/exynos_format_allocation.h index 30b2c15..bcaadfb 100644 --- a/gralloc4/src/core/exynos_format_allocation.h +++ b/gralloc4/src/core/exynos_format_allocation.h @@ -22,6 +22,8 @@ #include "mfc_macros_local.h" #endif +#include <gralloc_priv.h> + #define PLANE_SIZE(w, h) ((w) * (h)) #define S2B_PLANE_SIZE(w, h) (GRALLOC_ALIGN((w) / 4, 16) * (GRALLOC_ALIGN(h, 16))) diff --git a/gralloc4/src/core/mali_gralloc_bufferaccess.cpp b/gralloc4/src/core/mali_gralloc_bufferaccess.cpp index 6740556..adda46b 100644 --- a/gralloc4/src/core/mali_gralloc_bufferaccess.cpp +++ b/gralloc4/src/core/mali_gralloc_bufferaccess.cpp @@ -113,8 +113,6 @@ int validate_lock_input_parameters(const buffer_handle_t buffer, const int l, const int t, const int w, const int h, uint64_t usage) { - bool is_registered_process = false; - const int lock_pid = getpid(); const private_handle_t * const hnd = (private_handle_t *)buffer; /* TODO: do not check access region for blob formats. @@ -126,8 +124,7 @@ int validate_lock_input_parameters(const buffer_handle_t buffer, const int l, if ((l < 0) || (t < 0) || (w < 0) || (h < 0)) { MALI_GRALLOC_LOGW("Negative values for access region (l = %d t = %d w = %d and " - "h = %d) in buffer lock request are invalid. Locking PID:%d", - l, t, w, h, lock_pid); + "h = %d) in buffer lock request are invalid.", l, t, w, h); return -EINVAL; } @@ -135,8 +132,7 @@ int validate_lock_input_parameters(const buffer_handle_t buffer, const int l, if (((l + w) < 0) || ((t + h) < 0)) { MALI_GRALLOC_LOGW("Encountered overflow with access region (l = %d t = %d w = %d and" - " h = %d) in buffer lock request. Locking PID:%d", - l, t, w, h, lock_pid); + " h = %d) in buffer lock request.", l, t, w, h); return -EINVAL; } @@ -144,28 +140,12 @@ int validate_lock_input_parameters(const buffer_handle_t buffer, const int l, if (((t + h) > hnd->height) || ((l + w) > hnd->width)) { MALI_GRALLOC_LOGW("Buffer lock access region (l = %d t = %d w = %d " - "and h = %d) is outside allocated buffer (width = %d and height = %d)" - " Locking PID:%d", l, t, w, h, hnd->width, hnd->height, lock_pid); + "and h = %d) is outside allocated buffer (width = %d and height = %d)", + l, t, w, h, hnd->width, hnd->height); return -EINVAL; } } - /* Locking process should have a valid buffer virtual address. A process - * will have a valid buffer virtual address if it is the allocating - * process or it retained / registered a cloned buffer handle - */ - if ((hnd->allocating_pid == lock_pid) || (hnd->remote_pid == lock_pid)) - { - is_registered_process = true; - } - - /* TODO: differentiate between mappable and unmappable(secure, hfr etc) buffers */ - if (is_registered_process == false) - { - MALI_GRALLOC_LOGE("The buffer must be retained before lock request"); - return -EINVAL; - } - /* Reject lock requests for AFBC (compressed format) enabled buffers */ if ((hnd->alloc_format & MALI_GRALLOC_INTFMT_EXT_MASK) != 0) { @@ -210,7 +190,7 @@ int mali_gralloc_lock(buffer_handle_t buffer, { int status; - if (private_handle_t::validate(buffer) < 0) + if (mali_gralloc_reference_validate(buffer)) { MALI_GRALLOC_LOGE("Locking invalid buffer %p, returning error", buffer); return -EINVAL; diff --git a/gralloc4/src/core/mali_gralloc_bufferallocation.cpp b/gralloc4/src/core/mali_gralloc_bufferallocation.cpp index d82ff8c..704812e 100644 --- a/gralloc4/src/core/mali_gralloc_bufferallocation.cpp +++ b/gralloc4/src/core/mali_gralloc_bufferallocation.cpp @@ -29,7 +29,6 @@ #include "allocator/mali_gralloc_shared_memory.h" #include "mali_gralloc_buffer.h" #include "mali_gralloc_bufferdescriptor.h" -#include "mali_gralloc_debug.h" #include "mali_gralloc_log.h" #include "format_info.h" #include <exynos_format.h> diff --git a/gralloc4/src/core/mali_gralloc_debug.cpp b/gralloc4/src/core/mali_gralloc_debug.cpp deleted file mode 100644 index cc3bc7f..0000000 --- a/gralloc4/src/core/mali_gralloc_debug.cpp +++ /dev/null @@ -1,136 +0,0 @@ -/* - * Copyright (C) 2016, 2018 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 <inttypes.h> -#include <stdlib.h> -#include <vector> -#include <algorithm> - -#include <hardware/hardware.h> - -#include "mali_gralloc_debug.h" - -static pthread_mutex_t dump_lock = PTHREAD_MUTEX_INITIALIZER; -static std::vector<private_handle_t *> dump_buffers; -static android::String8 dumpStrings; - -void mali_gralloc_dump_buffer_add(private_handle_t *handle) -{ - if (NULL == handle) - { - MALI_GRALLOC_LOGE("Invalid handle %p and return", handle); - return; - } - - pthread_mutex_lock(&dump_lock); - dump_buffers.push_back(handle); - pthread_mutex_unlock(&dump_lock); -} - -void mali_gralloc_dump_buffer_erase(private_handle_t *handle) -{ - if (NULL == handle) - { - MALI_GRALLOC_LOGE("Invalid handle %p and return", handle); - return; - } - - pthread_mutex_lock(&dump_lock); - dump_buffers.erase(std::remove(dump_buffers.begin(), dump_buffers.end(), handle), dump_buffers.end()); - pthread_mutex_unlock(&dump_lock); -} - -void mali_gralloc_dump_string(android::String8 &buf, const char *fmt, ...) -{ - va_list args; - va_start(args, fmt); - buf.appendFormatV(fmt, args); - va_end(args); -} - -void mali_gralloc_dump_buffers(android::String8 &dumpStrings, uint32_t *outSize) -{ - if (NULL == outSize) - { - MALI_GRALLOC_LOGE("Invalid pointer to dump buffer size and return"); - return; - } - - dumpStrings.clear(); - mali_gralloc_dump_string(dumpStrings, - "-------------------------Start to dump Gralloc buffers info------------------------\n"); - private_handle_t *hnd; - size_t num; - - mali_gralloc_dump_string(dumpStrings, " handle | width | height | stride | req format |alloc " - "format|consumer usage|producer usage| shared fd | AFBC " - "|\n"); - mali_gralloc_dump_string(dumpStrings, "------------+-------+--------+--------+----------------+---------------+----" - "----------+--------------+-----------+------+\n"); - pthread_mutex_lock(&dump_lock); - - for (num = 0; num < dump_buffers.size(); num++) - { - hnd = dump_buffers[num]; - mali_gralloc_dump_string(dumpStrings, " %08" PRIxPTR " | %5d | %5d | %5d | %08x | %09" PRIx64 - " | %09" PRIx64 " | %09" PRIx64 " | %08x | %4d |\n", - hnd, hnd->width, hnd->height, hnd->stride, hnd->req_format, hnd->alloc_format, - hnd->consumer_usage, hnd->producer_usage, hnd->fds[0], - (hnd->alloc_format & MALI_GRALLOC_INTFMT_AFBCENABLE_MASK) ? true : false); - } - - pthread_mutex_unlock(&dump_lock); - mali_gralloc_dump_string( - dumpStrings, "---------------------End dump Gralloc buffers info with num %zu----------------------\n", num); - - *outSize = dumpStrings.size(); -} - -void mali_gralloc_dump_internal(uint32_t *outSize, char *outBuffer) -{ - uint32_t dumpSize; - - if (NULL == outSize) - { - MALI_GRALLOC_LOGE("Invalid pointer to dump buffer size and return"); - return; - } - - if (NULL == outBuffer) - { - if (!dumpStrings.isEmpty()) - { - dumpStrings.clear(); - } - - mali_gralloc_dump_buffers(dumpStrings, outSize); - } - else - { - if (dumpStrings.isEmpty()) - { - *outSize = 0; - } - else - { - dumpSize = dumpStrings.size(); - *outSize = (dumpSize < *outSize) ? dumpSize : *outSize; - memcpy(outBuffer, dumpStrings.string(), *outSize); - } - } -} diff --git a/gralloc4/src/core/mali_gralloc_debug.h b/gralloc4/src/core/mali_gralloc_debug.h deleted file mode 100644 index 9f99f0e..0000000 --- a/gralloc4/src/core/mali_gralloc_debug.h +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Copyright (C) 2016, 2018 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 MALI_GRALLOC_DEBUG_H_ -#define MALI_GRALLOC_DEBUG_H_ - -#include <utils/String8.h> -#include "gralloc_priv.h" - -#include <hardware/gralloc1.h> - -void mali_gralloc_dump_buffer_add(private_handle_t *handle); -void mali_gralloc_dump_buffer_erase(private_handle_t *handle); - -void mali_gralloc_dump_string(android::String8 &buf, const char *fmt, ...); -void mali_gralloc_dump_buffers(android::String8 &dumpBuffer, uint32_t *outSize); -void mali_gralloc_dump_internal(uint32_t *outSize, char *outBuffer); -#endif diff --git a/gralloc4/src/core/mali_gralloc_reference.cpp b/gralloc4/src/core/mali_gralloc_reference.cpp index 880f838..954c2b3 100644 --- a/gralloc4/src/core/mali_gralloc_reference.cpp +++ b/gralloc4/src/core/mali_gralloc_reference.cpp @@ -16,152 +16,223 @@ * limitations under the License. */ +#include "mali_gralloc_reference.h" + +#include <android-base/thread_annotations.h> #include <hardware/gralloc1.h> -#include "mali_gralloc_buffer.h" +#include <map> +#include <mutex> + #include "allocator/mali_gralloc_ion.h" #include "allocator/mali_gralloc_shared_memory.h" +#include "mali_gralloc_buffer.h" #include "mali_gralloc_bufferallocation.h" -#include "mali_gralloc_debug.h" -#include "mali_gralloc_reference.h" #include "mali_gralloc_usages.h" -static pthread_mutex_t s_map_lock = PTHREAD_MUTEX_INITIALIZER; - -int mali_gralloc_reference_retain(buffer_handle_t handle) -{ - if (private_handle_t::validate(handle) < 0) - { - MALI_GRALLOC_LOGE("Registering/Retaining invalid buffer %p, returning error", handle); - return -EINVAL; - } - - private_handle_t *hnd = (private_handle_t *)handle; - pthread_mutex_lock(&s_map_lock); - int retval = 0; - - if (hnd->allocating_pid == getpid() || hnd->remote_pid == getpid()) - { - hnd->ref_count++; - } - else - { - hnd->remote_pid = getpid(); - hnd->ref_count = 1; - - // Reset the handle bases, this is used to check if a buffer is mapped - for (int fidx = 0; fidx < hnd->fd_count; fidx++) { - hnd->bases[fidx] = 0; - } - } - - pthread_mutex_unlock(&s_map_lock); - - // TODO(b/187145254): CPU_READ/WRITE buffer is not being properly locked from - // MFC. This is a WA for the time being. - constexpr auto cpu_access_usage = ( - GRALLOC_USAGE_SW_WRITE_OFTEN | - GRALLOC_USAGE_SW_READ_OFTEN | - GRALLOC_USAGE_SW_WRITE_RARELY | - GRALLOC_USAGE_SW_READ_RARELY - ); - - if (hnd->get_usage() & cpu_access_usage) - retval = mali_gralloc_reference_map(handle); - - return retval; +class BufferManager { +private: + // This struct for now is just for validation and reference counting. When we + // are sure that private_handle_t::bases is not being used outside gralloc, this + // should become the only place where address mapping is maintained and can be + // queried from. + struct MappedData { + void *bases[MAX_BUFFER_FDS] = {}; + size_t alloc_sizes[MAX_BUFFER_FDS] = {}; + uint64_t ref_count = 0; + }; + + BufferManager() = default; + + std::mutex lock; + std::map<const private_handle_t *, std::unique_ptr<MappedData>> buffer_map GUARDED_BY(lock); + + static bool should_map_dmabuf(buffer_handle_t handle) { + private_handle_t *hnd = (private_handle_t *)handle; + + // TODO(b/187145254): CPU_READ/WRITE buffer is not being properly locked from + // MFC. This is a WA for the time being. + constexpr auto cpu_access_usage = + (GRALLOC_USAGE_SW_WRITE_OFTEN | GRALLOC_USAGE_SW_READ_OFTEN | + GRALLOC_USAGE_SW_WRITE_RARELY | GRALLOC_USAGE_SW_READ_RARELY); + return hnd->get_usage() & cpu_access_usage; + } + + int map_locked(buffer_handle_t handle) REQUIRES(lock) { + private_handle_t *hnd = (private_handle_t *)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"); + } + + if (data.bases[0] != nullptr) { + return 0; + } + + int error = mali_gralloc_ion_map(hnd); + if (error != 0) { + return error; + } + + 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]; + } + + return 0; + } + + int 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; + } + + const auto *hnd = (private_handle_t *)handle; + auto it = buffer_map.find(hnd); + if (it == buffer_map.end()) { + MALI_GRALLOC_LOGE("Reference unimported buffer %p, returning error", handle); + return -EINVAL; + } + + auto &data = *(it->second.get()); + if (data.bases[0] != nullptr) { + 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( + "Validation failed: Buffer attributes inconsistent with mapper"); + return -EINVAL; + } + } + } else { + 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; + } + } + } + + return 0; + } + +public: + static BufferManager &getInstance() { + static BufferManager instance; + return instance; + } + + int retain(buffer_handle_t handle) EXCLUDES(lock) { + if (private_handle_t::validate(handle) < 0) { + MALI_GRALLOC_LOGE("Registering/Retaining invalid buffer %p, returning error", handle); + return -EINVAL; + } + std::lock_guard<std::mutex> _l(lock); + + private_handle_t *hnd = (private_handle_t *)handle; + + auto it = buffer_map.find(hnd); + if (it == buffer_map.end()) { + bool success = false; + auto _data = std::make_unique<MappedData>(); + + std::tie(it, success) = buffer_map.insert({hnd, std::move(_data)}); + if (!success) { + MALI_GRALLOC_LOGE("Failed to create buffer data mapping"); + return -EINVAL; + } + + for (int i = 0; i < MAX_BUFFER_FDS; i++) { + hnd->bases[i] = 0; + } + } else if (it->second->ref_count == 0) { + MALI_GRALLOC_LOGE("BUG: Import counter of an imported buffer is 0, expect errors"); + } + auto &data = *(it->second.get()); + + data.ref_count++; + if (!should_map_dmabuf(handle)) return 0; + return map_locked(handle); + } + + 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; + } + + return map_locked(handle); + } + + 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 = (private_handle_t *)handle; + auto it = buffer_map.find(hnd); + if (it == buffer_map.end()) { + MALI_GRALLOC_LOGE("Trying to release a non-imported buffer"); + return -EINVAL; + } + + auto &data = *(it->second.get()); + if (data.ref_count == 0) { + MALI_GRALLOC_LOGE("BUG: Reference held for buffer whose counter is 0"); + return -EINVAL; + } + + data.ref_count--; + if (data.ref_count == 0) { + if (data.bases[0] != nullptr) { + mali_gralloc_ion_unmap(hnd); + } + + /* 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; + } + buffer_map.erase(it); + } + + return 0; + } + + int validate(buffer_handle_t handle) EXCLUDES(lock) { + std::lock_guard<std::mutex> _l(lock); + return validate_locked(handle); + } +}; + +int mali_gralloc_reference_retain(buffer_handle_t handle) { + return BufferManager::getInstance().retain(handle); } int mali_gralloc_reference_map(buffer_handle_t handle) { - private_handle_t *hnd = (private_handle_t *)handle; - - pthread_mutex_lock(&s_map_lock); - - if (hnd->bases[0]) { - MALI_GRALLOC_LOGV("Buffer is already mapped"); - pthread_mutex_unlock(&s_map_lock); - return 0; - } - - int retval = mali_gralloc_ion_map(hnd); - - pthread_mutex_unlock(&s_map_lock); - - return retval; + return BufferManager::getInstance().map(handle); } -int mali_gralloc_reference_release(buffer_handle_t handle, bool canFree) -{ - if (private_handle_t::validate(handle) < 0) - { - MALI_GRALLOC_LOGE("unregistering/releasing invalid buffer %p, returning error", handle); - return -EINVAL; - } - - private_handle_t *hnd = (private_handle_t *)handle; - pthread_mutex_lock(&s_map_lock); - - if (hnd->ref_count == 0) - { - MALI_GRALLOC_LOGE("Buffer %p should have already been released", handle); - pthread_mutex_unlock(&s_map_lock); - return -EINVAL; - } - - if (hnd->allocating_pid == getpid()) - { - hnd->ref_count--; - - if (hnd->ref_count == 0 && canFree) - { - mali_gralloc_dump_buffer_erase(hnd); - mali_gralloc_buffer_free(handle); - } - } - else if (hnd->remote_pid == getpid()) // never unmap buffers that were not imported into this process - { - hnd->ref_count--; - - if (hnd->ref_count == 0) - { - mali_gralloc_ion_unmap(hnd); - - /* 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; - } - } - } - else - { - MALI_GRALLOC_LOGE("Trying to unregister buffer %p from process %d that was not imported into current process: %d", hnd, - hnd->remote_pid, getpid()); - } - - pthread_mutex_unlock(&s_map_lock); - return 0; +int mali_gralloc_reference_release(buffer_handle_t handle) { + return BufferManager::getInstance().release(handle); } -int mali_gralloc_reference_validate(buffer_handle_t handle) -{ - if (private_handle_t::validate(handle) < 0) - { - MALI_GRALLOC_LOGE("Reference invalid buffer %p, returning error", handle); - return -EINVAL; - } - - const auto *hnd = (private_handle_t *)handle; - pthread_mutex_lock(&s_map_lock); - - if (hnd->allocating_pid == getpid() || hnd->remote_pid == getpid()) { - pthread_mutex_unlock(&s_map_lock); - return 0; - } else { - pthread_mutex_unlock(&s_map_lock); - MALI_GRALLOC_LOGE("Reference unimported buffer %p, returning error", handle); - return -EINVAL; - } +int mali_gralloc_reference_validate(buffer_handle_t handle) { + return BufferManager::getInstance().validate(handle); } - diff --git a/gralloc4/src/core/mali_gralloc_reference.h b/gralloc4/src/core/mali_gralloc_reference.h index 85bc1c9..acf8e82 100644 --- a/gralloc4/src/core/mali_gralloc_reference.h +++ b/gralloc4/src/core/mali_gralloc_reference.h @@ -22,7 +22,7 @@ #include "gralloc_priv.h" int mali_gralloc_reference_retain(buffer_handle_t handle); -int mali_gralloc_reference_release(buffer_handle_t handle, bool canFree); +int mali_gralloc_reference_release(buffer_handle_t handle); int mali_gralloc_reference_validate(buffer_handle_t handle); int mali_gralloc_reference_map(buffer_handle_t handle); diff --git a/gralloc4/src/hidl_common/Mapper.cpp b/gralloc4/src/hidl_common/Mapper.cpp index 80ea7e1..e8f5b10 100644 --- a/gralloc4/src/hidl_common/Mapper.cpp +++ b/gralloc4/src/hidl_common/Mapper.cpp @@ -90,7 +90,7 @@ static Error unregisterBuffer(buffer_handle_t bufferHandle) return Error::BAD_BUFFER; } - const int status = mali_gralloc_reference_release(bufferHandle, true); + const int status = mali_gralloc_reference_release(bufferHandle); if (status != 0) { MALI_GRALLOC_LOGE("Unable to release buffer:%p", bufferHandle); diff --git a/gralloc4/src/mali_gralloc_buffer.h b/gralloc4/src/mali_gralloc_buffer.h index a163d1a..02172c6 100644 --- a/gralloc4/src/mali_gralloc_buffer.h +++ b/gralloc4/src/mali_gralloc_buffer.h @@ -50,6 +50,21 @@ */ #define MAX_PLANES 3 +/* + * Maximum number of fds in a private_handle_t. + */ +#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 #else @@ -155,7 +170,7 @@ struct private_handle_t * DO NOT MOVE THIS ELEMENT! */ union { - int fds[5]; + int fds[MAX_FDS]; }; // ints @@ -215,14 +230,11 @@ struct private_handle_t uint64_t backing_store_id DEFAULT_INITIALIZER(0x0); int cpu_read DEFAULT_INITIALIZER(0); /**< Buffer is locked for CPU read when non-zero. */ int cpu_write DEFAULT_INITIALIZER(0); /**< Buffer is locked for CPU write when non-zero. */ - int allocating_pid DEFAULT_INITIALIZER(0); - int remote_pid DEFAULT_INITIALIZER(-1); - int ref_count DEFAULT_INITIALIZER(0); // 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); @@ -247,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]) @@ -265,8 +277,6 @@ struct private_handle_t stride = _stride; alloc_format = _alloc_format; layer_count = _layer_count; - allocating_pid = getpid(); - ref_count = 1; version = sizeof(native_handle); set_numfds(fd_count); memcpy(plane_info, _plane_info, sizeof(plane_info_t) * 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, diff --git a/libvendorgraphicbuffer/gralloc4/vendor_graphicbuffer_meta.cpp b/libvendorgraphicbuffer/gralloc4/vendor_graphicbuffer_meta.cpp index 9034019..cc0a20d 100644 --- a/libvendorgraphicbuffer/gralloc4/vendor_graphicbuffer_meta.cpp +++ b/libvendorgraphicbuffer/gralloc4/vendor_graphicbuffer_meta.cpp @@ -45,12 +45,7 @@ using android::hardware::graphics::mapper::V4_0::Error; // Gralloc. int mali_gralloc_reference_validate(buffer_handle_t handle) { auto hnd = static_cast<const private_handle_t *>(handle); - - if (hnd->allocating_pid != getpid() && hnd->remote_pid != getpid()) { - return -EINVAL; - } - - return 0; + return private_handle_t::validate(hnd); } const private_handle_t * convertNativeHandleToPrivateHandle(buffer_handle_t handle) { |