Merge pull request #3663 from brauner/2021-02-10/fixes

src/lxc/cgroups/cgfsng.c

@@ -2018,8 +2018,8 @@ __cgfsng_ops static bool cgfsng_mount(struct cgroup_ops *ops,
 }
 
 /* Only root needs to escape to the cgroup of its init. */
-__cgfsng_ops static bool cgfsng_escape(const struct cgroup_ops *ops,
-				       struct lxc_conf *conf)
+__cgfsng_ops static bool cgfsng_criu_escape(const struct cgroup_ops *ops,
+					    struct lxc_conf *conf)
 {
 	if (!ops)
 		return ret_set_errno(false, ENOENT);
@@ -2049,7 +2049,7 @@ __cgfsng_ops static bool cgfsng_escape(const struct cgroup_ops *ops,
 	return true;
 }
 
-__cgfsng_ops static int cgfsng_num_hierarchies(struct cgroup_ops *ops)
+__cgfsng_ops static int cgfsng_criu_num_hierarchies(struct cgroup_ops *ops)
 {
 	int i = 0;
 
@@ -2065,8 +2065,8 @@ __cgfsng_ops static int cgfsng_num_hierarchies(struct cgroup_ops *ops)
 	return i;
 }
 
-__cgfsng_ops static bool cgfsng_get_hierarchies(struct cgroup_ops *ops, int n,
-						char ***out)
+__cgfsng_ops static bool cgfsng_criu_get_hierarchies(struct cgroup_ops *ops,
+						     int n, char ***out)
 {
 	int i;
 
@@ -3516,9 +3516,6 @@ struct cgroup_ops *cgfsng_ops_init(struct lxc_conf *conf)
 	cgfsng_ops->payload_create			= cgfsng_payload_create;
 	cgfsng_ops->payload_enter			= cgfsng_payload_enter;
 	cgfsng_ops->payload_finalize			= cgfsng_payload_finalize;
-	cgfsng_ops->escape				= cgfsng_escape;
-	cgfsng_ops->num_hierarchies			= cgfsng_num_hierarchies;
-	cgfsng_ops->get_hierarchies			= cgfsng_get_hierarchies;
 	cgfsng_ops->get_cgroup				= cgfsng_get_cgroup;
 	cgfsng_ops->get					= cgfsng_get;
 	cgfsng_ops->set 				= cgfsng_set;
@@ -3534,6 +3531,10 @@ struct cgroup_ops *cgfsng_ops_init(struct lxc_conf *conf)
 	cgfsng_ops->devices_activate			= cgfsng_devices_activate;
 	cgfsng_ops->get_limiting_cgroup			= cgfsng_get_limiting_cgroup;
 
+	cgfsng_ops->criu_escape				= cgfsng_criu_escape;
+	cgfsng_ops->criu_num_hierarchies		= cgfsng_criu_num_hierarchies;
+	cgfsng_ops->criu_get_hierarchies		= cgfsng_criu_get_hierarchies;
+
 	return move_ptr(cgfsng_ops);
 }
 

src/lxc/cgroups/cgroup.h

@@ -157,9 +157,9 @@ struct cgroup_ops {
 	bool (*payload_create)(struct cgroup_ops *ops, struct lxc_handler *handler);
 	bool (*payload_enter)(struct cgroup_ops *ops, struct lxc_handler *handler);
 	const char *(*get_cgroup)(struct cgroup_ops *ops, const char *controller);
-	bool (*escape)(const struct cgroup_ops *ops, struct lxc_conf *conf);
-	int (*num_hierarchies)(struct cgroup_ops *ops);
-	bool (*get_hierarchies)(struct cgroup_ops *ops, int n, char ***out);
+	bool (*criu_escape)(const struct cgroup_ops *ops, struct lxc_conf *conf);
+	int (*criu_num_hierarchies)(struct cgroup_ops *ops);
+	bool (*criu_get_hierarchies)(struct cgroup_ops *ops, int n, char ***out);
 	int (*set)(struct cgroup_ops *ops, const char *filename,
 		   const char *value, const char *name, const char *lxcpath);
 	int (*get)(struct cgroup_ops *ops, const char *filename, char *value,

src/lxc/conf.c

@@ -911,22 +911,23 @@ static int lxc_setup_ttys(struct lxc_conf *conf)
 	return 0;
 }
 
+define_cleanup_function(struct lxc_tty_info *, lxc_delete_tty);
+
 static int lxc_allocate_ttys(struct lxc_conf *conf)
 {
-	struct lxc_terminal_info *tty_new = NULL;
+	call_cleaner(lxc_delete_tty) struct lxc_tty_info *ttys = &conf->ttys;
 	int ret;
-	struct lxc_tty_info *ttys = &conf->ttys;
 
 	/* no tty in the configuration */
 	if (ttys->max == 0)
 		return 0;
 
-	tty_new = malloc(sizeof(struct lxc_terminal_info) * ttys->max);
-	if (!tty_new)
+	ttys->tty = zalloc(sizeof(struct lxc_terminal_info) * ttys->max);
+	if (!ttys->tty)
 		return -ENOMEM;
 
 	for (size_t i = 0; i < conf->ttys.max; i++) {
-		struct lxc_terminal_info *tty = &tty_new[i];
+		struct lxc_terminal_info *tty = &ttys->tty[i];
 
 		tty->ptx = -EBADF;
 		tty->pty = -EBADF;
@@ -959,7 +960,7 @@ static int lxc_allocate_ttys(struct lxc_conf *conf)
 	}
 
 	INFO("Finished creating %zu tty devices", ttys->max);
-	conf->ttys.tty = move_ptr(tty_new);
+	move_ptr(ttys);
 	return 0;
 }
 

src/lxc/criu.c

@@ -22,6 +22,7 @@
 #include "log.h"
 #include "lxc.h"
 #include "lxclock.h"
+#include "memory_utils.h"
 #include "network.h"
 #include "storage.h"
 #include "syscall_wrappers.h"
@@ -143,18 +144,34 @@ static int cmp_version(const char *v1, const char *v2)
 	return -1;
 }
 
-static void exec_criu(struct cgroup_ops *cgroup_ops, struct lxc_conf *conf,
-		      struct criu_opts *opts)
+struct criu_exec_args {
+	int argc;
+	char *argv[];
+};
+
+static void put_criu_exec_args(struct criu_exec_args *args)
+{
+	if (args) {
+		for (int i = 0; i < args->argc; i++)
+			free_disarm(args->argv[i]);
+		free_disarm(args);
+	}
+}
+
+define_cleanup_function(struct criu_exec_args *, put_criu_exec_args);
+
+static int exec_criu(struct cgroup_ops *cgroup_ops, struct lxc_conf *conf,
+		     struct criu_opts *opts)
 {
-	char **argv, log[PATH_MAX];
-	int static_args = 23, argc = 0, i, ret;
+	call_cleaner(put_criu_exec_args) struct criu_exec_args *args = NULL;
+	__do_fclose FILE *f_mnt = NULL;
+	char log[PATH_MAX];
+	int static_args = 23, ret;
 	int netnr = 0;
 	struct lxc_list *it;
-	FILE *mnts;
 	struct mntent mntent;
 
 	char buf[4096], ttys[32];
-	size_t pos;
 
 	/* If we are currently in a cgroup /foo/bar, and the container is in a
 	 * cgroup /lxc/foo, lxcfs will give us an ENOENT if some task in the
@@ -163,10 +180,8 @@ static void exec_criu(struct cgroup_ops *cgroup_ops, struct lxc_conf *conf,
 	 * /actual/ root cgroup so that lxcfs thinks criu has enough rights to
 	 * see all cgroups.
 	 */
-	if (!cgroup_ops->escape(cgroup_ops, conf)) {
-		ERROR("failed to escape cgroups");
-		return;
-	}
+	if (!cgroup_ops->criu_escape(cgroup_ops, conf))
+		return log_error_errno(-ENOENT, ENOENT, "Failed to escape to root cgroup");
 
 	/* The command line always looks like:
 	 * criu $(action) --tcp-established --file-locks --link-remap \
@@ -212,17 +227,19 @@ static void exec_criu(struct cgroup_ops *cgroup_ops, struct lxc_conf *conf,
 
 		ttys[0] = 0;
 		if (load_tty_major_minor(opts->user->directory, ttys, sizeof(ttys)))
-			return;
+			return log_error_errno(-EINVAL, EINVAL, "Failed to load tty information");
 
 		/* --inherit-fd fd[%d]:tty[%s] */
 		if (ttys[0])
 			static_args += 2;
+
+		static_args += lxc_list_len(&opts->c->lxc_conf->network) * 2;
 	} else {
-		return;
+		return log_error_errno(-EINVAL, EINVAL, "Invalid criu operation specified");
 	}
 
-	if (cgroup_ops->num_hierarchies(cgroup_ops) > 0)
-		static_args += 2 * cgroup_ops->num_hierarchies(cgroup_ops);
+	if (cgroup_ops->criu_num_hierarchies(cgroup_ops) > 0)
+		static_args += 2 * cgroup_ops->criu_num_hierarchies(cgroup_ops);
 
 	if (opts->user->verbose)
 		static_args++;
@@ -233,33 +250,27 @@ static void exec_criu(struct cgroup_ops *cgroup_ops, struct lxc_conf *conf,
 	static_args += 2 * lxc_list_len(&opts->c->lxc_conf->mount_list);
 
 	ret = snprintf(log, PATH_MAX, "%s/%s.log", opts->user->directory, opts->action);
-	if (ret < 0 || ret >= PATH_MAX) {
-		ERROR("logfile name too long");
-		return;
-	}
+	if (ret < 0 || ret >= PATH_MAX)
+		return ret_errno(EIO);
 
-	argv = malloc(static_args * sizeof(*argv));
-	if (!argv)
-		return;
-
-	memset(argv, 0, static_args * sizeof(*argv));
-
-#define DECLARE_ARG(arg) 					\
-	do {							\
-		if (arg == NULL) {				\
-			ERROR("Got NULL argument for criu");	\
-			goto err;				\
-		}						\
-		argv[argc++] = strdup(arg);			\
-		if (!argv[argc-1])				\
-			goto err;				\
+	args = zalloc(sizeof(struct criu_exec_args) + (static_args * sizeof(char **)));
+	if (!args)
+		return log_error_errno(-ENOMEM, ENOMEM, "Failed to allocate static arguments");
+
+#define DECLARE_ARG(arg)                                                                 \
+	do {                                                                             \
+		if (arg == NULL)                                                         \
+			return log_error_errno(-EINVAL, EINVAL,                          \
+					       "Got NULL argument for criu");            \
+		args->argv[(args->argc)++] = strdup(arg);                                \
+		if (!args->argv[args->argc - 1])                                         \
+			return log_error_errno(-ENOMEM, ENOMEM,				 \
+					"Failed to duplicate argumen %s", arg);          \
 	} while (0)
 
-	argv[argc++] = on_path("criu", NULL);
-	if (!argv[argc-1]) {
-		ERROR("Couldn't find criu binary");
-		goto err;
-	}
+	args->argv[(args->argc)++] = on_path("criu", NULL);
+	if (!args->argv[args->argc - 1])
+		return log_error_errno(-ENOENT, ENOENT, "Failed to find criu binary");
 
 	DECLARE_ARG(opts->action);
 	DECLARE_ARG("--tcp-established");
@@ -279,65 +290,52 @@ static void exec_criu(struct cgroup_ops *cgroup_ops, struct lxc_conf *conf,
 	DECLARE_ARG("-o");
 	DECLARE_ARG(log);
 
-	for (i = 0; i < cgroup_ops->num_hierarchies(cgroup_ops); i++) {
-		char **controllers = NULL, *fullname;
-		char *path, *tmp;
+	for (int i = 0; i < cgroup_ops->criu_num_hierarchies(cgroup_ops); i++) {
+		__do_free char *cgroup_base_path = NULL, *controllers;
+		char **controllers_list = NULL;
+		char *tmp;
 
-		if (!cgroup_ops->get_hierarchies(cgroup_ops, i, &controllers)) {
-			ERROR("failed to get hierarchy %d", i);
-			goto err;
-		}
+		if (!cgroup_ops->criu_get_hierarchies(cgroup_ops, i, &controllers_list))
+			return log_error_errno(-ENOENT, ENOENT, "Failed to retrieve cgroup hierarchies %d", i);
 
-		/* if we are in a dump, we have to ask the monitor process what
+		/*
+		 * If we are in a dump, we have to ask the monitor process what
 		 * the right cgroup is. if this is a restore, we can just use
 		 * the handler the restore task created.
 		 */
 		if (!strcmp(opts->action, "dump") || !strcmp(opts->action, "pre-dump")) {
-			path = lxc_cmd_get_limiting_cgroup_path(opts->c->name, opts->c->config_path, controllers[0]);
-			if (!path) {
-				ERROR("failed to get cgroup path for %s", controllers[0]);
-				goto err;
-			}
+			cgroup_base_path = lxc_cmd_get_limiting_cgroup_path(opts->c->name, opts->c->config_path, controllers_list[0]);
+			if (!cgroup_base_path)
+				return log_error_errno(-ENOENT, ENOENT, "Failed to retrieve limiting cgroup path for %s", controllers_list[0] ?: "(null)");
 		} else {
 			const char *p;
 
-			p = cgroup_ops->get_limiting_cgroup(cgroup_ops, controllers[0]);
-			if (!p) {
-				ERROR("failed to get cgroup path for %s", controllers[0]);
-				goto err;
-			}
+			p = cgroup_ops->get_limiting_cgroup(cgroup_ops, controllers_list[0]);
+			if (!p)
+				return log_error_errno(-ENOENT, ENOENT, "Failed to retrieve limiting cgroup path for %s", controllers_list[0] ?: "(null)");
 
-			path = strdup(p);
-			if (!path) {
-				ERROR("strdup failed");
-				goto err;
-			}
+			cgroup_base_path = strdup(p);
+			if (!cgroup_base_path)
+				return log_error_errno(-ENOMEM, ENOMEM, "Failed to duplicate limiting cgroup path");
 		}
 
-		tmp = lxc_deslashify(path);
-		if (!tmp) {
-			ERROR("Failed to remove extraneous slashes from \"%s\"",
-			      path);
-			free(path);
-			goto err;
-		}
-		free(path);
-		path = tmp;
-
-		fullname = lxc_string_join(",", (const char **) controllers, false);
-		if (!fullname) {
-			ERROR("failed to join controllers");
-			free(path);
-			goto err;
-		}
+		tmp = lxc_deslashify(cgroup_base_path);
+		if (!tmp)
+			return log_error_errno(-ENOMEM, ENOMEM, "Failed to remove extraneous slashes from \"%s\"", tmp);
+		free_move_ptr(cgroup_base_path, tmp);
 
-		ret = sprintf(buf, "%s:%s", fullname, path);
-		free(path);
-		free(fullname);
-		if (ret < 0 || ret >= sizeof(buf)) {
-			ERROR("sprintf of cgroup root arg failed");
-			goto err;
+		if (controllers_list[0]) {
+			controllers = lxc_string_join(",", (const char **)controllers_list, false);
+			if (!controllers)
+				return log_error_errno(-ENOMEM, ENOMEM, "Failed to join controllers");
+
+			ret = sprintf(buf, "%s:%s", controllers, cgroup_base_path);
+		} else {
+			WARN("No cgroup controllers configured in container's cgroup %s", cgroup_base_path);
+			ret = sprintf(buf, "%s", cgroup_base_path);
 		}
+		if (ret < 0 || ret >= sizeof(buf))
+			return log_error_errno(-EIO, EIO, "sprintf of cgroup root arg failed");
 
 		DECLARE_ARG("--cgroup-root");
 		DECLARE_ARG(buf);
@@ -351,65 +349,62 @@ static void exec_criu(struct cgroup_ops *cgroup_ops, struct lxc_conf *conf,
 		DECLARE_ARG(opts->user->action_script);
 	}
 
-	mnts = make_anonymous_mount_file(&opts->c->lxc_conf->mount_list,
+	f_mnt = make_anonymous_mount_file(&opts->c->lxc_conf->mount_list,
 	                                 opts->c->lxc_conf->lsm_aa_allow_nesting);
-	if (!mnts)
-		goto err;
+	if (!f_mnt)
+		return log_error_errno(-ENOENT, ENOENT, "Failed to create anonymous mount file");
 
-	while (getmntent_r(mnts, &mntent, buf, sizeof(buf))) {
+	while (getmntent_r(f_mnt, &mntent, buf, sizeof(buf))) {
+		__do_free char *mnt_options = NULL;
 		unsigned long flags = 0;
-		char *mntdata = NULL;
 		char arg[2 * PATH_MAX + 2];
 
-		if (parse_mntopts(mntent.mnt_opts, &flags, &mntdata) < 0)
-			goto err;
-
-		free(mntdata);
+		if (parse_mntopts(mntent.mnt_opts, &flags, &mnt_options) < 0)
+			return log_error_errno(-EINVAL, EINVAL, "Failed to parse mount options");
 
 		/* only add --ext-mount-map for actual bind mounts */
 		if (!(flags & MS_BIND))
 			continue;
 
 		if (strcmp(opts->action, "dump") == 0)
-			ret = snprintf(arg, sizeof(arg), "/%s:%s",
-				       mntent.mnt_dir, mntent.mnt_dir);
+			ret = snprintf(arg, sizeof(arg), "/%s:%s", mntent.mnt_dir, mntent.mnt_dir);
 		else
-			ret = snprintf(arg, sizeof(arg), "%s:%s",
-				       mntent.mnt_dir, mntent.mnt_fsname);
-		if (ret < 0 || ret >= sizeof(arg)) {
-			fclose(mnts);
-			ERROR("snprintf failed");
-			goto err;
-		}
+			ret = snprintf(arg, sizeof(arg), "%s:%s", mntent.mnt_dir, mntent.mnt_fsname);
+		if (ret < 0 || ret >= sizeof(arg))
+			return log_error_errno(-EIO, EIO, "Failed to create mount entry");
 
 		DECLARE_ARG("--ext-mount-map");
 		DECLARE_ARG(arg);
 	}
-	fclose(mnts);
 
 	if (strcmp(opts->action, "dump") == 0 || strcmp(opts->action, "pre-dump") == 0) {
-		char pid[32], *freezer_relative;
+		pid_t init_pid;
+		char init_pid_str[INTTYPE_TO_STRLEN(int)];
+		char *freezer_relative;
+
+		init_pid = opts->c->init_pid(opts->c);
+		if (init_pid < 0)
+			return log_error_errno(-ESRCH, ESRCH, "Failed to retrieve init pid of container");
 
-		if (sprintf(pid, "%d", opts->c->init_pid(opts->c)) < 0)
-			goto err;
+		ret = snprintf(init_pid_str, sizeof(init_pid_str), "%d", init_pid);
+		if (ret < 0 || (size_t)ret >= sizeof(init_pid_str))
+			return log_error_errno(-EIO, EIO, "Failed to create entry for init pid of container");
 
 		DECLARE_ARG("-t");
-		DECLARE_ARG(pid);
+		DECLARE_ARG(init_pid_str);
 
 		freezer_relative = lxc_cmd_get_limiting_cgroup_path(opts->c->name,
 								    opts->c->config_path,
 								    "freezer");
-		if (!freezer_relative) {
-			ERROR("failed getting freezer path");
-			goto err;
-		}
+		if (!freezer_relative)
+			return log_error_errno(-ENOENT, ENOENT, "Failed getting freezer path");
 
 		if (pure_unified_layout(cgroup_ops))
 			ret = snprintf(log, sizeof(log), "/sys/fs/cgroup/%s", freezer_relative);
 		else
 			ret = snprintf(log, sizeof(log), "/sys/fs/cgroup/freezer/%s", freezer_relative);
 		if (ret < 0 || ret >= sizeof(log))
-			goto err;
+			return log_error_errno(-EIO, EIO, "Failed to freezer cgroup entry");
 
 		if (!opts->user->disable_skip_in_flight &&
 				strcmp(opts->criu_version, CRIU_IN_FLIGHT_SUPPORT) >= 0)
@@ -447,10 +442,8 @@ static void exec_criu(struct cgroup_ops *cgroup_ops, struct lxc_conf *conf,
 			char ghost_limit[32];
 
 			ret = sprintf(ghost_limit, "%"PRIu64, opts->user->ghost_limit);
-			if (ret < 0 || ret >= sizeof(ghost_limit)) {
-				ERROR("failed to print ghost limit %"PRIu64, opts->user->ghost_limit);
-				goto err;
-			}
+			if (ret < 0 || ret >= sizeof(ghost_limit))
+				return log_error_errno(-EIO, EIO, "Failed to print ghost limit %"PRIu64, opts->user->ghost_limit);
 
 			DECLARE_ARG("--ghost-limit");
 			DECLARE_ARG(ghost_limit);
@@ -460,8 +453,6 @@ static void exec_criu(struct cgroup_ops *cgroup_ops, struct lxc_conf *conf,
 		if (strcmp(opts->action, "dump") == 0 && !opts->user->stop)
 			DECLARE_ARG("--leave-running");
 	} else if (strcmp(opts->action, "restore") == 0) {
-		void *m;
-		int additional;
 		struct lxc_conf *lxc_conf = opts->c->lxc_conf;
 
 		DECLARE_ARG("--root");
@@ -470,22 +461,20 @@ static void exec_criu(struct cgroup_ops *cgroup_ops, struct lxc_conf *conf,
 		DECLARE_ARG("--restore-sibling");
 
 		if (ttys[0]) {
-			if (opts->console_fd < 0) {
-				ERROR("lxc.console.path configured on source host but not target");
-				goto err;
-			}
+			if (opts->console_fd < 0)
+				return log_error_errno(-EINVAL, EINVAL, "lxc.console.path configured on source host but not target");
 
 			ret = snprintf(buf, sizeof(buf), "fd[%d]:%s", opts->console_fd, ttys);
 			if (ret < 0 || ret >= sizeof(buf))
-				goto err;
+				return log_error_errno(-EIO, EIO, "Failed to create console entry");
 
 			DECLARE_ARG("--inherit-fd");
 			DECLARE_ARG(buf);
 		}
 		if (opts->console_name) {
-			if (snprintf(buf, sizeof(buf), "console:%s", opts->console_name) < 0) {
-				SYSERROR("sprintf'd too many bytes");
-			}
+			if (snprintf(buf, sizeof(buf), "console:%s", opts->console_name) < 0)
+				return log_error_errno(-EIO, EIO, "Failed to create console entry");
+
 			DECLARE_ARG("--ext-mount-map");
 			DECLARE_ARG(buf);
 		}
@@ -496,21 +485,13 @@ static void exec_criu(struct cgroup_ops *cgroup_ops, struct lxc_conf *conf,
 				ret = snprintf(buf, sizeof(buf), "apparmor:%s", lxc_conf->lsm_aa_profile);
 			else
 				ret = snprintf(buf, sizeof(buf), "selinux:%s", lxc_conf->lsm_se_context);
-
 			if (ret < 0 || ret >= sizeof(buf))
-				goto err;
+				return log_error_errno(-EIO, EIO, "Failed to create lsm entry");
 
 			DECLARE_ARG("--lsm-profile");
 			DECLARE_ARG(buf);
 		}
 
-		additional = lxc_list_len(&opts->c->lxc_conf->network) * 2;
-
-		m = realloc(argv, (argc + additional + 1) * sizeof(*argv));
-		if (!m)
-			goto err;
-		argv = m;
-
 		lxc_list_for_each(it, &opts->c->lxc_conf->network) {
 			size_t retlen;
 			char eth[128], *veth;
@@ -530,11 +511,11 @@ static void exec_criu(struct cgroup_ops *cgroup_ops, struct lxc_conf *conf,
 			if (n->name[0] != '\0') {
 				retlen = strlcpy(eth, n->name, sizeof(eth));
 				if (retlen >= sizeof(eth))
-					goto err;
+					return log_error_errno(-E2BIG, E2BIG, "Failed to append veth device name");
 			} else {
 				ret = snprintf(eth, sizeof(eth), "eth%d", netnr);
 				if (ret < 0 || ret >= sizeof(eth))
-					goto err;
+					return log_error_errno(-E2BIG, E2BIG, "Failed to append veth device name");
 			}
 
 			switch (n->type) {
@@ -545,44 +526,37 @@ static void exec_criu(struct cgroup_ops *cgroup_ops, struct lxc_conf *conf,
 
 				if (n->link[0] != '\0') {
 					if (external_not_veth)
-						ret = snprintf(buf, sizeof(buf),
-							       "veth[%s]:%s@%s",
-							       eth, veth,
-							       n->link);
+						ret = snprintf(buf, sizeof(buf), "veth[%s]:%s@%s", eth, veth, n->link);
 					else
-						ret = snprintf(buf, sizeof(buf),
-							       "%s=%s@%s", eth,
-							       veth, n->link);
+						ret = snprintf(buf, sizeof(buf), "%s=%s@%s", eth, veth, n->link);
 				} else {
 					if (external_not_veth)
-						ret = snprintf(buf, sizeof(buf),
-							       "veth[%s]:%s",
-							       eth, veth);
+						ret = snprintf(buf, sizeof(buf), "veth[%s]:%s", eth, veth);
 					else
-						ret = snprintf(buf, sizeof(buf),
-							       "%s=%s", eth,
-							       veth);
+						ret = snprintf(buf, sizeof(buf), "%s=%s", eth, veth);
 				}
 				if (ret < 0 || ret >= sizeof(buf))
-					goto err;
+					return log_error_errno(-EIO, EIO, "Failed to append veth device name");
+
+				TRACE("Added veth device entry %s", buf);
 				break;
 			case LXC_NET_MACVLAN:
-				if (n->link[0] == '\0') {
-					ERROR("no host interface for macvlan %s", n->name);
-					goto err;
-				}
+				if (n->link[0] == '\0')
+					return log_error_errno(-EINVAL, EINVAL, "Failed to find host interface for macvlan %s", n->name);
 
 				ret = snprintf(buf, sizeof(buf), "macvlan[%s]:%s", eth, n->link);
 				if (ret < 0 || ret >= sizeof(buf))
-					goto err;
+					return log_error_errno(-EIO, EIO, "Failed to add macvlan entry");
+
+				TRACE("Added macvlan device entry %s", buf);
+
 				break;
 			case LXC_NET_NONE:
 			case LXC_NET_EMPTY:
 				break;
 			default:
 				/* we have screened for this earlier... */
-				ERROR("unexpected network type %d", n->type);
-				goto err;
+				return log_error_errno(-EINVAL, EINVAL, "Unsupported network type %d", n->type);
 			}
 
 			if (external_not_veth)
@@ -595,42 +569,35 @@ static void exec_criu(struct cgroup_ops *cgroup_ops, struct lxc_conf *conf,
 
 	}
 
-	argv[argc] = NULL;
+	args->argv[args->argc] = NULL;
 
-	buf[0] = 0;
-	pos = 0;
+	if (lxc_log_trace()) {
+		buf[0] = 0;
+		for (int i = 0, pos = 0; i < args->argc && args->argv[i]; i++) {
+			ret = snprintf(buf + pos, sizeof(buf) - pos, "%s ", args->argv[i]);
+			if (ret < 0 || ret >= sizeof(buf) - pos)
+				return log_error_errno(-EIO, EIO, "Failed to reorder entries");
+			else
+				pos += ret;
+		}
 
-	for (i = 0; argv[i]; i++) {
-		ret = snprintf(buf + pos, sizeof(buf) - pos, "%s ", argv[i]);
-		if (ret < 0 || ret >= sizeof(buf) - pos)
-			goto err;
-		else
-			pos += ret;
+		TRACE("Using command line %s", buf);
 	}
 
-	INFO("execing: %s", buf);
-
 	/* before criu inits its log, it sometimes prints things to stdout/err;
 	 * let's be sure we capture that.
 	 */
-	if (dup2(opts->pipefd, STDOUT_FILENO) < 0) {
-		SYSERROR("dup2 stdout failed");
-		goto err;
-	}
+	if (dup2(opts->pipefd, STDOUT_FILENO) < 0)
+		return log_error_errno(-errno, errno, "Failed to duplicate stdout");
 
-	if (dup2(opts->pipefd, STDERR_FILENO) < 0) {
-		SYSERROR("dup2 stderr failed");
-		goto err;
-	}
+	if (dup2(opts->pipefd, STDERR_FILENO) < 0)
+		return log_error_errno(-errno, errno, "Failed to duplicate stderr");
 
 	close(opts->pipefd);
 
 #undef DECLARE_ARG
-	execv(argv[0], argv);
-err:
-	for (i = 0; argv[i]; i++)
-		free(argv[i]);
-	free(argv);
+	execv(args->argv[0], args->argv);
+	return -ENOEXEC;
 }
 
 /*
@@ -941,14 +908,25 @@ static void do_restore(struct lxc_container *c, int status_pipe, struct migrate_
 
 	if (lxc_init(c->name, handler) < 0)
 		goto out;
+	cgroup_ops = handler->cgroup_ops;
 
-	cgroup_ops = cgroup_init(c->lxc_conf);
-	if (!cgroup_ops)
+	if (!cgroup_ops->monitor_create(cgroup_ops, handler)) {
+		ERROR("Failed to create monitor cgroup");
 		goto out_fini_handler;
-	handler->cgroup_ops = cgroup_ops;
+	}
+
+	if (!cgroup_ops->monitor_enter(cgroup_ops, handler)) {
+		ERROR("Failed to enter monitor cgroup");
+		goto out_fini_handler;
+	}
+
+	if (!cgroup_ops->monitor_delegate_controllers(cgroup_ops)) {
+		ERROR("Failed to delegate controllers to monitor cgroup");
+		goto out_fini_handler;
+	}
 
 	if (!cgroup_ops->payload_create(cgroup_ops, handler)) {
-		ERROR("failed creating groups");
+		ERROR("Failed creating cgroups");
 		goto out_fini_handler;
 	}
 
@@ -1037,7 +1015,9 @@ static void do_restore(struct lxc_container *c, int status_pipe, struct migrate_
 		os.console_name = c->lxc_conf->console.name;
 
 		/* exec_criu() returning is an error */
-		exec_criu(cgroup_ops, c->lxc_conf, &os);
+		ret = exec_criu(handler->cgroup_ops, c->lxc_conf, &os);
+		if (ret)
+			SYSERROR("Failed to execute criu");
 		umount(rootfs->mount);
 		(void)rmdir(rootfs->mount);
 		goto out_fini_handler;
@@ -1262,7 +1242,9 @@ static bool do_dump(struct lxc_container *c, char *mode, struct migrate_opts *op
 		}
 
 		/* exec_criu() returning is an error */
-		exec_criu(cgroup_ops, c->lxc_conf, &os);
+		ret = exec_criu(cgroup_ops, c->lxc_conf, &os);
+		if (ret)
+			SYSERROR("Failed to execute criu");
 		free(criu_version);
 		_exit(EXIT_FAILURE);
 	} else {

src/lxc/macro.h

@@ -644,12 +644,6 @@ enum {
 		-(__errno__);        \
 	})
 
-#define free_move_ptr(a, b)          \
-	({                           \
-		free(a);             \
-		(a) = move_ptr((b)); \
-	})
-
 /* Container's specific file/directory names */
 #define LXC_CONFIG_FNAME      "config"
 #define LXC_PARTIAL_FNAME     "partial"

src/lxc/memory_utils.h

@@ -83,4 +83,10 @@ static inline void *memdup(const void *data, size_t len)
 
 #define zalloc(__size__) (calloc(1, __size__))
 
+#define free_move_ptr(a, b)          \
+	({                           \
+		free(a);             \
+		(a) = move_ptr((b)); \
+	})
+
 #endif /* __LXC_MEMORY_UTILS_H */