commit 6c6a9c322d9ba3caa4a61ce906aa5cdb62756f01
author Josh Gao <jmgao@fb.com> Thu Feb 15 12:25:29 2024 -0800
committer Josh Gao <jmgao@fb.com> Mon Mar 04 19:05:30 2024 -0800
tree caedab292b39b8e6e92ce6adce5fbdbaac2a785e
parent a77be890e217905ce354ed7e4b08499ff496cc0b

SharedMemory: mark fdsan ownership

We noticed some internal test failures that were due to fdsan aborts
from SharedMemory.Closer getting its fd stomped on and then aborting
when it tried to close an fd owned by a unique_fd that got allocated in
its spot.

This immediately found a bug in our code where we were creating a
ParcelFileDescriptor from SharedMemory.getFileDescriptor, along with a
bug in ContentResolverTest.

Test: `atest android.os.cts.SharedMemoryTest android.content.ContentResolverTest`
Change-Id: I3fd4a5c5e478e0674021ffed3d624046e978dfdc

core/java/android/os/SharedMemory.java

diff --git a/core/java/android/os/SharedMemory.java b/core/java/android/os/SharedMemory.java
index cba4423..d008034 100644
--- a/core/java/android/os/SharedMemory.java
+++ b/core/java/android/os/SharedMemory.java
@@ -25,6 +25,8 @@
 
 import dalvik.system.VMRuntime;
 
+import libcore.io.IoUtils;
+
 import java.io.Closeable;
 import java.io.FileDescriptor;
 import java.io.IOException;
@@ -63,7 +65,7 @@
 
         mMemoryRegistration = new MemoryRegistration(mSize);
         mCleaner = Cleaner.create(mFileDescriptor,
-                new Closer(mFileDescriptor.getInt$(), mMemoryRegistration));
+                new Closer(mFileDescriptor, mMemoryRegistration));
     }
 
     /**
@@ -326,21 +328,20 @@
      * Cleaner that closes the FD
      */
     private static final class Closer implements Runnable {
-        private int mFd;
+        private FileDescriptor mFd;
         private MemoryRegistration mMemoryReference;
 
-        private Closer(int fd, MemoryRegistration memoryReference) {
+        private Closer(FileDescriptor fd, MemoryRegistration memoryReference) {
             mFd = fd;
+            IoUtils.setFdOwner(mFd, this);
             mMemoryReference = memoryReference;
         }
 
         @Override
         public void run() {
-            try {
-                FileDescriptor fd = new FileDescriptor();
-                fd.setInt$(mFd);
-                Os.close(fd);
-            } catch (ErrnoException e) { /* swallow error */ }
+            IoUtils.closeQuietly(mFd);
+            mFd = null;
+
             mMemoryReference.release();
             mMemoryReference = null;
         }

core/tests/coretests/src/android/content/ContentResolverTest.java

diff --git a/core/tests/coretests/src/android/content/ContentResolverTest.java b/core/tests/coretests/src/android/content/ContentResolverTest.java
index 7b70b41..c8015d4 100644
--- a/core/tests/coretests/src/android/content/ContentResolverTest.java
+++ b/core/tests/coretests/src/android/content/ContentResolverTest.java
@@ -87,7 +87,7 @@
         bitmap.compress(Bitmap.CompressFormat.PNG, 90, mImage.getOutputStream());
 
         final AssetFileDescriptor afd = new AssetFileDescriptor(
-                new ParcelFileDescriptor(mImage.getFileDescriptor()), 0, mSize, null);
+                ParcelFileDescriptor.dup(mImage.getFileDescriptor()), 0, mSize, null);
         when(mProvider.openTypedAssetFile(any(), any(), any(), any(), any())).thenReturn(
                 afd);
     }