[lxc-devel] [PATCH v2] safe_mount: Handle mounting proc and refactor

[Bogdan Purcareata]

The safe_mount primitive will mount the fs in the new container
environment by using file descriptors referred in /proc/self/fd.
However, when the mounted filesystem is proc itself, it will have
been previously unmounted, therefore resulting in an error when
searching for these file descriptors. This only happens when there's
no container rootfs prefix (commonly with lxc-execute).

Implement the support for this use case as well, by doing the mount
based on the full path.

Refactor the whole function in order to remove duplicated code checks
and improve readability.

Changes since v1:
- In order to address CVE-2015-1335, still check if the destination is
not a symlink. Do the mount only if the destination file descriptor
exists.

Signed-off-by: Bogdan Purcareata <bogdan.purcareata at nxp.com>
---
 src/lxc/utils.c | 49 ++++++++++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 21 deletions(-)

diff --git a/src/lxc/utils.c b/src/lxc/utils.c
index d9e769d..c53711a 100644
--- a/src/lxc/utils.c
+++ b/src/lxc/utils.c
@@ -1644,9 +1644,9 @@ out:
 int safe_mount(const char *src, const char *dest, const char *fstype,
 		unsigned long flags, const void *data, const char *rootfs)
 {
-	int srcfd = -1, destfd, ret, saved_errno;
+	int srcfd = -1, destfd = -1, ret = 0;
 	char srcbuf[50], destbuf[50]; // only needs enough for /proc/self/fd/<fd>
-	const char *mntsrc = src;
+	const char *mntsrc = src, *mntdest = dest;
 
 	if (!rootfs)
 		rootfs = "";
@@ -1655,45 +1655,52 @@ int safe_mount(const char *src, const char *dest, const char *fstype,
 	if (flags & MS_BIND && src && src[0] != '/') {
 		INFO("this is a relative bind mount");
 		srcfd = open_without_symlink(src, NULL);
-		if (srcfd < 0)
-			return srcfd;
+		if (srcfd < 0) {
+			ret = srcfd;
+			goto out;
+		}
 		ret = snprintf(srcbuf, 50, "/proc/self/fd/%d", srcfd);
 		if (ret < 0 || ret > 50) {
-			close(srcfd);
 			ERROR("Out of memory");
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out_src;
 		}
 		mntsrc = srcbuf;
 	}
 
 	destfd = open_without_symlink(dest, rootfs);
 	if (destfd < 0) {
-		if (srcfd != -1)
-			close(srcfd);
-		return destfd;
+		ret = destfd;
+		goto out_src;
 	}
 
 	ret = snprintf(destbuf, 50, "/proc/self/fd/%d", destfd);
 	if (ret < 0 || ret > 50) {
-		if (srcfd != -1)
-			close(srcfd);
-		close(destfd);
 		ERROR("Out of memory");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_dest;
 	}
 
-	ret = mount(mntsrc, destbuf, fstype, flags, data);
-	saved_errno = errno;
-	if (srcfd != -1)
-		close(srcfd);
-	close(destfd);
+	/* make sure the destination descriptor exists */
+	if (access(destbuf, F_OK) == 0)
+		mntdest = destbuf;
+
+	ret = mount(mntsrc, mntdest, fstype, flags, data);
 	if (ret < 0) {
-		errno = saved_errno;
 		SYSERROR("Failed to mount %s onto %s", src, dest);
-		return ret;
+		goto out_dest;
 	}
 
-	return 0;
+	ret = 0;
+
+out_dest:
+	if (destfd > 0)
+		close(destfd);
+out_src:
+	if (srcfd > 0)
+		close(srcfd);
+out:
+	return ret;
 }
 
 /*
-- 
1.9.1