Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge "FileBridge: fix fd ownership mismanagement." am: e80c7cb69a

Browse Source 
Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/1433025

Change-Id: Iab74aa9ba074f6c953d658a12103e5e74a9ea13c
This commit is contained in:
Treehugger Robot
2020-10-01 23:44:09 +00:00
committed by Automerger Merge Worker
parent f74b2d4192 e80c7cb69a
commit bc2ef840e1
3 changed files with 53 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

41
core/java/android/os/FileBridge.java
View File

@@ -16,7 +16,6 @@
package android.os; package android.os;
import static android.system.OsConstants.AF_UNIX;
import static android.system.OsConstants.SOCK_STREAM; import static android.system.OsConstants.SOCK_STREAM;
import android.system.ErrnoException; import android.system.ErrnoException;
@@ -58,17 +57,19 @@ public class FileBridge extends Thread {
/** CMD_CLOSE */ /** CMD_CLOSE */
private static final int CMD_CLOSE = 3; private static final int CMD_CLOSE = 3;
private FileDescriptor mTarget; private ParcelFileDescriptor mTarget;
private final FileDescriptor mServer = new FileDescriptor(); private ParcelFileDescriptor mServer;
private final FileDescriptor mClient = new FileDescriptor(); private ParcelFileDescriptor mClient;
private volatile boolean mClosed; private volatile boolean mClosed;
public FileBridge() { public FileBridge() {
try { try {
Os.socketpair(AF_UNIX, SOCK_STREAM, 0, mServer, mClient); ParcelFileDescriptor[] fds = ParcelFileDescriptor.createSocketPair(SOCK_STREAM);
} catch (ErrnoException e) { mServer = fds[0];
mClient = fds[1];
} catch (IOException e) {
throw new RuntimeException("Failed to create bridge"); throw new RuntimeException("Failed to create bridge");
} }
} }
@@ -80,15 +81,14 @@ public class FileBridge extends Thread {
public void forceClose() { public void forceClose() {
IoUtils.closeQuietly(mTarget); IoUtils.closeQuietly(mTarget);
IoUtils.closeQuietly(mServer); IoUtils.closeQuietly(mServer);
IoUtils.closeQuietly(mClient);
mClosed = true; mClosed = true;
} }
public void setTargetFile(FileDescriptor target) { public void setTargetFile(ParcelFileDescriptor target) {
mTarget = target; mTarget = target;
} }
public FileDescriptor getClientSocket() { public ParcelFileDescriptor getClientSocket() {
return mClient; return mClient;
} }
@@ -96,32 +96,33 @@ public class FileBridge extends Thread {
public void run() { public void run() {
final byte[] temp = new byte[8192]; final byte[] temp = new byte[8192];
try { 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); final int cmd = Memory.peekInt(temp, 0, ByteOrder.BIG_ENDIAN);
if (cmd == CMD_WRITE) { if (cmd == CMD_WRITE) {
// Shuttle data into local file // Shuttle data into local file
int len = Memory.peekInt(temp, 4, ByteOrder.BIG_ENDIAN); int len = Memory.peekInt(temp, 4, ByteOrder.BIG_ENDIAN);
while (len > 0) { 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) { if (n == -1) {
throw new IOException( throw new IOException(
"Unexpected EOF; still expected " + len + " bytes"); "Unexpected EOF; still expected " + len + " bytes");
} }
IoBridge.write(mTarget, temp, 0, n); IoBridge.write(mTarget.getFileDescriptor(), temp, 0, n);
len -= n; len -= n;
} }
} else if (cmd == CMD_FSYNC) { } else if (cmd == CMD_FSYNC) {
// Sync and echo back to confirm // Sync and echo back to confirm
Os.fsync(mTarget); Os.fsync(mTarget.getFileDescriptor());
IoBridge.write(mServer, temp, 0, MSG_LENGTH); IoBridge.write(mServer.getFileDescriptor(), temp, 0, MSG_LENGTH);
} else if (cmd == CMD_CLOSE) { } else if (cmd == CMD_CLOSE) {
// Close and echo back to confirm // Close and echo back to confirm
Os.fsync(mTarget); Os.fsync(mTarget.getFileDescriptor());
Os.close(mTarget); mTarget.close();
mClosed = true; mClosed = true;
IoBridge.write(mServer, temp, 0, MSG_LENGTH); IoBridge.write(mServer.getFileDescriptor(), temp, 0, MSG_LENGTH);
break; break;
} }
} }
@@ -143,17 +144,11 @@ public class FileBridge extends Thread {
mClient = clientPfd.getFileDescriptor(); mClient = clientPfd.getFileDescriptor();
} }
public FileBridgeOutputStream(FileDescriptor client) {
mClientPfd = null;
mClient = client;
}
@Override @Override
public void close() throws IOException { public void close() throws IOException {
try { try {
writeCommandAndBlock(CMD_CLOSE, "close()"); writeCommandAndBlock(CMD_CLOSE, "close()");
} finally { } finally {
IoBridge.closeAndSignalBlockedThreads(mClient);
IoUtils.closeQuietly(mClientPfd); IoUtils.closeQuietly(mClientPfd);
} }
} }

12
core/tests/coretests/src/android/os/FileBridgeTest.java
View File

@@ -16,6 +16,9 @@
package android.os; 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.os.FileBridge.FileBridgeOutputStream;
import android.test.AndroidTestCase; import android.test.AndroidTestCase;
import android.test.MoreAsserts; import android.test.MoreAsserts;
@@ -25,7 +28,6 @@ import libcore.io.Streams;
import java.io.ByteArrayOutputStream; import java.io.ByteArrayOutputStream;
import java.io.File; import java.io.File;
import java.io.FileInputStream; import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException; import java.io.IOException;
import java.nio.charset.StandardCharsets; import java.nio.charset.StandardCharsets;
import java.util.Random; import java.util.Random;
@@ -33,7 +35,7 @@ import java.util.Random;
public class FileBridgeTest extends AndroidTestCase { public class FileBridgeTest extends AndroidTestCase {
private File file; private File file;
private FileOutputStream fileOs; private ParcelFileDescriptor outputFile;
private FileBridge bridge; private FileBridge bridge;
private FileBridgeOutputStream client; private FileBridgeOutputStream client;
@@ -44,17 +46,17 @@ public class FileBridgeTest extends AndroidTestCase {
file = getContext().getFileStreamPath("meow.dat"); file = getContext().getFileStreamPath("meow.dat");
file.delete(); file.delete();
fileOs = new FileOutputStream(file); outputFile = ParcelFileDescriptor.open(file, MODE_CREATE | MODE_READ_WRITE);
bridge = new FileBridge(); bridge = new FileBridge();
bridge.setTargetFile(fileOs.getFD()); bridge.setTargetFile(outputFile);
bridge.start(); bridge.start();
client = new FileBridgeOutputStream(bridge.getClientSocket()); client = new FileBridgeOutputStream(bridge.getClientSocket());
} }
@Override @Override
protected void tearDown() throws Exception { protected void tearDown() throws Exception {
fileOs.close(); outputFile.close();
file.delete(); file.delete();
} }

40
services/core/java/com/android/server/pm/PackageInstallerSession.java
View File

@@ -943,6 +943,23 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
} }
} }
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, private ParcelFileDescriptor doWriteInternal(String name, long offsetBytes, long lengthBytes,
ParcelFileDescriptor incomingFd) throws IOException { ParcelFileDescriptor incomingFd) throws IOException {
// Quick sanity check of state, and allocate a pipe for ourselves. We // Quick sanity check of state, and allocate a pipe for ourselves. We
@@ -975,21 +992,20 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
Binder.restoreCallingIdentity(identity); Binder.restoreCallingIdentity(identity);
} }
// TODO: this should delegate to DCS so the system process avoids ParcelFileDescriptor targetPfd = openTargetInternal(target.getAbsolutePath(),
// holding open FDs into containers.
final FileDescriptor targetFd = Os.open(target.getAbsolutePath(),
O_CREAT | O_WRONLY, 0644); O_CREAT | O_WRONLY, 0644);
Os.chmod(target.getAbsolutePath(), 0644); Os.chmod(target.getAbsolutePath(), 0644);
// If caller specified a total length, allocate it for them. Free up // If caller specified a total length, allocate it for them. Free up
// cache space to grow, if needed. // cache space to grow, if needed.
if (stageDir != null && lengthBytes > 0) { if (stageDir != null && lengthBytes > 0) {
mContext.getSystemService(StorageManager.class).allocateBytes(targetFd, lengthBytes, mContext.getSystemService(StorageManager.class).allocateBytes(
targetPfd.getFileDescriptor(), lengthBytes,
PackageHelper.translateAllocateFlags(params.installFlags)); PackageHelper.translateAllocateFlags(params.installFlags));
} }
if (offsetBytes > 0) { if (offsetBytes > 0) {
Os.lseek(targetFd, offsetBytes, OsConstants.SEEK_SET); Os.lseek(targetPfd.getFileDescriptor(), offsetBytes, OsConstants.SEEK_SET);
} }
if (incomingFd != null) { if (incomingFd != null) {
@@ -999,8 +1015,9 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
// inserted above to hold the session active. // inserted above to hold the session active.
try { try {
final Int64Ref last = new Int64Ref(0); final Int64Ref last = new Int64Ref(0);
FileUtils.copy(incomingFd.getFileDescriptor(), targetFd, lengthBytes, null, FileUtils.copy(incomingFd.getFileDescriptor(), targetPfd.getFileDescriptor(),
Runnable::run, (long progress) -> { lengthBytes, null, Runnable::run,
(long progress) -> {
if (params.sizeBytes > 0) { if (params.sizeBytes > 0) {
final long delta = progress - last.value; final long delta = progress - last.value;
last.value = progress; last.value = progress;
@@ -1011,7 +1028,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
} }
}); });
} finally { } finally {
IoUtils.closeQuietly(targetFd); IoUtils.closeQuietly(targetPfd);
IoUtils.closeQuietly(incomingFd); IoUtils.closeQuietly(incomingFd);
// We're done here, so remove the "bridge" that was holding // We're done here, so remove the "bridge" that was holding
@@ -1027,12 +1044,11 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
} }
return null; return null;
} else if (PackageInstaller.ENABLE_REVOCABLE_FD) { } else if (PackageInstaller.ENABLE_REVOCABLE_FD) {
fd.init(mContext, targetFd); return createRevocableFdInternal(fd, targetPfd);
return fd.getRevocableFileDescriptor();
} else { } else {
bridge.setTargetFile(targetFd); bridge.setTargetFile(targetPfd);
bridge.start(); bridge.start();
return new ParcelFileDescriptor(bridge.getClientSocket()); return bridge.getClientSocket();
} }
} catch (ErrnoException e) { } catch (ErrnoException e) {