1 files changed, 71 insertions, 6 deletions

diff --git a/libminijail.c b/libminijail.c
index 582554a..e0d9d54 100644
--- a/libminijail.c
+++ b/libminijail.c
@@ -8,6 +8,7 @@
 #define _GNU_SOURCE
 
 #include <asm/unistd.h>
+#include <assert.h>
 #include <dirent.h>
 #include <errno.h>
 #include <fcntl.h>
@@ -26,6 +27,7 @@
 #include <sys/param.h>
 #include <sys/prctl.h>
 #include <sys/resource.h>
+#include <sys/select.h>
 #include <sys/stat.h>
 #include <sys/sysmacros.h>
 #include <sys/types.h>
@@ -2592,8 +2594,74 @@ static int close_open_fds(int *inheritable_fds, size_t size)
 	return 0;
 }
 
+/* Return true if the specified file descriptor is already open. */
+static int fd_is_open(int fd)
+{
+	return fcntl(fd, F_GETFD) != -1 || errno != EBADF;
+}
+
+static_assert(FD_SETSIZE >= MAX_PRESERVED_FDS * 2 - 1,
+	      "If true, ensure_no_fd_conflict will always find an unused fd.");
+
+/* If p->parent_fd will be used by a child_fd, move it to an unused fd. */
+static int ensure_no_fd_conflict(const fd_set* child_fds,
+				 struct preserved_fd* p)
+{
+	if (!FD_ISSET(p->parent_fd, child_fds)){
+		return 0;
+	}
+
+	/*
+	 * If no other parent_fd matches the child_fd then use it instead of a
+	 * temporary.
+	 */
+	int fd = p->child_fd;
+	if (fd_is_open(fd)) {
+		fd = FD_SETSIZE - 1;
+		while (FD_ISSET(fd, child_fds) || fd_is_open(fd)) {
+			--fd;
+			if (fd < 0) {
+				die("failed to find an unused fd");
+			}
+		}
+	}
+
+	int ret = dup2(p->parent_fd, fd);
+	/*
+	 * warn() opens a file descriptor so it needs to happen after dup2 to
+	 * avoid unintended side effects. This can be avoided by reordering the
+	 * mapping requests so that the source fds with overlap are mapped
+	 * first (unless there are cycles).
+	 */
+	warn("mapped fd overlap: moving %d to %d", p->parent_fd, fd);
+	if (ret == -1) {
+		return -1;
+	}
+
+	p->parent_fd = fd;
+	return 0;
+}
+
 static int redirect_fds(struct minijail *j)
 {
+	fd_set child_fds;
+	FD_ZERO(&child_fds);
+
+	/* Relocate parent_fds that would be replaced by a child_fd. */
+	for (size_t i = 0; i < j->preserved_fd_count; i++) {
+		int child_fd = j->preserved_fds[i].child_fd;
+		if (FD_ISSET(child_fd, &child_fds)) {
+			die("fd %d is mapped more than once", child_fd);
+		}
+
+		if (ensure_no_fd_conflict(&child_fds,
+					  &j->preserved_fds[i]) == -1) {
+			return -1;
+		}
+
+		FD_SET(child_fd, &child_fds);
+	}
+
 	for (size_t i = 0; i < j->preserved_fd_count; i++) {
 		if (j->preserved_fds[i].parent_fd ==
 		    j->preserved_fds[i].child_fd) {
@@ -2609,13 +2677,10 @@ static int redirect_fds(struct minijail *j)
 	 * fds that are *not* child fds.
 	 */
 	for (size_t i = 0; i < j->preserved_fd_count; i++) {
-		int closeable = true;
-		for (size_t i2 = 0; i2 < j->preserved_fd_count; i2++) {
-			closeable &= j->preserved_fds[i].parent_fd !=
-				     j->preserved_fds[i2].child_fd;
+		int parent_fd = j->preserved_fds[i].parent_fd;
+		if (!FD_ISSET(parent_fd, &child_fds)) {
+			close(parent_fd);
 		}
-		if (closeable)
-			close(j->preserved_fds[i].parent_fd);
 	}
 	return 0;
 }