Merge "Add support for bionic fd ownership enforcement."
diff --git a/luni/src/main/java/libcore/io/ForwardingOs.java b/luni/src/main/java/libcore/io/ForwardingOs.java
index f141694..48c4e1c 100644
--- a/luni/src/main/java/libcore/io/ForwardingOs.java
+++ b/luni/src/main/java/libcore/io/ForwardingOs.java
@@ -70,6 +70,8 @@
public void chmod(String path, int mode) throws ErrnoException { os.chmod(path, mode); }
public void chown(String path, int uid, int gid) throws ErrnoException { os.chown(path, uid, gid); }
public void close(FileDescriptor fd) throws ErrnoException { os.close(fd); }
+ public void android_fdsan_exchange_owner_tag(FileDescriptor fd, long previousOwnerId, long newOwnerId) { os.android_fdsan_exchange_owner_tag(fd, previousOwnerId, newOwnerId); }
+
public void connect(FileDescriptor fd, InetAddress address, int port) throws ErrnoException, SocketException { os.connect(fd, address, port); }
public void connect(FileDescriptor fd, SocketAddress address) throws ErrnoException, SocketException { os.connect(fd, address); }
public FileDescriptor dup(FileDescriptor oldFd) throws ErrnoException { return os.dup(oldFd); }
diff --git a/luni/src/main/java/libcore/io/IoBridge.java b/luni/src/main/java/libcore/io/IoBridge.java
index fcd8b9a..338400b 100644
--- a/luni/src/main/java/libcore/io/IoBridge.java
+++ b/luni/src/main/java/libcore/io/IoBridge.java
@@ -229,9 +229,10 @@
}
/**
- * Closes the supplied file descriptor and sends a signal to any threads are currently blocking.
- * In order for the signal to be sent the blocked threads must have registered with
- * the AsynchronousCloseMonitor before they entered the blocking operation.
+ * Closes the Unix file descriptor associated with the supplied file descriptor and sends a
+ * signal to any threads are currently blocking. In order for the signal to be sent the blocked
+ * threads must have registered with the AsynchronousCloseMonitor before they entered the
+ * blocking operation. {@code fd} will be invalid after this call.
*
* <p>This method is a no-op if passed a {@code null} or already-closed file descriptor.
*/
@@ -239,15 +240,13 @@
if (fd == null || !fd.valid()) {
return;
}
- int intFd = fd.getInt$();
- fd.setInt$(-1);
- FileDescriptor oldFd = new FileDescriptor();
- oldFd.setInt$(intFd);
+ // fd is invalid after we call release.
+ FileDescriptor oldFd = fd.release$();
AsynchronousCloseMonitor.signalBlockedThreads(oldFd);
try {
Libcore.os.close(oldFd);
} catch (ErrnoException errnoException) {
- // TODO: are there any cases in which we should throw?
+ throw errnoException.rethrowAsIOException();
}
}
diff --git a/luni/src/main/java/libcore/io/IoUtils.java b/luni/src/main/java/libcore/io/IoUtils.java
index b01759d..43b800c 100644
--- a/luni/src/main/java/libcore/io/IoUtils.java
+++ b/luni/src/main/java/libcore/io/IoUtils.java
@@ -26,7 +26,9 @@
import java.net.Socket;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
+import java.util.Objects;
import java.util.Random;
+import libcore.util.NonNull;
import static android.system.OsConstants.*;
public final class IoUtils {
@@ -34,17 +36,74 @@
}
/**
+ * Acquires ownership of an integer file descriptor from a FileDescriptor.
+ *
+ * This method invalidates the FileDescriptor passed in.
+ *
+ * The important part of this function is that you are taking ownership of a resource that you
+ * must either clean up yourself, or hand off to some other object that does that for you.
+ *
+ * See bionic/include/android/fdsan.h for more details.
+ *
+ * @param fd FileDescriptor to take ownership from, must be non-null.
+ * @throws NullPointerException if fd is null
+ */
+ public static int acquireRawFd(@NonNull FileDescriptor fd) {
+ Objects.requireNonNull(fd);
+
+ FileDescriptor copy = fd.release$();
+ // Get the numeric Unix file descriptor. -1 means it is invalid; for example if
+ // {@link FileDescriptor#release$()} has already been called on the FileDescriptor.
+ int rawFd = copy.getInt$();
+ long previousOwnerId = copy.getOwnerId$();
+ if (rawFd != -1 && previousOwnerId != FileDescriptor.NO_OWNER) {
+ // Clear the file descriptor's owner ID, aborting if the previous value isn't as expected.
+ Libcore.os.android_fdsan_exchange_owner_tag(copy, previousOwnerId,
+ FileDescriptor.NO_OWNER);
+ }
+ return rawFd;
+ }
+
+ /**
+ * Assigns ownership of an unowned FileDescriptor.
+ *
+ * Associates the supplied FileDescriptor and the underlying Unix file descriptor with an owner
+ * ID derived from the supplied {@code owner} object. If the FileDescriptor already has an
+ * associated owner an {@link IllegalStateException} will be thrown. If the underlying Unix
+ * file descriptor already has an associated owner, the process will abort.
+ *
+ * See bionic/include/android/fdsan.h for more details.
+ *
+ * @param fd FileDescriptor to take ownership from, must be non-null.
+ * @throws NullPointerException if fd or owner are null
+ * @throws IllegalStateException if fd is already owned
+ */
+ public static void setFdOwner(@NonNull FileDescriptor fd, @NonNull Object owner) {
+ Objects.requireNonNull(fd);
+ Objects.requireNonNull(owner);
+
+ long previousOwnerId = fd.getOwnerId$();
+ if (previousOwnerId != FileDescriptor.NO_OWNER) {
+ throw new IllegalStateException("Attempted to take ownership of already-owned " +
+ "FileDescriptor");
+ }
+
+ // ownerId is not required to be unique but should be stable and should attempt to avoid
+ // collision with identifiers generated both here and in native code (which are simply the
+ // address of the owning object). identityHashCode(Object) meets these requirements.
+ int ownerId = System.identityHashCode(owner);
+ fd.setOwnerId$(ownerId);
+
+ // Set the file descriptor's owner ID, aborting if the previous value isn't as expected.
+ Libcore.os.android_fdsan_exchange_owner_tag(fd, previousOwnerId, ownerId);
+ }
+
+ /**
* Calls close(2) on 'fd'. Also resets the internal int to -1. Does nothing if 'fd' is null
* or invalid.
*/
public static void close(FileDescriptor fd) throws IOException {
- try {
- if (fd != null && fd.valid()) {
- Libcore.os.close(fd);
- }
- } catch (ErrnoException errnoException) {
- throw errnoException.rethrowAsIOException();
- }
+ IoBridge.closeAndSignalBlockedThreads(fd);
}
/**
diff --git a/luni/src/main/java/libcore/io/Linux.java b/luni/src/main/java/libcore/io/Linux.java
index 807e5a2..bdbdcf7 100644
--- a/luni/src/main/java/libcore/io/Linux.java
+++ b/luni/src/main/java/libcore/io/Linux.java
@@ -60,7 +60,10 @@
throws ErrnoException;
public native void chmod(String path, int mode) throws ErrnoException;
public native void chown(String path, int uid, int gid) throws ErrnoException;
+
public native void close(FileDescriptor fd) throws ErrnoException;
+ public native void android_fdsan_exchange_owner_tag(FileDescriptor fd, long previousOwnerId, long newOwnerId);
+
public native void connect(FileDescriptor fd, InetAddress address, int port) throws ErrnoException, SocketException;
public native void connect(FileDescriptor fd, SocketAddress address) throws ErrnoException, SocketException;
public native FileDescriptor dup(FileDescriptor oldFd) throws ErrnoException;
diff --git a/luni/src/main/java/libcore/io/Os.java b/luni/src/main/java/libcore/io/Os.java
index 61584b4..f791915 100644
--- a/luni/src/main/java/libcore/io/Os.java
+++ b/luni/src/main/java/libcore/io/Os.java
@@ -54,7 +54,10 @@
public void capset(StructCapUserHeader hdr, StructCapUserData[] data) throws ErrnoException;
public void chmod(String path, int mode) throws ErrnoException;
public void chown(String path, int uid, int gid) throws ErrnoException;
+
public void close(FileDescriptor fd) throws ErrnoException;
+ public void android_fdsan_exchange_owner_tag(FileDescriptor fd, long previousOwnerId, long newOwnerId);
+
public void connect(FileDescriptor fd, InetAddress address, int port) throws ErrnoException, SocketException;
public void connect(FileDescriptor fd, SocketAddress address) throws ErrnoException, SocketException;
public FileDescriptor dup(FileDescriptor oldFd) throws ErrnoException;
diff --git a/luni/src/main/native/libcore_io_Linux.cpp b/luni/src/main/native/libcore_io_Linux.cpp
index 30da35c..5a64218 100644
--- a/luni/src/main/native/libcore_io_Linux.cpp
+++ b/luni/src/main/native/libcore_io_Linux.cpp
@@ -49,8 +49,13 @@
#include <memory>
+#if defined(__BIONIC__)
+#include <android/fdsan.h>
+#endif
+
#include <android-base/file.h>
#include <android-base/logging.h>
+#include <android-base/macros.h>
#include <android-base/strings.h>
#include <log/log.h>
#include <nativehelper/AsynchronousCloseMonitor.h>
@@ -1089,10 +1094,29 @@
int fd = jniGetFDFromFileDescriptor(env, javaFd);
jniSetFileDescriptorOfFD(env, javaFd, -1);
+#if defined(__BIONIC__)
+ jlong ownerId = jniGetOwnerIdFromFileDescriptor(env, javaFd);
+
+ // Close with bionic's fd ownership tracking (which returns 0 in the case of EINTR).
+ throwIfMinusOne(env, "close", android_fdsan_close_with_tag(fd, ownerId));
+#else
// Even if close(2) fails with EINTR, the fd will have been closed.
// Using TEMP_FAILURE_RETRY will either lead to EBADF or closing someone else's fd.
// http://lkml.indiana.edu/hypermail/linux/kernel/0509.1/0877.html
throwIfMinusOne(env, "close", close(fd));
+#endif
+}
+
+static void Linux_android_fdsan_exchange_owner_tag(JNIEnv* env, jclass,
+ jobject javaFd,
+ jlong expectedOwnerId,
+ jlong newOwnerId) {
+#if defined(__BIONIC__)
+ int fd = jniGetFDFromFileDescriptor(env, javaFd);
+ android_fdsan_exchange_owner_tag(fd, expectedOwnerId, newOwnerId);
+#else
+ UNUSED(env, javaFd, expectedOwnerId, newOwnerId);
+#endif
}
static void Linux_connect(JNIEnv* env, jobject, jobject javaFd, jobject javaAddress, jint port) {
@@ -2467,6 +2491,7 @@
static JNINativeMethod gMethods[] = {
NATIVE_METHOD(Linux, accept, "(Ljava/io/FileDescriptor;Ljava/net/SocketAddress;)Ljava/io/FileDescriptor;"),
NATIVE_METHOD(Linux, access, "(Ljava/lang/String;I)Z"),
+ NATIVE_METHOD(Linux, android_fdsan_exchange_owner_tag, "(Ljava/io/FileDescriptor;JJ)V"),
NATIVE_METHOD(Linux, android_getaddrinfo, "(Ljava/lang/String;Landroid/system/StructAddrinfo;I)[Ljava/net/InetAddress;"),
NATIVE_METHOD(Linux, bind, "(Ljava/io/FileDescriptor;Ljava/net/InetAddress;I)V"),
NATIVE_METHOD_OVERLOAD(Linux, bind, "(Ljava/io/FileDescriptor;Ljava/net/SocketAddress;)V", SocketAddress),
diff --git a/ojluni/src/main/java/java/io/FileDescriptor.java b/ojluni/src/main/java/java/io/FileDescriptor.java
index 36d8133..a2c833d 100644
--- a/ojluni/src/main/java/java/io/FileDescriptor.java
+++ b/ojluni/src/main/java/java/io/FileDescriptor.java
@@ -54,6 +54,15 @@
// fetching the descriptor value.
private int descriptor;
+ // Android-added: Track fd owner to guard against accidental closure. http://b/110100358
+ // The owner on the libc side is an pointer-sized value that can be set to an arbitrary
+ // value (with 0 meaning 'unowned'). libcore chooses to use System.identityHashCode.
+ private long ownerId = NO_OWNER;
+
+ // Android-added: value of ownerId indicating that a FileDescriptor is unowned.
+ /** @hide */
+ public static final long NO_OWNER = 0L;
+
/**
* Constructs an (invalid) FileDescriptor
* object.
@@ -160,6 +169,43 @@
this.descriptor = fd;
}
+ // BEGIN Android-added: Methods to enable ownership enforcement of Unix file descriptors.
+ /**
+ * Returns the owner ID of this FileDescriptor. It's highly unlikely you should be calling this.
+ * Please discuss your needs with a libcore maintainer before using this method.
+ * @hide internal use only
+ */
+ public long getOwnerId$() {
+ return this.ownerId;
+ }
+
+ /**
+ * Sets the owner ID of this FileDescriptor. The owner ID does not need to be unique, but it is
+ * assumed that clashes are rare. See bionic/include/android/fdsan.h for more details.
+ *
+ * It's highly unlikely you should be calling this.
+ * Please discuss your needs with a libcore maintainer before using this method.
+ * @param owner the owner ID of the Object that is responsible for closing this FileDescriptor
+ * @hide internal use only
+ */
+ public void setOwnerId$(long newOwnerId) {
+ this.ownerId = newOwnerId;
+ }
+
+ /**
+ * Returns a copy of this FileDescriptor, and sets this to an invalid state.
+ * @hide internal use only
+ */
+ public FileDescriptor release$() {
+ FileDescriptor result = new FileDescriptor();
+ result.descriptor = this.descriptor;
+ result.ownerId = this.ownerId;
+ this.descriptor = -1;
+ this.ownerId = FileDescriptor.NO_OWNER;
+ return result;
+ }
+ // END Android-added: Methods to enable ownership enforcement of Unix file descriptors.
+
/**
* @hide internal use only
*/
diff --git a/ojluni/src/main/java/java/io/FileInputStream.java b/ojluni/src/main/java/java/io/FileInputStream.java
index 8df5f4d..5551b68 100755
--- a/ojluni/src/main/java/java/io/FileInputStream.java
+++ b/ojluni/src/main/java/java/io/FileInputStream.java
@@ -34,6 +34,7 @@
import sun.nio.ch.FileChannelImpl;
import libcore.io.IoBridge;
import libcore.io.IoTracker;
+import libcore.io.IoUtils;
/**
@@ -166,6 +167,9 @@
open(name);
+ // Android-added: File descriptor ownership tracking.
+ IoUtils.setFdOwner(this.fd, this);
+
// Android-added: CloseGuard support.
guard.open("close");
}
diff --git a/ojluni/src/main/java/java/io/FileOutputStream.java b/ojluni/src/main/java/java/io/FileOutputStream.java
index f29a741..9bd1bc6 100755
--- a/ojluni/src/main/java/java/io/FileOutputStream.java
+++ b/ojluni/src/main/java/java/io/FileOutputStream.java
@@ -34,6 +34,7 @@
import sun.nio.ch.FileChannelImpl;
import libcore.io.IoBridge;
import libcore.io.IoTracker;
+import libcore.io.IoUtils;
/**
* A file output stream is an output stream for writing data to a
@@ -237,6 +238,9 @@
open(name, append);
+ // Android-added: File descriptor ownership tracking.
+ IoUtils.setFdOwner(this.fd, this);
+
// Android-added: CloseGuard support.
guard.open("close");
}
diff --git a/ojluni/src/main/java/java/io/RandomAccessFile.java b/ojluni/src/main/java/java/io/RandomAccessFile.java
index a83829f..06683ad 100755
--- a/ojluni/src/main/java/java/io/RandomAccessFile.java
+++ b/ojluni/src/main/java/java/io/RandomAccessFile.java
@@ -34,6 +34,7 @@
import dalvik.system.CloseGuard;
import libcore.io.IoBridge;
import libcore.io.IoTracker;
+import libcore.io.IoUtils;
import libcore.io.Libcore;
import static android.system.OsConstants.*;
@@ -286,6 +287,7 @@
// BEGIN Android-changed: Use IoBridge.open() instead of open.
fd = IoBridge.open(name, imode);
+ IoUtils.setFdOwner(fd, this);
maybeSync();
guard.open("close");
// END Android-changed: Use IoBridge.open() instead of open.
