Merge "FileBridge: fix fd ownership mismanagement." am: e80c7cb69a am: bc2ef840e1 am: 5cc3eac681 am: 58c615fbaa am: ff31d87058
Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/1433025
Change-Id: I877dc8e34324a0f4b303d8bb3c8af450dcea341d
diff --git a/core/java/android/os/FileBridge.java b/core/java/android/os/FileBridge.java
index 21fd819..ab5637c 100644
--- a/core/java/android/os/FileBridge.java
+++ b/core/java/android/os/FileBridge.java
@@ -16,7 +16,6 @@
package android.os;
-import static android.system.OsConstants.AF_UNIX;
import static android.system.OsConstants.SOCK_STREAM;
import android.system.ErrnoException;
@@ -58,17 +57,19 @@
/** CMD_CLOSE */
private static final int CMD_CLOSE = 3;
- private FileDescriptor mTarget;
+ private ParcelFileDescriptor mTarget;
- private final FileDescriptor mServer = new FileDescriptor();
- private final FileDescriptor mClient = new FileDescriptor();
+ private ParcelFileDescriptor mServer;
+ private ParcelFileDescriptor mClient;
private volatile boolean mClosed;
public FileBridge() {
try {
- Os.socketpair(AF_UNIX, SOCK_STREAM, 0, mServer, mClient);
- } catch (ErrnoException e) {
+ ParcelFileDescriptor[] fds = ParcelFileDescriptor.createSocketPair(SOCK_STREAM);
+ mServer = fds[0];
+ mClient = fds[1];
+ } catch (IOException e) {
throw new RuntimeException("Failed to create bridge");
}
}
@@ -80,15 +81,14 @@
public void forceClose() {
IoUtils.closeQuietly(mTarget);
IoUtils.closeQuietly(mServer);
- IoUtils.closeQuietly(mClient);
mClosed = true;
}
- public void setTargetFile(FileDescriptor target) {
+ public void setTargetFile(ParcelFileDescriptor target) {
mTarget = target;
}
- public FileDescriptor getClientSocket() {
+ public ParcelFileDescriptor getClientSocket() {
return mClient;
}
@@ -96,32 +96,33 @@
public void run() {
final byte[] temp = new byte[8192];
try {
- while (IoBridge.read(mServer, temp, 0, MSG_LENGTH) == MSG_LENGTH) {
+ while (IoBridge.read(mServer.getFileDescriptor(), temp, 0, MSG_LENGTH) == MSG_LENGTH) {
final int cmd = Memory.peekInt(temp, 0, ByteOrder.BIG_ENDIAN);
if (cmd == CMD_WRITE) {
// Shuttle data into local file
int len = Memory.peekInt(temp, 4, ByteOrder.BIG_ENDIAN);
while (len > 0) {
- int n = IoBridge.read(mServer, temp, 0, Math.min(temp.length, len));
+ int n = IoBridge.read(mServer.getFileDescriptor(), temp, 0,
+ Math.min(temp.length, len));
if (n == -1) {
throw new IOException(
"Unexpected EOF; still expected " + len + " bytes");
}
- IoBridge.write(mTarget, temp, 0, n);
+ IoBridge.write(mTarget.getFileDescriptor(), temp, 0, n);
len -= n;
}
} else if (cmd == CMD_FSYNC) {
// Sync and echo back to confirm
- Os.fsync(mTarget);
- IoBridge.write(mServer, temp, 0, MSG_LENGTH);
+ Os.fsync(mTarget.getFileDescriptor());
+ IoBridge.write(mServer.getFileDescriptor(), temp, 0, MSG_LENGTH);
} else if (cmd == CMD_CLOSE) {
// Close and echo back to confirm
- Os.fsync(mTarget);
- Os.close(mTarget);
+ Os.fsync(mTarget.getFileDescriptor());
+ mTarget.close();
mClosed = true;
- IoBridge.write(mServer, temp, 0, MSG_LENGTH);
+ IoBridge.write(mServer.getFileDescriptor(), temp, 0, MSG_LENGTH);
break;
}
}
@@ -143,17 +144,11 @@
mClient = clientPfd.getFileDescriptor();
}
- public FileBridgeOutputStream(FileDescriptor client) {
- mClientPfd = null;
- mClient = client;
- }
-
@Override
public void close() throws IOException {
try {
writeCommandAndBlock(CMD_CLOSE, "close()");
} finally {
- IoBridge.closeAndSignalBlockedThreads(mClient);
IoUtils.closeQuietly(mClientPfd);
}
}
diff --git a/core/tests/coretests/src/android/os/FileBridgeTest.java b/core/tests/coretests/src/android/os/FileBridgeTest.java
index d4f6b1f..708bfa6 100644
--- a/core/tests/coretests/src/android/os/FileBridgeTest.java
+++ b/core/tests/coretests/src/android/os/FileBridgeTest.java
@@ -16,6 +16,9 @@
package android.os;
+import static android.os.ParcelFileDescriptor.MODE_CREATE;
+import static android.os.ParcelFileDescriptor.MODE_READ_WRITE;
+
import android.os.FileBridge.FileBridgeOutputStream;
import android.test.AndroidTestCase;
import android.test.MoreAsserts;
@@ -25,7 +28,6 @@
import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.FileInputStream;
-import java.io.FileOutputStream;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.Random;
@@ -33,7 +35,7 @@
public class FileBridgeTest extends AndroidTestCase {
private File file;
- private FileOutputStream fileOs;
+ private ParcelFileDescriptor outputFile;
private FileBridge bridge;
private FileBridgeOutputStream client;
@@ -44,17 +46,17 @@
file = getContext().getFileStreamPath("meow.dat");
file.delete();
- fileOs = new FileOutputStream(file);
+ outputFile = ParcelFileDescriptor.open(file, MODE_CREATE | MODE_READ_WRITE);
bridge = new FileBridge();
- bridge.setTargetFile(fileOs.getFD());
+ bridge.setTargetFile(outputFile);
bridge.start();
client = new FileBridgeOutputStream(bridge.getClientSocket());
}
@Override
protected void tearDown() throws Exception {
- fileOs.close();
+ outputFile.close();
file.delete();
}
diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java
index 949dcb25..9e6e1d9 100644
--- a/services/core/java/com/android/server/pm/PackageInstallerSession.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java
@@ -1086,6 +1086,23 @@
}
}
+ private ParcelFileDescriptor openTargetInternal(String path, int flags, int mode)
+ throws IOException, ErrnoException {
+ // TODO: this should delegate to DCS so the system process avoids
+ // holding open FDs into containers.
+ final FileDescriptor fd = Os.open(path, flags, mode);
+ return new ParcelFileDescriptor(fd);
+ }
+
+ private ParcelFileDescriptor createRevocableFdInternal(RevocableFileDescriptor fd,
+ ParcelFileDescriptor pfd) throws IOException {
+ int releasedFdInt = pfd.detachFd();
+ FileDescriptor releasedFd = new FileDescriptor();
+ releasedFd.setInt$(releasedFdInt);
+ fd.init(mContext, releasedFd);
+ return fd.getRevocableFileDescriptor();
+ }
+
private ParcelFileDescriptor doWriteInternal(String name, long offsetBytes, long lengthBytes,
ParcelFileDescriptor incomingFd) throws IOException {
// Quick validity check of state, and allocate a pipe for ourselves. We
@@ -1118,21 +1135,20 @@
Binder.restoreCallingIdentity(identity);
}
- // TODO: this should delegate to DCS so the system process avoids
- // holding open FDs into containers.
- final FileDescriptor targetFd = Os.open(target.getAbsolutePath(),
+ ParcelFileDescriptor targetPfd = openTargetInternal(target.getAbsolutePath(),
O_CREAT | O_WRONLY, 0644);
Os.chmod(target.getAbsolutePath(), 0644);
// If caller specified a total length, allocate it for them. Free up
// cache space to grow, if needed.
if (stageDir != null && lengthBytes > 0) {
- mContext.getSystemService(StorageManager.class).allocateBytes(targetFd, lengthBytes,
+ mContext.getSystemService(StorageManager.class).allocateBytes(
+ targetPfd.getFileDescriptor(), lengthBytes,
PackageHelper.translateAllocateFlags(params.installFlags));
}
if (offsetBytes > 0) {
- Os.lseek(targetFd, offsetBytes, OsConstants.SEEK_SET);
+ Os.lseek(targetPfd.getFileDescriptor(), offsetBytes, OsConstants.SEEK_SET);
}
if (incomingFd != null) {
@@ -1142,8 +1158,9 @@
// inserted above to hold the session active.
try {
final Int64Ref last = new Int64Ref(0);
- FileUtils.copy(incomingFd.getFileDescriptor(), targetFd, lengthBytes, null,
- Runnable::run, (long progress) -> {
+ FileUtils.copy(incomingFd.getFileDescriptor(), targetPfd.getFileDescriptor(),
+ lengthBytes, null, Runnable::run,
+ (long progress) -> {
if (params.sizeBytes > 0) {
final long delta = progress - last.value;
last.value = progress;
@@ -1154,7 +1171,7 @@
}
});
} finally {
- IoUtils.closeQuietly(targetFd);
+ IoUtils.closeQuietly(targetPfd);
IoUtils.closeQuietly(incomingFd);
// We're done here, so remove the "bridge" that was holding
@@ -1170,12 +1187,11 @@
}
return null;
} else if (PackageInstaller.ENABLE_REVOCABLE_FD) {
- fd.init(mContext, targetFd);
- return fd.getRevocableFileDescriptor();
+ return createRevocableFdInternal(fd, targetPfd);
} else {
- bridge.setTargetFile(targetFd);
+ bridge.setTargetFile(targetPfd);
bridge.start();
- return new ParcelFileDescriptor(bridge.getClientSocket());
+ return bridge.getClientSocket();
}
} catch (ErrnoException e) {