From 390d9e6a1806626eb521d55a36b1578d28714cc8 Mon Sep 17 00:00:00 2001
From: Josh Gao <jmgao@google.com>
Date: Wed, 24 Jul 2019 15:40:57 -0700
Subject: [PATCH 1/2] SharedMemory: break Cleaner reference cycle.

Previously, the Cleaner we create to close the ashmem file descriptor
used a thunk that held a strong reference to the FileDescriptor we
wanted to clean up, which prevented the Cleaner from ever running.

Break the cycle by storing the integer value of the file descriptor
instead.

Bug: http://b/138323667
Test: treehugger
Change-Id: I613a7d035892032f9567d59acb04672957c96011
(cherry picked from commit 6ca916a657cd56158212a57601108716ce78cbe8)
---
 core/java/android/os/SharedMemory.java | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/core/java/android/os/SharedMemory.java b/core/java/android/os/SharedMemory.java
index 57a88012a31a0..3e2ba3d3115f7 100644
--- a/core/java/android/os/SharedMemory.java
+++ b/core/java/android/os/SharedMemory.java
@@ -62,7 +62,7 @@ public final class SharedMemory implements Parcelable, Closeable {
 
         mMemoryRegistration = new MemoryRegistration(mSize);
         mCleaner = Cleaner.create(mFileDescriptor,
-                new Closer(mFileDescriptor, mMemoryRegistration));
+                new Closer(mFileDescriptor.getInt$(), mMemoryRegistration));
     }
 
     /**
@@ -290,10 +290,10 @@ public final class SharedMemory implements Parcelable, Closeable {
      * Cleaner that closes the FD
      */
     private static final class Closer implements Runnable {
-        private FileDescriptor mFd;
+        private int mFd;
         private MemoryRegistration mMemoryReference;
 
-        private Closer(FileDescriptor fd, MemoryRegistration memoryReference) {
+        private Closer(int fd, MemoryRegistration memoryReference) {
             mFd = fd;
             mMemoryReference = memoryReference;
         }
@@ -301,7 +301,9 @@ public final class SharedMemory implements Parcelable, Closeable {
         @Override
         public void run() {
             try {
-                Os.close(mFd);
+                FileDescriptor fd = new FileDescriptor();
+                fd.setInt$(mFd);
+                Os.close(fd);
             } catch (ErrnoException e) { /* swallow error */ }
             mMemoryReference.release();
             mMemoryReference = null;

From 20ab1e34273aa179053f5dc93e70c0191a39e91b Mon Sep 17 00:00:00 2001
From: Josh Gao <jmgao@google.com>
Date: Thu, 25 Jul 2019 13:54:23 -0700
Subject: [PATCH 2/2] SharedMemory: clear file descriptor when explicitly
 closed.

We run the Cleaner in close, but after the fix in commit 6ca916a6, this
no longer clears the value stored in the FileDescriptor, which means
that subsequent operations on an explicitly closed SharedMemory will
operate on a bogus fd number. Clearing the FileDescriptor value in close
is sufficient, because Cleaner.clean is idempotent, and the only other
case where it executes is when the FileDescriptor is phantom reachable,
which means no one can access it to get its integer value.

Bug: http://b/138392115
Bug: http://b/138323667
Test: treehugger
Change-Id: I8bdb4c745466532a0712976416184c53fcf0dbf6
(cherry picked from commit a7641806ddf1099239632d53c629c062ff2168f4)
---
 core/java/android/os/SharedMemory.java | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/core/java/android/os/SharedMemory.java b/core/java/android/os/SharedMemory.java
index 3e2ba3d3115f7..0540e3611b52c 100644
--- a/core/java/android/os/SharedMemory.java
+++ b/core/java/android/os/SharedMemory.java
@@ -259,6 +259,9 @@ public final class SharedMemory implements Parcelable, Closeable {
             mCleaner.clean();
             mCleaner = null;
         }
+
+        // Cleaner.clean doesn't clear the value of the file descriptor.
+        mFileDescriptor.setInt$(-1);
     }
 
     @Override