From ecec0db8084c11ed2eb5c43cf9e0d931ef039166 Mon Sep 17 00:00:00 2001 From: Jesse Hall Date: Thu, 28 Mar 2013 11:04:16 -0700 Subject: Remove pid check in register/unregister The register/unregister gralloc calls were avoiding mmapping/munmapping the shared memory region if the buffer was created by the current process. This is left over from the pmem-based implementation, where trying to map the same region twice in the same process would fail, or would reuse a single mapping without refcounting. This causes problems if a buffer is - allocated in process A, - transferred from A to process B and registered there - unregistered/freed in A - transferred back from B to A and re-registered Process A then has a new handle to the buffer, but since it originally created the buffer it will not be mmapped, so trying to read or write the buffer will crash. With ashmem, mmaping a region twice in the same process creates two distinct mappings which can be used and munmapped independently. So we no longer need to avoid mmapping again in the allocating process. Bug: 8468756 Change-Id: I167bec5ca07e5534c5e2115630fe8386e481388e --- modules/gralloc/gralloc_priv.h | 8 +++----- modules/gralloc/mapper.cpp | 23 +++++++---------------- 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/modules/gralloc/gralloc_priv.h b/modules/gralloc/gralloc_priv.h index e1c502a6..9d14fe0e 100644 --- a/modules/gralloc/gralloc_priv.h +++ b/modules/gralloc/gralloc_priv.h @@ -74,18 +74,16 @@ struct private_handle_t { int size; int offset; - // FIXME: the attributes below should be out-of-line + // FIXME: this should be out-of-line int base; - int pid; #ifdef __cplusplus - static const int sNumInts = 6; + static const int sNumInts = 5; static const int sNumFds = 1; static const int sMagic = 0x3141592; private_handle_t(int fd, int size, int flags) : - fd(fd), magic(sMagic), flags(flags), size(size), offset(0), - base(0), pid(getpid()) + fd(fd), magic(sMagic), flags(flags), size(size), offset(0), base(0) { version = sizeof(native_handle); numInts = sNumInts; diff --git a/modules/gralloc/mapper.cpp b/modules/gralloc/mapper.cpp index c4096aeb..8aadb4a2 100644 --- a/modules/gralloc/mapper.cpp +++ b/modules/gralloc/mapper.cpp @@ -82,7 +82,7 @@ static int gralloc_unmap(gralloc_module_t const* module, /*****************************************************************************/ -static pthread_mutex_t sMapLock = PTHREAD_MUTEX_INITIALIZER; +static pthread_mutex_t sMapLock = PTHREAD_MUTEX_INITIALIZER; /*****************************************************************************/ @@ -92,14 +92,8 @@ int gralloc_register_buffer(gralloc_module_t const* module, if (private_handle_t::validate(handle) < 0) return -EINVAL; - // if this handle was created in this process, then we keep it as is. - int err = 0; - private_handle_t* hnd = (private_handle_t*)handle; - if (hnd->pid != getpid()) { - void *vaddr; - err = gralloc_map(module, handle, &vaddr); - } - return err; + void *vaddr; + return gralloc_map(module, handle, &vaddr); } int gralloc_unregister_buffer(gralloc_module_t const* module, @@ -108,13 +102,10 @@ int gralloc_unregister_buffer(gralloc_module_t const* module, if (private_handle_t::validate(handle) < 0) return -EINVAL; - // never unmap buffers that were created in this process private_handle_t* hnd = (private_handle_t*)handle; - if (hnd->pid != getpid()) { - if (hnd->base) { - gralloc_unmap(module, handle); - } - } + if (hnd->base) + gralloc_unmap(module, handle); + return 0; } @@ -157,7 +148,7 @@ int gralloc_lock(gralloc_module_t const* module, return 0; } -int gralloc_unlock(gralloc_module_t const* module, +int gralloc_unlock(gralloc_module_t const* module, buffer_handle_t handle) { // we're done with a software buffer. nothing to do in this -- cgit v1.2.3