SharedMemory API changes

Hides getFd & getFileDescriptor due to lifecycle concenrs. Adds ASharedMemory_dupFromJava to allow sharing a shared memory region between Java & Native as safe as possible. Mis-use results in an FD leak instead of double-close. Bug: 64394076 Test: SharedMemory CTS tests Change-Id: I01a5eb978fc4e99559a79baac75754c32f13bdc4
lin15-microG · Aug 7, 2017 · e4f60cc · e4f60cc
1 parent cbf1657
commit e4f60cc
Show file tree

Hide file tree

Showing 7 changed files with 52 additions and 10 deletions.
diff --git a/api/current.txt b/api/current.txt
@@ -31066,7 +31066,6 @@ package android.os {
     ctor public MemoryFile(java.lang.String, int) throws java.io.IOException;
     method public deprecated synchronized boolean allowPurging(boolean) throws java.io.IOException;
     method public void close();
-    method public java.io.FileDescriptor getFileDescriptor() throws java.io.IOException;
     method public java.io.InputStream getInputStream();
     method public java.io.OutputStream getOutputStream();
     method public deprecated boolean isPurgingAllowed();
@@ -31498,8 +31497,6 @@ package android.os {
     method public void close();
     method public static android.os.SharedMemory create(java.lang.String, int) throws android.system.ErrnoException;
     method public int describeContents();
-    method public int getFd();
-    method public java.io.FileDescriptor getFileDescriptor();
     method public int getSize();
     method public java.nio.ByteBuffer map(int, int, int) throws android.system.ErrnoException;
     method public java.nio.ByteBuffer mapReadOnly() throws android.system.ErrnoException;

diff --git a/api/system-current.txt b/api/system-current.txt
@@ -33911,7 +33911,6 @@ package android.os {
     ctor public MemoryFile(java.lang.String, int) throws java.io.IOException;
     method public deprecated synchronized boolean allowPurging(boolean) throws java.io.IOException;
     method public void close();
-    method public java.io.FileDescriptor getFileDescriptor() throws java.io.IOException;
     method public java.io.InputStream getInputStream();
     method public java.io.OutputStream getOutputStream();
     method public deprecated boolean isPurgingAllowed();
@@ -34372,8 +34371,6 @@ package android.os {
     method public void close();
     method public static android.os.SharedMemory create(java.lang.String, int) throws android.system.ErrnoException;
     method public int describeContents();
-    method public int getFd();
-    method public java.io.FileDescriptor getFileDescriptor();
     method public int getSize();
     method public java.nio.ByteBuffer map(int, int, int) throws android.system.ErrnoException;
     method public java.nio.ByteBuffer mapReadOnly() throws android.system.ErrnoException;

diff --git a/api/test-current.txt b/api/test-current.txt
@@ -31223,7 +31223,6 @@ package android.os {
     ctor public MemoryFile(java.lang.String, int) throws java.io.IOException;
     method public deprecated synchronized boolean allowPurging(boolean) throws java.io.IOException;
     method public void close();
-    method public java.io.FileDescriptor getFileDescriptor() throws java.io.IOException;
     method public java.io.InputStream getInputStream();
     method public java.io.OutputStream getOutputStream();
     method public deprecated boolean isPurgingAllowed();
@@ -31656,8 +31655,6 @@ package android.os {
     method public void close();
     method public static android.os.SharedMemory create(java.lang.String, int) throws android.system.ErrnoException;
     method public int describeContents();
-    method public int getFd();
-    method public java.io.FileDescriptor getFileDescriptor();
     method public int getSize();
     method public java.nio.ByteBuffer map(int, int, int) throws android.system.ErrnoException;
     method public java.nio.ByteBuffer mapReadOnly() throws android.system.ErrnoException;

diff --git a/core/java/android/os/MemoryFile.java b/core/java/android/os/MemoryFile.java
@@ -219,6 +219,8 @@ public void writeBytes(byte[] buffer, int srcOffset, int destOffset, int count)
      * The returned file descriptor is not duplicated.
      *
      * @throws IOException If the memory file has been closed.
+     *
+     * @hide
      */
     public FileDescriptor getFileDescriptor() throws IOException {
         return mSharedMemory.getFileDescriptor();

diff --git a/core/java/android/os/SharedMemory.java b/core/java/android/os/SharedMemory.java
@@ -60,7 +60,8 @@ private SharedMemory(FileDescriptor fd) {
         }
 
         mMemoryRegistration = new MemoryRegistration(mSize);
-        mCleaner = Cleaner.create(this, new Closer(mFileDescriptor, mMemoryRegistration));
+        mCleaner = Cleaner.create(mFileDescriptor,
+                new Closer(mFileDescriptor, mMemoryRegistration));
     }
 
     /**
@@ -138,6 +139,8 @@ public boolean setProtect(int prot) {
      * This FileDescriptor is interoperable with the ASharedMemory NDK APIs.
      *
      * @return Returns the FileDescriptor associated with this object.
+     *
+     * @hide Exists only for MemoryFile interop
      */
     public @NonNull FileDescriptor getFileDescriptor() {
         return mFileDescriptor;
@@ -150,6 +153,8 @@ public boolean setProtect(int prot) {
      * This fd is interoperable with the ASharedMemory NDK APIs.
      *
      * @return Returns the native fd associated with this object, or -1 if it is already closed.
+     *
+     * @hide Exposed for native ASharedMemory_dupFromJava()
      */
     public int getFd() {
         return mFileDescriptor.getInt$();

diff --git a/native/android/libandroid.map.txt b/native/android/libandroid.map.txt
@@ -198,6 +198,7 @@ LIBANDROID {
     ASharedMemory_create; # introduced=26
     ASharedMemory_getSize; # introduced=26
     ASharedMemory_setProt; # introduced=26
+    ASharedMemory_dupFromJava; # introduced=27
     AStorageManager_delete;
     AStorageManager_getMountedObbPath;
     AStorageManager_isObbMounted;

diff --git a/native/android/sharedmem.cpp b/native/android/sharedmem.cpp
@@ -14,10 +14,36 @@
  * limitations under the License.
  */
 
+#include <jni.h>
+
 #include <android/sharedmem.h>
+#include <android/sharedmem_jni.h>
 #include <cutils/ashmem.h>
+#include <log/log.h>
 #include <utils/Errors.h>
 
+#include <mutex>
+#include <unistd.h>
+
+static struct {
+    jclass clazz;
+    jmethodID getFd;
+} sSharedMemory;
+
+static void jniInit(JNIEnv* env) {
+    static std::once_flag sJniInitialized;
+    std::call_once(sJniInitialized, [](JNIEnv* env) {
+        jclass clazz = env->FindClass("android/os/SharedMemory");
+        LOG_ALWAYS_FATAL_IF(clazz == nullptr, "Failed to find android.os.SharedMemory");
+        sSharedMemory.clazz = (jclass) env->NewGlobalRef(clazz);
+        LOG_ALWAYS_FATAL_IF(sSharedMemory.clazz == nullptr,
+                "Failed to create global ref of android.os.SharedMemory");
+        sSharedMemory.getFd = env->GetMethodID(sSharedMemory.clazz, "getFd", "()I");
+        LOG_ALWAYS_FATAL_IF(sSharedMemory.getFd == nullptr,
+                "Failed to find method SharedMemory#getFd()");
+    }, env);
+}
+
 int ASharedMemory_create(const char *name, size_t size) {
     if (size == 0) {
         return android::BAD_VALUE;
@@ -32,3 +58,20 @@ size_t ASharedMemory_getSize(int fd) {
 int ASharedMemory_setProt(int fd, int prot) {
     return ashmem_set_prot_region(fd, prot);
 }
+
+int ASharedMemory_dupFromJava(JNIEnv* env, jobject javaSharedMemory) {
+    if (env == nullptr || javaSharedMemory == nullptr) {
+        return -1;
+    }
+    jniInit(env);
+    if (!env->IsInstanceOf(javaSharedMemory, sSharedMemory.clazz)) {
+        ALOGW("ASharedMemory_dupFromJava called with object "
+                "that's not an instanceof android.os.SharedMemory");
+        return -1;
+    }
+    int fd = env->CallIntMethod(javaSharedMemory, sSharedMemory.getFd);
+    if (fd != -1) {
+        fd = dup(fd);
+    }
+    return fd;
+}