[lxc-devel] [lxc/master] utils: fix lxc_mount_proc_if_needed()

[brauner on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 529 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20170518/088d888f/attachment.bin>
-------------- next part --------------
From a3815c4e185ebee0d78a8bf83c7c0b295b3e192a Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 18 May 2017 16:31:42 +0200
Subject: [PATCH] utils: fix lxc_mount_proc_if_needed()

- check for buffer overflow
- only call INFO() after we ensured that readlink() was successful
- simplify logic

Reported-by: Benedikt Rosenkranz beluro at web.de
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/lxc/utils.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/src/lxc/utils.c b/src/lxc/utils.c
index 15c9f91..ec00e89 100644
--- a/src/lxc/utils.c
+++ b/src/lxc/utils.c
@@ -1758,9 +1758,8 @@ int safe_mount(const char *src, const char *dest, const char *fstype,
 int lxc_mount_proc_if_needed(const char *rootfs)
 {
 	char path[MAXPATHLEN];
-	char link[20];
-	int link_to_pid, linklen, ret;
-	int mypid;
+	int link_to_pid, linklen, mypid, ret;
+	char link[LXC_NUMSTRLEN64] = {0};
 
 	ret = snprintf(path, MAXPATHLEN, "%s/proc/self", rootfs);
 	if (ret < 0 || ret >= MAXPATHLEN) {
@@ -1768,10 +1767,7 @@ int lxc_mount_proc_if_needed(const char *rootfs)
 		return -1;
 	}
 
-	memset(link, 0, 20);
 	linklen = readlink(path, link, 20);
-	mypid = (int)getpid();
-	INFO("I am %d, /proc/self points to \"%s\"", mypid, link);
 
 	ret = snprintf(path, MAXPATHLEN, "%s/proc", rootfs);
 	if (ret < 0 || ret >= MAXPATHLEN) {
@@ -1784,24 +1780,29 @@ int lxc_mount_proc_if_needed(const char *rootfs)
 		if (mkdir(path, 0755) && errno != EEXIST)
 			return -1;
 		goto domount;
+	} else if (linklen >= LXC_NUMSTRLEN64) {
+		link[linklen - 1] = '\0';
+		ERROR("readlink returned truncated content: \"%s\"", link);
+		return -1;
 	}
 
+	mypid = getpid();
+	INFO("I am %d, /proc/self points to \"%s\"", mypid, link);
+
 	if (lxc_safe_int(link, &link_to_pid) < 0)
 		return -1;
 
-	/* wrong /procs mounted */
-	if (link_to_pid != mypid) {
-		/* ignore failure */
-		umount2(path, MNT_DETACH);
-		goto domount;
-	}
+	/* correct procfs is already mounted */
+	if (link_to_pid == mypid)
+		return 0;
 
-	/* the right proc is already mounted */
-	return 0;
+	ret = umount2(path, MNT_DETACH);
+	if (ret < 0)
+		WARN("failed to umount \"%s\" with MNT_DETACH", path);
 
 domount:
 	/* rootfs is NULL */
-	if (!strcmp(rootfs,""))
+	if (!strcmp(rootfs, ""))
 		ret = mount("proc", path, "proc", 0, NULL);
 	else
 		ret = safe_mount("proc", path, "proc", 0, NULL, rootfs);