From: Daniel Golle <daniel@makrotopia.org>
Date: Thu, 12 Jan 2023 17:57:51 +0000 (+0000)
Subject: jail: fs: don't overwrite existing mount target
X-Git-Url: http://git.openwrt.org/?a=commitdiff_plain;ds=sidebyside;h=04d7570f473ca2a657a89f74ac4027f6c94f4305;p=project%2Fprocd.git

jail: fs: don't overwrite existing mount target

Using the creat() function overwrites existing files which is
unintended when it comes to making sure the target of a single-file
mount exists. Instead, use open() with the O_EXCL flag to make sure
mount targets are only created if actually needed.

While at it also clean up various error paths of the do_mounts
function, making sure the additionally allocated string being created
for the path inside the jail's root filesystem is always freed
and also making it a bit more readable and less bloated.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---

diff --git a/jail/fs.c b/jail/fs.c
index c14027b..423878b 100644
--- a/jail/fs.c
+++ b/jail/fs.c
@@ -61,20 +61,22 @@ static int do_mount(const char *root, const char *orig_source, const char *targe
 	struct stat s;
 	char new[PATH_MAX];
 	char *source = (char *)orig_source;
-	int fd;
+	int fd, ret = 0;
 	bool is_bind = (orig_mountflags & MS_BIND);
 	bool is_mask = (source == (void *)(-1));
 	unsigned long mountflags = orig_mountflags;
 
+	assert(!(inner && is_mask));
+	assert(!(inner && !orig_source));
+
 	if (source && is_bind && stat(source, &s)) {
 		ERROR("stat(%s) failed: %m\n", source);
 		return error;
 	}
 
-	if (!is_mask && orig_source && inner) {
+	if (inner)
 		if (asprintf(&source, "%s%s", root, orig_source) < 0)
 			return ENOMEM;
-	}
 
 	snprintf(new, sizeof(new), "%s%s", root, target?target:source);
 
@@ -83,7 +85,7 @@ static int do_mount(const char *root, const char *orig_source, const char *targe
 			return 0; /* doesn't exists, nothing to mask */
 
 		if (S_ISDIR(s.st_mode)) {/* use empty 0-sized tmpfs for directories */
-			if (mount(NULL, new, "tmpfs", MS_RDONLY | MS_NOSUID | MS_NOEXEC | MS_NODEV | MS_NOATIME, "size=0,mode=000"))
+			if (mount("none", new, "tmpfs", MS_RDONLY | MS_NOSUID | MS_NOEXEC | MS_NODEV | MS_NOATIME, "size=0,mode=000"))
 				return error;
 		} else {
 			/* mount-bind 0-sized file having mode 000 */
@@ -104,12 +106,16 @@ static int do_mount(const char *root, const char *orig_source, const char *targe
 	} else if (is_bind && source) {
 		mkdir_p(dirname(new), 0755);
 		snprintf(new, sizeof(new), "%s%s", root, target?target:source);
-		fd = creat(new, 0644);
-		if (fd < 0) {
-			ERROR("creat(%s) failed: %m\n", new);
-			return error;
+		fd = open(new, O_CREAT|O_WRONLY|O_TRUNC|O_EXCL, 0644);
+		if (fd >= 0)
+			close(fd);
+
+		if (error && fd < 0 && errno != EEXIST) {
+			ERROR("failed to create mount target %s: %m\n", new);
+
+			ret = errno;
+			goto free_source_out;
 		}
-		close(fd);
 	}
 
 	if (is_bind) {
@@ -117,10 +123,8 @@ static int do_mount(const char *root, const char *orig_source, const char *targe
 			if (error)
 				ERROR("failed to mount -B %s %s: %m\n", source, new);
 
-			if (inner)
-				free(source);
-
-			return error;
+			ret = error;
+			goto free_source_out;
 		}
 		mountflags |= MS_REMOUNT;
 	}
@@ -130,10 +134,8 @@ static int do_mount(const char *root, const char *orig_source, const char *targe
 		if (error)
 			ERROR("failed to mount %s %s: %m\n", source, new);
 
-		if (inner)
-			free(source);
-
-		return error;
+		ret = error;
+		goto free_source_out;
 	}
 
 	DEBUG("mount %s%s %s (%s)\n", (mountflags & MS_BIND)?"-B ":"", source, new,
@@ -143,16 +145,14 @@ static int do_mount(const char *root, const char *orig_source, const char *targe
 		if (error)
 			ERROR("failed to mount --make-... %s \n", new);
 
-		if (inner)
-			free(source);
-
-		return error;
+		ret = error;
 	}
 
+free_source_out:
 	if (inner)
 		free(source);
 
-	return 0;
+	return ret;
 }
 
 static int _add_mount(const char *source, const char *target, const char *filesystemtype,