From 59d8a539d106ba17e54f75a92c1278907c55bb56 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Wed, 10 Feb 2021 12:27:26 +0100 Subject: [PATCH] criu: massage exec_criu() Signed-off-by: Christian Brauner --- src/lxc/criu.c | 272 ++++++++++++++++++----------------------- src/lxc/macro.h | 6 - src/lxc/memory_utils.h | 6 + 3 files changed, 123 insertions(+), 161 deletions(-) diff --git a/src/lxc/criu.c b/src/lxc/criu.c index 0f954a256..0b5920a75 100644 --- a/src/lxc/criu.c +++ b/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,11 +144,28 @@ 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) { - char **argv, log[PATH_MAX]; - int static_args = 23, argc = 0, i, ret; + 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) +{ + call_cleaner(put_criu_exec_args) struct criu_exec_args *args = NULL; + char log[PATH_MAX]; + int static_args = 23, ret; int netnr = 0; struct lxc_list *it; FILE *mnts; @@ -163,10 +181,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->criu_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,13 +228,15 @@ 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->criu_num_hierarchies(cgroup_ops) > 0) @@ -233,33 +251,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 +291,47 @@ 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->criu_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->criu_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; - } + 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); + 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); @@ -354,7 +348,7 @@ static void exec_criu(struct cgroup_ops *cgroup_ops, struct lxc_conf *conf, mnts = make_anonymous_mount_file(&opts->c->lxc_conf->mount_list, opts->c->lxc_conf->lsm_aa_allow_nesting); if (!mnts) - goto err; + return log_error_errno(-ENOENT, ENOENT, "Failed to create anonymous mount file"); while (getmntent_r(mnts, &mntent, buf, sizeof(buf))) { unsigned long flags = 0; @@ -362,7 +356,7 @@ static void exec_criu(struct cgroup_ops *cgroup_ops, struct lxc_conf *conf, char arg[2 * PATH_MAX + 2]; if (parse_mntopts(mntent.mnt_opts, &flags, &mntdata) < 0) - goto err; + return log_error_errno(-EINVAL, EINVAL, "Failed to parse mount options"); free(mntdata); @@ -378,8 +372,7 @@ static void exec_criu(struct cgroup_ops *cgroup_ops, struct lxc_conf *conf, mntent.mnt_dir, mntent.mnt_fsname); if (ret < 0 || ret >= sizeof(arg)) { fclose(mnts); - ERROR("snprintf failed"); - goto err; + return log_error_errno(-EIO, EIO, "Failed to create mount entry"); } DECLARE_ARG("--ext-mount-map"); @@ -391,7 +384,7 @@ static void exec_criu(struct cgroup_ops *cgroup_ops, struct lxc_conf *conf, char pid[32], *freezer_relative; if (sprintf(pid, "%d", opts->c->init_pid(opts->c)) < 0) - goto err; + return log_error_errno(-EIO, EIO, "Failed to create init pid entry"); DECLARE_ARG("-t"); DECLARE_ARG(pid); @@ -399,17 +392,15 @@ static void exec_criu(struct cgroup_ops *cgroup_ops, struct lxc_conf *conf, 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 +438,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 +449,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 +457,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 +481,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 +507,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 +522,32 @@ 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"); 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"); 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,15 +560,15 @@ 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; - for (i = 0; argv[i]; i++) { - ret = snprintf(buf + pos, sizeof(buf) - pos, "%s ", argv[i]); + for (int i = 0; args->argc; i++) { + ret = snprintf(buf + pos, sizeof(buf) - pos, "%s ", args->argv[i]); if (ret < 0 || ret >= sizeof(buf) - pos) - goto err; + return log_error_errno(-EIO, EIO, "Failed to reorder entries"); else pos += ret; } @@ -613,24 +578,17 @@ static void exec_criu(struct cgroup_ops *cgroup_ops, struct lxc_conf *conf, /* 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; } /* @@ -1037,7 +995,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(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 +1222,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 { diff --git a/src/lxc/macro.h b/src/lxc/macro.h index bb8be340f..41873dd9a 100644 --- a/src/lxc/macro.h +++ b/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" diff --git a/src/lxc/memory_utils.h b/src/lxc/memory_utils.h index a7a9afcf3..bcf3b67ae 100644 --- a/src/lxc/memory_utils.h +++ b/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 */ -- 2.47.2