Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 6c6a9c32 authored by Josh Gao's avatar Josh Gao
Browse files

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
parent a77be890

core/java/android/os/SharedMemory.java

+9 −8
Original line number Original line Diff line number Diff line
@@ -25,6 +25,8 @@ import android.system.OsConstants; 




import dalvik.system.VMRuntime;

import dalvik.system.VMRuntime;





import libcore.io.IoUtils;



import java.io.Closeable;

import java.io.Closeable;

import java.io.FileDescriptor;

import java.io.FileDescriptor;

import java.io.IOException;

import java.io.IOException;
@@ -63,7 +65,7 @@ public final class SharedMemory implements Parcelable, Closeable { 




        mMemoryRegistration = new MemoryRegistration(mSize);

        mMemoryRegistration = new MemoryRegistration(mSize);

        mCleaner = Cleaner.create(mFileDescriptor,

        mCleaner = Cleaner.create(mFileDescriptor,

                new Closer(mFileDescriptor.getInt$(), mMemoryRegistration));

                new Closer(mFileDescriptor, mMemoryRegistration));

    }

    }





    /**

    /**
@@ -326,21 +328,20 @@ public final class SharedMemory implements Parcelable, Closeable { 
     * Cleaner that closes the FD

     * Cleaner that closes the FD

     */

     */

    private static final class Closer implements Runnable {

    private static final class Closer implements Runnable {

        private int mFd;

        private FileDescriptor mFd;

        private MemoryRegistration mMemoryReference;

        private MemoryRegistration mMemoryReference;





        private Closer(int fd, MemoryRegistration memoryReference) {

        private Closer(FileDescriptor fd, MemoryRegistration memoryReference) {

            mFd = fd;

            mFd = fd;

            IoUtils.setFdOwner(mFd, this);

            mMemoryReference = memoryReference;

            mMemoryReference = memoryReference;

        }

        }





        @Override

        @Override

        public void run() {

        public void run() {

            try {

            IoUtils.closeQuietly(mFd);

                FileDescriptor fd = new FileDescriptor();

            mFd = null;

                fd.setInt$(mFd);



                Os.close(fd);

            } catch (ErrnoException e) { /* swallow error */ }

            mMemoryReference.release();

            mMemoryReference.release();

            mMemoryReference = null;

            mMemoryReference = null;

        }

        }

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

+1 −1
Original line number Original line Diff line number Diff line
@@ -87,7 +87,7 @@ public class ContentResolverTest { 
        bitmap.compress(Bitmap.CompressFormat.PNG, 90, mImage.getOutputStream());

        bitmap.compress(Bitmap.CompressFormat.PNG, 90, mImage.getOutputStream());





        final AssetFileDescriptor afd = new AssetFileDescriptor(

        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(

        when(mProvider.openTypedAssetFile(any(), any(), any(), any(), any())).thenReturn(

                afd);

                afd);

    }

    }