Merge "FileBridge: fix fd ownership mismanagement." am: e80c7cb69a (bc2ef840) · Commits · e / os / android_frameworks_base

core/java/android/os/FileBridge.java

+18 −23

Original line number	Original line	Diff line number	Diff line
	@@ -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);
	}		}
	}		}

core/tests/coretests/src/android/os/FileBridgeTest.java

+7 −5

Original line number	Original line	Diff line number	Diff line
	@@ -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();
	}		}

services/core/java/com/android/server/pm/PackageInstallerSession.java

+28 −12

Original line number	Original line	Diff line number	Diff line
	@@ -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) {