commit ecec0db8084c11ed2eb5c43cf9e0d931ef039166
author Jesse Hall <jessehall@google.com> Thu Mar 28 11:04:16 2013 -0700
committer The Android Automerger <android-build@android.com> Thu Mar 28 15:48:36 2013 -0700
tree 74ab0b13394355d51b15012434ba8e5b5a83ee4d
parent fb40d5de36fc93a1a3cab9c2cd73988ac52043a2

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

diff --git a/modules/gralloc/gralloc_priv.h b/modules/gralloc/gralloc_priv.h
index e1c502a..9d14fe0 100644
--- a/modules/gralloc/gralloc_priv.h
+++ b/modules/gralloc/gralloc_priv.h
@@ -74,18 +74,16 @@
     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;

modules/gralloc/mapper.cpp

diff --git a/modules/gralloc/mapper.cpp b/modules/gralloc/mapper.cpp
index c4096ae..8aadb4a 100644
--- a/modules/gralloc/mapper.cpp
+++ b/modules/gralloc/mapper.cpp
@@ -82,7 +82,7 @@
 
 /*****************************************************************************/
 
-static pthread_mutex_t sMapLock = PTHREAD_MUTEX_INITIALIZER; 
+static pthread_mutex_t sMapLock = PTHREAD_MUTEX_INITIALIZER;
 
 /*****************************************************************************/
 
@@ -92,14 +92,8 @@
     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 @@
     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 @@
     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