jail: fix several issues discovered by Coverity

Coverity CID: 1430874 Untrusted pointer read
Coverity CID: 1490028 Resource leak
Coverity CID: 1490029 Resource leak
Coverity CID: 1490057 Uninitialized scalar variable
Coverity CID: 1490069 Resource leak
Coverity CID: 1490074 Resource leak

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

3 files changed, 39 insertions, 24 deletions

diff --git a/jail/cgroups.c b/jail/cgroups.c
index 3e53dd5..a9484e8 100644
--- a/jail/cgroups.c
+++ b/jail/cgroups.c
@@ -178,13 +178,14 @@ void cgroups_apply(pid_t pid)
 		*cdir = '\0';
 		snprintf(ent, maxlen, "%s/cgroup.subtree_control", cgroup_path);
 		DEBUG(" * %s\n", ent);
-		if ((fd = open(ent, O_WRONLY)) == -1) {
+		if ((fd = open(ent, O_WRONLY)) < 0) {
 			ERROR("can't open %s: %m\n", ent);
 			continue;
 		}
 
 		if (write(fd, subtree_control, strlen(subtree_control)) == -1) {
 			ERROR("can't write to %s: %m\n", ent);
+			close(fd);
 			continue;
 		}
 
@@ -196,7 +197,7 @@ void cgroups_apply(pid_t pid)
 		DEBUG("applying cgroup2 %s=\"%s\"\n", (char *)valp->avl.key, valp->val);
 		snprintf(ent, maxlen, "%s/%s", cgroup_path, (char *)valp->avl.key);
 		fd = open(ent, O_WRONLY);
-		if (fd == -1) {
+		if (fd < 0) {
 			ERROR("can't open %s: %m\n", ent);
 			continue;
 		}
diff --git a/jail/fs.c b/jail/fs.c
index f70a751..d6aed00 100644
--- a/jail/fs.c
+++ b/jail/fs.c
@@ -105,7 +105,7 @@ static int do_mount(const char *root, const char *orig_source, const char *targe
 		mkdir_p(dirname(new), 0755);
 		snprintf(new, sizeof(new), "%s%s", root, target?target:source);
 		fd = creat(new, 0644);
-		if (fd == -1) {
+		if (fd < 0) {
 			ERROR("creat(%s) failed: %m\n", new);
 			return error;
 		}
@@ -414,7 +414,7 @@ static void build_noafile(void) {
 	int fd;
 
 	fd = creat(UJAIL_NOAFILE, 0000);
-	if (fd == -1)
+	if (fd < 0)
 		return;
 
 	close(fd);
@@ -493,7 +493,7 @@ int add_path_and_deps(const char *path, int readonly, int error, int lib)
 		if (avl_find(&mounts, path))
 			return 0;
 		fd = open(path, O_RDONLY|O_CLOEXEC);
-		if (fd == -1)
+		if (fd < 0)
 			return error;
 		add_mount_bind(path, readonly, error);
 	} else {
@@ -501,7 +501,7 @@ int add_path_and_deps(const char *path, int readonly, int error, int lib)
 			return 0;
 		char *fullpath;
 		fd = lib_open(&fullpath, path);
-		if (fd == -1)
+		if (fd < 0)
 			return error;
 		if (fullpath) {
 			alloc_library(fullpath, path);
diff --git a/jail/jail.c b/jail/jail.c
index 92ced45..fc8d824 100644
--- a/jail/jail.c
+++ b/jail/jail.c
@@ -315,7 +315,7 @@ static int mount_overlay(char *jail_root, char *overlaydir) {
 		goto upper_etc_printf;
 
 	fd = creat(upperresolvconf, 0644);
-	if (fd == -1) {
+	if (fd < 0) {
 		if (errno != EEXIST)
 			ERROR("creat(%s) failed: %m\n", upperresolvconf);
 	} else {
@@ -372,7 +372,7 @@ static int create_dev_console(const char *jail_root)
 
 	/* Open UNIX/98 virtual console */
 	console_fd = posix_openpt(O_RDWR | O_NOCTTY);
-	if (console_fd == -1)
+	if (console_fd < 0)
 		return -1;
 
 	console_fname = ptsname(console_fd);
@@ -532,12 +532,15 @@ static int apply_sysctl(const char *jail_root)
 		DEBUG("sysctl: writing '%s' to %s\n", (*cur)->value, fname);
 
 		f = open(fname, O_WRONLY);
-		if (f == -1) {
+		if (f < 0) {
 			ERROR("sysctl: can't open %s\n", fname);
+			free(fname);
 			return errno;
 		}
 		if (write(f, (*cur)->value, strlen((*cur)->value)) < 0) {
 			ERROR("sysctl: write to %s\n", fname);
+			free(fname);
+			close(f);
 			return errno;
 		}
 
@@ -779,7 +782,7 @@ static int write_uid_gid_map(pid_t child_pid, bool gidmap, char *mapstr)
 		child_pid, gidmap?"gid_map":"uid_map") < 0)
 		return -1;
 
-	if ((map_file = open(map_path, O_WRONLY)) == -1)
+	if ((map_file = open(map_path, O_WRONLY)) < 0)
 		return -1;
 
 	if (dprintf(map_file, "%s", mapstr)) {
@@ -800,10 +803,10 @@ static int write_single_uid_gid_map(pid_t child_pid, bool gidmap, int id)
 		child_pid, gidmap?"gid_map":"uid_map") < 0)
 		return -1;
 
-	if ((map_file = open(map_path, O_WRONLY)) == -1)
+	if ((map_file = open(map_path, O_WRONLY)) < 0)
 		return -1;
 
-	if (dprintf(map_file, map_format, 0, id, 1) == -1) {
+	if (dprintf(map_file, map_format, 0, id, 1) < 0) {
 		close(map_file);
 		return -1;
 	}
@@ -822,7 +825,7 @@ static int write_setgroups(pid_t child_pid, bool allow)
 		return -1;
 	}
 
-	if ((setgroups_file = open(setgroups_path, O_WRONLY)) == -1) {
+	if ((setgroups_file = open(setgroups_path, O_WRONLY)) < 0) {
 		return -1;
 	}
 
@@ -1014,7 +1017,7 @@ static int setns_open(unsigned long nstype)
 
 	assert(fd != NULL);
 
-	if (*fd == -1)
+	if (*fd < 0)
 		return 0;
 
 	if (setns(*fd, nstype) == -1) {
@@ -1347,7 +1350,8 @@ static int parseOCIroot(const char *jsonfile, struct blob_attr *msg)
 
 	/* prepend bundle directory in case of relative paths */
 	if (root_path[0] != '/') {
-		strncpy(extroot, jsonfile, PATH_MAX);
+		strncpy(extroot, jsonfile, PATH_MAX - 1);
+
 		cur = strrchr(extroot, '/');
 
 		if (!cur)
@@ -1815,11 +1819,13 @@ static int parseOCIlinuxns(struct blob_attr *msg)
 			blobmsg_get_string(tb[OCI_LINUX_NAMESPACE_PATH]));
 
 		fd = open(blobmsg_get_string(tb[OCI_LINUX_NAMESPACE_PATH]), O_RDONLY);
-		if (fd == -1)
+		if (fd < 0)
 			return errno?:ESTALE;
 
-		if (ioctl(fd, NS_GET_NSTYPE) != nstype)
+		if (ioctl(fd, NS_GET_NSTYPE) != nstype) {
+			close(fd);
 			return EINVAL;
+		}
 
 		DEBUG("opened existing %s namespace got filehandler %u\n",
 			blobmsg_get_string(tb[OCI_LINUX_NAMESPACE_TYPE]),
@@ -1967,20 +1973,26 @@ static int parseOCIdevices(struct blob_attr *msg)
 			return ENOMEM;
 
 		tmp->mode = resolve_devtype(blobmsg_get_string(tb[OCI_DEVICES_TYPE]));
-		if (!tmp->mode)
+		if (!tmp->mode) {
+			free(tmp);
 			return EINVAL;
+		}
 
 		if (tmp->mode != S_IFIFO) {
-			if (!tb[OCI_DEVICES_MAJOR] || !tb[OCI_DEVICES_MINOR])
+			if (!tb[OCI_DEVICES_MAJOR] || !tb[OCI_DEVICES_MINOR]) {
+				free(tmp);
 				return ENODATA;
+			}
 
 			tmp->dev = makedev(blobmsg_get_u32(tb[OCI_DEVICES_MAJOR]),
 					   blobmsg_get_u32(tb[OCI_DEVICES_MINOR]));
 		}
 
 		if (tb[OCI_DEVICES_FILEMODE]) {
-			if (~(S_IRWXU|S_IRWXG|S_IRWXO) & blobmsg_get_u32(tb[OCI_DEVICES_FILEMODE]))
+			if (~(S_IRWXU|S_IRWXG|S_IRWXO) & blobmsg_get_u32(tb[OCI_DEVICES_FILEMODE])) {
+				free(tmp);
 				return EINVAL;
+			}
 
 			tmp->mode |= blobmsg_get_u32(tb[OCI_DEVICES_FILEMODE]);
 		} else {
@@ -2282,7 +2294,7 @@ static int set_oom_score_adj(void)
 
 	snprintf(fname, sizeof(fname), "/proc/%u/oom_score_adj", jail_process.pid);
 	f = open(fname, O_WRONLY | O_TRUNC);
-	if (f == -1)
+	if (f < 0)
 		return errno;
 
 	dprintf(f, "%d", opts.oom_score_adj);
@@ -2423,7 +2435,7 @@ jail_writepid(pid_t pid)
 static int checkpath(const char *path)
 {
 	int dirfd = open(path, O_RDONLY | O_DIRECTORY | O_CLOEXEC);
-	if (dirfd == -1) {
+	if (dirfd < 0) {
 		ERROR("path %s open failed %m\n", path);
 		return -1;
 	}
@@ -2462,7 +2474,8 @@ int main(int argc, char **argv)
 	uid_t uid = getuid();
 	const char log[] = "/dev/log";
 	const char ubus[] = "/var/run/ubus/ubus.sock";
-	int ch, ret;
+	int ret = EXIT_FAILURE;
+	int ch;
 
 	if (uid) {
 		ERROR("not root, aborting: %m\n");
@@ -2753,7 +2766,8 @@ static void post_main(struct uloop_timeout *t)
 
 			if (!(opts.namespace & CLONE_NEWNET)) {
 				add_mount_bind("/etc/resolv.conf", 1, 0);
-			} else if (opts.setns.net == -1) {
+			} else if (opts.setns.ns == -1) {
+				/* new mount namespace to provide /dev/resolv.conf.d */
 				char hostdir[PATH_MAX];
 
 				snprintf(hostdir, PATH_MAX, "/tmp/resolv.conf-%s.d", opts.name);