Merge pull request #3381 from brauner/2020-04-15/fixes

src/lxc/attach.c

@@ -1018,6 +1018,8 @@ int lxc_attach(struct lxc_container *container, lxc_attach_exec_t exec_function,
 		}
 	}
 	conf = init_ctx->container->lxc_conf;
+	if (!conf)
+		return log_error_errno(-EINVAL, EINVAL, "Missing container confifg");
 
 	if (!fetch_seccomp(init_ctx->container, options))
 		WARN("Failed to get seccomp policy");
@@ -1275,7 +1277,7 @@ int lxc_attach(struct lxc_container *container, lxc_attach_exec_t exec_function,
 			TRACE("Sent LSM label file descriptor %d to child", labelfd);
 		}
 
-		if (conf && conf->seccomp.seccomp) {
+		if (conf->seccomp.seccomp) {
 			ret = lxc_seccomp_recv_notifier_fd(&conf->seccomp, ipc_sockets[0]);
 			if (ret < 0)
 				goto close_mainloop;

src/lxc/cgroups/cgfsng.c

@@ -2736,6 +2736,9 @@ static int device_cgroup_rule_parse_devpath(struct device_item *device,
 			return ret_set_errno(-1, EINVAL);
 	}
 
+	if (!mode)
+		return ret_errno(EINVAL);
+
 	if (device_cgroup_parse_access(device, mode) < 0)
 		return -1;
 

src/lxc/cgroups/cgroup2_devices.c

@@ -167,7 +167,7 @@ struct bpf_program *bpf_program_new(uint32_t prog_type)
 {
 	__do_free struct bpf_program *prog = NULL;
 
-	prog = calloc(1, sizeof(struct bpf_program));
+	prog = zalloc(sizeof(struct bpf_program));
 	if (!prog)
 		return NULL;
 
@@ -183,9 +183,6 @@ struct bpf_program *bpf_program_new(uint32_t prog_type)
 
 int bpf_program_init(struct bpf_program *prog)
 {
-	if (!prog)
-		return ret_set_errno(-1, EINVAL);
-
 	const struct bpf_insn pre_insn[] = {
 	    /* load device type to r2 */
 	    BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1, offsetof(struct bpf_cgroup_dev_ctx, access_type)),
@@ -202,19 +199,17 @@ int bpf_program_init(struct bpf_program *prog)
 	    BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1, offsetof(struct bpf_cgroup_dev_ctx, minor)),
 	};
 
+	if (!prog)
+		return ret_set_errno(-1, EINVAL);
+
 	return bpf_program_add_instructions(prog, pre_insn, ARRAY_SIZE(pre_insn));
 }
 
 int bpf_program_append_device(struct bpf_program *prog, struct device_item *device)
 {
-	int ret;
 	int jump_nr = 1;
-	struct bpf_insn bpf_access_decision[] = {
-	    BPF_MOV64_IMM(BPF_REG_0, device->allow),
-	    BPF_EXIT_INSN(),
-	};
-	int access_mask;
-	int device_type;
+	int access_mask, device_type, ret;
+	struct bpf_insn bpf_access_decision[2];
 
 	if (!prog || !device)
 		return ret_set_errno(-1, EINVAL);
@@ -285,6 +280,8 @@ int bpf_program_append_device(struct bpf_program *prog, struct device_item *devi
 			return log_error_errno(-1, errno, "Failed to add instructions to bpf cgroup program");
 	}
 
+	bpf_access_decision[0] = BPF_MOV64_IMM(BPF_REG_0, device->allow);
+	bpf_access_decision[1] = BPF_EXIT_INSN();
 	ret = bpf_program_add_instructions(prog, bpf_access_decision,
 					    ARRAY_SIZE(bpf_access_decision));
 	if (ret)
@@ -295,10 +292,7 @@ int bpf_program_append_device(struct bpf_program *prog, struct device_item *devi
 
 int bpf_program_finalize(struct bpf_program *prog)
 {
-	struct bpf_insn ins[] = {
-	    BPF_MOV64_IMM(BPF_REG_0, prog->device_list_type),
-	    BPF_EXIT_INSN(),
-	};
+	struct bpf_insn ins[2];
 
 	if (!prog)
 		return ret_set_errno(-1, EINVAL);
@@ -307,6 +301,9 @@ int bpf_program_finalize(struct bpf_program *prog)
 	      prog->device_list_type == LXC_BPF_DEVICE_CGROUP_BLACKLIST
 		  ? "blacklist"
 		  : "whitelist");
+
+	ins[0] = BPF_MOV64_IMM(BPF_REG_0, prog->device_list_type);
+	ins[1] = BPF_EXIT_INSN();
 	return bpf_program_add_instructions(prog, ins, ARRAY_SIZE(ins));
 }
 
@@ -340,12 +337,12 @@ static int bpf_program_load_kernel(struct bpf_program *prog, char *log_buf,
 int bpf_program_cgroup_attach(struct bpf_program *prog, int type,
 			      const char *path, uint32_t flags)
 {
-	__do_free char *copy = NULL;
 	__do_close int fd = -EBADF;
+	__do_free char *copy = NULL;
 	union bpf_attr attr;
 	int ret;
 
-	if (!prog)
+	if (!path || !prog)
 		return ret_set_errno(-1, EINVAL);
 
 	if (flags & ~(BPF_F_ALLOW_OVERRIDE | BPF_F_ALLOW_MULTI))
@@ -395,8 +392,8 @@ int bpf_program_cgroup_attach(struct bpf_program *prog, int type,
 
 int bpf_program_cgroup_detach(struct bpf_program *prog)
 {
-	int ret;
 	__do_close int fd = -EBADF;
+	int ret;
 
 	if (!prog)
 		return 0;
@@ -444,6 +441,9 @@ int bpf_list_add_device(struct lxc_conf *conf, struct device_item *device)
 	__do_free struct device_item *new_device = NULL;
 	struct lxc_list *it;
 
+	if (!conf || !device)
+		return ret_errno(EINVAL);
+
 	lxc_list_for_each(it, &conf->devices) {
 		struct device_item *cur = it->elem;
 
@@ -502,12 +502,11 @@ int bpf_list_add_device(struct lxc_conf *conf, struct device_item *device)
 
 bool bpf_devices_cgroup_supported(void)
 {
+	__do_bpf_program_free struct bpf_program *prog = NULL;
 	const struct bpf_insn dummy[] = {
 	    BPF_MOV64_IMM(BPF_REG_0, 1),
 	    BPF_EXIT_INSN(),
 	};
-
-	__do_bpf_program_free struct bpf_program *prog = NULL;
 	int ret;
 
 	if (geteuid() != 0)
@@ -515,7 +514,7 @@ bool bpf_devices_cgroup_supported(void)
 				 "The bpf device cgroup requires real root");
 
 	prog = bpf_program_new(BPF_PROG_TYPE_CGROUP_DEVICE);
-	if (prog < 0)
+	if (!prog)
 		return log_trace(false, "Failed to allocate new bpf device cgroup program");
 
 	ret = bpf_program_add_instructions(prog, dummy, ARRAY_SIZE(dummy));

src/lxc/commands_utils.c

@@ -62,11 +62,14 @@ int lxc_cmd_sock_get_state(const char *name, const char *lxcpath,
 
 	ret = lxc_cmd_add_state_client(name, lxcpath, states, &state_client_fd);
 	if (ret < 0)
-		return -1;
+		return ret_errno(EINVAL);
 
 	if (ret < MAX_STATE)
 		return ret;
 
+	if (state_client_fd < 0)
+		return ret_errno(EBADF);
+
 	return lxc_cmd_sock_rcv_state(state_client_fd, timeout);
 }
 

src/lxc/conf.c

@@ -3954,7 +3954,7 @@ static struct lxc_list *get_minimal_idmap(const struct lxc_conf *conf,
 	euid = geteuid();
 	if (euid >= container_root_uid->hostid &&
 	    euid < (container_root_uid->hostid + container_root_uid->range))
-		host_uid_map = container_root_uid;
+		host_uid_map = move_ptr(container_root_uid);
 
 	container_root_gid = mapped_nsid_add(conf, nsgid, ID_TYPE_GID);
 	if (!container_root_gid)
@@ -3962,7 +3962,7 @@ static struct lxc_list *get_minimal_idmap(const struct lxc_conf *conf,
 	egid = getegid();
 	if (egid >= container_root_gid->hostid &&
 	    egid < (container_root_gid->hostid + container_root_gid->range))
-		host_gid_map = container_root_gid;
+		host_gid_map = move_ptr(container_root_gid);
 
 	/* Check whether the {g,u}id of the user has a mapping. */
 	if (!host_uid_map)
@@ -3988,7 +3988,7 @@ static struct lxc_list *get_minimal_idmap(const struct lxc_conf *conf,
 	lxc_list_add_elem(tmplist, container_root_uid);
 	lxc_list_add_tail(idmap, tmplist);
 
-	if (host_uid_map != container_root_uid) {
+	if (container_root_uid) {
 		/* idmap will now keep track of that memory. */
 		move_ptr(container_root_uid);
 
@@ -4010,7 +4010,7 @@ static struct lxc_list *get_minimal_idmap(const struct lxc_conf *conf,
 	lxc_list_add_elem(tmplist, container_root_gid);
 	lxc_list_add_tail(idmap, tmplist);
 
-	if (host_gid_map != container_root_gid) {
+	if (container_root_gid) {
 		/* idmap will now keep track of that memory. */
 		move_ptr(container_root_gid);
 

src/lxc/criu.c

@@ -1011,7 +1011,7 @@ static void do_restore(struct lxc_container *c, int status_pipe, struct migrate_
 			}
 
 			if (mount(rootfs->path, rootfs->mount, NULL, MS_BIND, NULL) < 0) {
-				rmdir(rootfs->mount);
+				(void)rmdir(rootfs->mount);
 				goto out_fini_handler;
 			}
 		}
@@ -1046,7 +1046,7 @@ static void do_restore(struct lxc_container *c, int status_pipe, struct migrate_
 		/* exec_criu() returning is an error */
 		exec_criu(cgroup_ops, c->lxc_conf, &os);
 		umount(rootfs->mount);
-		rmdir(rootfs->mount);
+		(void)rmdir(rootfs->mount);
 		goto out_fini_handler;
 	} else {
 		char title[2048];
@@ -1323,7 +1323,7 @@ static bool do_dump(struct lxc_container *c, char *mode, struct migrate_opts *op
 fail:
 	close(criuout[0]);
 	close(criuout[1]);
-	rmdir(opts->directory);
+	(void)rmdir(opts->directory);
 	free(criu_version);
 	return false;
 }

src/lxc/log.c

@@ -485,10 +485,9 @@ static int build_dir(const char *name)
 		*p = '\0';
 
 		ret = lxc_unpriv(mkdir(n, 0755));
+		*p = '/';
 		if (ret && errno != EEXIST)
 			return log_error_errno(-errno, errno, "Failed to create directory \"%s\"", n);
-
-		*p = '/';
 	}
 
 	return 0;

src/lxc/network.c

@@ -2844,6 +2844,9 @@ bool lxc_delete_network_unpriv(struct lxc_handler *handler)
 				      netdev->ifindex, netdev->link);
 
 			ret = netdev_deconf[netdev->type](handler, netdev);
+			if (ret < 0)
+				WARN("Failed to deconfigure interface with index %d and initial name \"%s\"",
+				     netdev->ifindex, netdev->link);
 			goto clear_ifindices;
 		}
 
@@ -3226,6 +3229,9 @@ bool lxc_delete_network_priv(struct lxc_handler *handler)
 			}
 
 			ret = netdev_deconf[netdev->type](handler, netdev);
+			if (ret < 0)
+				WARN("Failed to deconfigure interface with index %d and initial name \"%s\"",
+				     netdev->ifindex, netdev->link);
 			goto clear_ifindices;
 		}
 

src/lxc/rexec.c

@@ -88,7 +88,7 @@ static int is_memfd(void)
 static void lxc_rexec_as_memfd(char **argv, char **envp, const char *memfd_name)
 {
 	__do_close int execfd = -EBADF, fd = -EBADF, memfd = -EBADF,
-				  tmpfd = -EBADF;
+		       tmpfd = -EBADF;
 	int ret;
 	ssize_t bytes_sent = 0;
 	struct stat st = {0};
@@ -143,7 +143,7 @@ static void lxc_rexec_as_memfd(char **argv, char **envp, const char *memfd_name)
 		if (fcntl(memfd, F_ADD_SEALS, LXC_MEMFD_REXEC_SEALS))
 			return;
 
-		execfd = memfd;
+		execfd = move_fd(memfd);
 	} else {
 		char procfd[LXC_PROC_PID_FD_LEN];
 
@@ -169,13 +169,12 @@ extern char **environ;
 
 int lxc_rexec(const char *memfd_name)
 {
+	__do_free_string_list char **argv = NULL;
 	int ret;
-	char **argv = NULL;
 
 	ret = is_memfd();
 	if (ret < 0 && ret == -ENOTRECOVERABLE) {
-		fprintf(stderr,
-			"%s - Failed to determine whether this is a memfd\n",
+		fprintf(stderr, "%s - Failed to determine whether this is a memfd\n",
 			strerror(errno));
 		return -1;
 	} else if (ret > 0) {
@@ -184,8 +183,7 @@ int lxc_rexec(const char *memfd_name)
 
 	ret = parse_argv(&argv);
 	if (ret < 0) {
-		fprintf(stderr,
-			"%s - Failed to parse command line parameters\n",
+		fprintf(stderr, "%s - Failed to parse command line parameters\n",
 			strerror(errno));
 		return -1;
 	}

src/lxc/storage/zfs.c

@@ -159,8 +159,8 @@ bool zfs_detect(const char *path)
 
 int zfs_mount(struct lxc_storage *bdev)
 {
+	__do_free char *mntdata = NULL;
 	unsigned long mntflags = 0;
-	char *mntdata = NULL;
 	int ret;
 	size_t oldlen, newlen, totallen;
 	char *tmp;
@@ -176,7 +176,6 @@ int zfs_mount(struct lxc_storage *bdev)
 	ret = parse_mntopts(bdev->mntopts, &mntflags, &mntdata);
 	if (ret < 0) {
 		ERROR("Failed to parse mount options");
-		free(mntdata);
 		return -22;
 	}
 
@@ -221,7 +220,6 @@ int zfs_mount(struct lxc_storage *bdev)
 	tmp = realloc(mntdata, totallen);
 	if (!tmp) {
 		ERROR("Failed to reallocate memory");
-		free(mntdata);
 		return -1;
 	}
 	mntdata = tmp;
@@ -229,12 +227,10 @@ int zfs_mount(struct lxc_storage *bdev)
 	ret = snprintf((mntdata + oldlen), newlen, ",zfsutil,mntpoint=%s", src);
 	if (ret < 0 || (size_t)ret >= newlen) {
 		ERROR("Failed to create string");
-		free(mntdata);
 		return -1;
 	}
 
 	ret = mount(src, bdev->dest, "zfs", mntflags, mntdata);
-	free(mntdata);
 	if (ret < 0 && errno != EBUSY) {
 		SYSERROR("Failed to mount \"%s\" on \"%s\"", src, bdev->dest);
 		return -1;

src/lxc/uuid.c

@@ -116,7 +116,7 @@ int lxc_id128_write_fd(int fd, lxc_id128_t id)
 
 int lxc_id128_write(const char *p, lxc_id128_t id)
 {
-	int fd = -1;
+	__do_close int fd = -EBADF;
 
         fd = open(p, O_WRONLY|O_CREAT|O_CLOEXEC|O_NOCTTY|O_TRUNC, 0444);
         if (fd < 0)