The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/lxc/pull/3279
This e-mail was sent by the LXC bot, direct replies will not reach the author unless they happen to be subscribed to this list. === Description (from pull-request) ===
From 55171a21aff5db21b5307b4732739e05bff82eb8 Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Wed, 4 Mar 2020 15:21:18 +0100 Subject: [PATCH 01/11] af_unix: cleanup Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- src/lxc/af_unix.c | 124 +++++++++++++++++----------------------------- 1 file changed, 46 insertions(+), 78 deletions(-) diff --git a/src/lxc/af_unix.c b/src/lxc/af_unix.c index 7f8c88b1a2..12fb1679d3 100644 --- a/src/lxc/af_unix.c +++ b/src/lxc/af_unix.c @@ -16,6 +16,7 @@ #include "config.h" #include "log.h" +#include "macro.h" #include "memory_utils.h" #include "raw_syscalls.h" #include "utils.h" @@ -27,14 +28,12 @@ lxc_log_define(af_unix, lxc); static ssize_t lxc_abstract_unix_set_sockaddr(struct sockaddr_un *addr, - const char *path) + const char *path) { size_t len; - if (!addr || !path) { - errno = EINVAL; - return -1; - } + if (!addr || !path) + return ret_errno(EINVAL); /* Clear address structure */ memset(addr, 0, sizeof(*addr)); @@ -44,10 +43,8 @@ static ssize_t lxc_abstract_unix_set_sockaddr(struct sockaddr_un *addr, len = strlen(&path[1]); /* do not enforce \0-termination */ - if (len >= INT_MAX || len >= sizeof(addr->sun_path)) { - errno = ENAMETOOLONG; - return -1; - } + if (len >= INT_MAX || len >= sizeof(addr->sun_path)) + return ret_errno(ENAMETOOLONG); /* do not enforce \0-termination */ memcpy(&addr->sun_path[1], &path[1], len); @@ -56,7 +53,8 @@ static ssize_t lxc_abstract_unix_set_sockaddr(struct sockaddr_un *addr, int lxc_abstract_unix_open(const char *path, int type, int flags) { - int fd, ret; + __do_close_prot_errno int fd = -EBADF; + int ret; ssize_t len; struct sockaddr_un addr; @@ -65,36 +63,24 @@ int lxc_abstract_unix_open(const char *path, int type, int flags) return -1; if (!path) - return fd; + return move_fd(fd); len = lxc_abstract_unix_set_sockaddr(&addr, path); - if (len < 0) { - int saved_errno = errno; - close(fd); - errno = saved_errno; + if (len < 0) return -1; - } ret = bind(fd, (struct sockaddr *)&addr, offsetof(struct sockaddr_un, sun_path) + len + 1); - if (ret < 0) { - int saved_errno = errno; - close(fd); - errno = saved_errno; + if (ret < 0) return -1; - } if (type == SOCK_STREAM) { ret = listen(fd, 100); - if (ret < 0) { - int saved_errno = errno; - close(fd); - errno = saved_errno; + if (ret < 0) return -1; - } } - return fd; + return move_fd(fd); } void lxc_abstract_unix_close(int fd) @@ -104,7 +90,8 @@ void lxc_abstract_unix_close(int fd) int lxc_abstract_unix_connect(const char *path) { - int fd, ret; + __do_close_prot_errno int fd = -EBADF; + int ret; ssize_t len; struct sockaddr_un addr; @@ -113,23 +100,15 @@ int lxc_abstract_unix_connect(const char *path) return -1; len = lxc_abstract_unix_set_sockaddr(&addr, path); - if (len < 0) { - int saved_errno = errno; - close(fd); - errno = saved_errno; + if (len < 0) return -1; - } ret = connect(fd, (struct sockaddr *)&addr, offsetof(struct sockaddr_un, sun_path) + len + 1); - if (ret < 0) { - int saved_errno = errno; - close(fd); - errno = saved_errno; + if (ret < 0) return -1; - } - return fd; + return move_fd(fd); } int lxc_abstract_unix_send_fds_iov(int fd, int *sendfds, int num_sendfds, @@ -164,11 +143,9 @@ int lxc_abstract_unix_send_fds_iov(int fd, int *sendfds, int num_sendfds, msg.msg_iov = iov; msg.msg_iovlen = iovlen; -again: - ret = sendmsg(fd, &msg, MSG_NOSIGNAL); - if (ret < 0) - if (errno == EINTR) - goto again; + do { + ret = sendmsg(fd, &msg, MSG_NOSIGNAL); + } while (ret < 0 && errno == EINTR); return ret; } @@ -181,8 +158,7 @@ int lxc_abstract_unix_send_fds(int fd, int *sendfds, int num_sendfds, .iov_base = data ? data : buf, .iov_len = data ? size : sizeof(buf), }; - return lxc_abstract_unix_send_fds_iov(fd, sendfds, num_sendfds, &iov, - 1); + return lxc_abstract_unix_send_fds_iov(fd, sendfds, num_sendfds, &iov, 1); } int lxc_unix_send_fds(int fd, int *sendfds, int num_sendfds, void *data, @@ -197,17 +173,14 @@ static int lxc_abstract_unix_recv_fds_iov(int fd, int *recvfds, int num_recvfds, __do_free char *cmsgbuf = NULL; int ret; struct msghdr msg; - struct cmsghdr *cmsg = NULL; size_t cmsgbufsize = CMSG_SPACE(sizeof(struct ucred)) + CMSG_SPACE(num_recvfds * sizeof(int)); memset(&msg, 0, sizeof(msg)); cmsgbuf = malloc(cmsgbufsize); - if (!cmsgbuf) { - errno = ENOMEM; - return -1; - } + if (!cmsgbuf) + return ret_errno(ENOMEM); msg.msg_control = cmsgbuf; msg.msg_controllen = cmsgbufsize; @@ -216,20 +189,18 @@ static int lxc_abstract_unix_recv_fds_iov(int fd, int *recvfds, int num_recvfds, msg.msg_iovlen = iovlen; again: - ret = recvmsg(fd, &msg, 0); - if (ret < 0) { - if (errno == EINTR) - goto again; + do { + ret = recvmsg(fd, &msg, 0); + } while (ret < 0 && errno == EINTR); + + if (!ret) + return 0; - goto out; - } - if (ret == 0) - goto out; /* * If SO_PASSCRED is set we will always get a ucred message. */ - for (cmsg = CMSG_FIRSTHDR(&msg); cmsg; cmsg = CMSG_NXTHDR(&msg, cmsg)) { + for (struct cmsghdr *cmsg = CMSG_FIRSTHDR(&msg); cmsg; cmsg = CMSG_NXTHDR(&msg, cmsg)) { if (cmsg->cmsg_type != SCM_RIGHTS) continue; @@ -241,7 +212,6 @@ static int lxc_abstract_unix_recv_fds_iov(int fd, int *recvfds, int num_recvfds, break; } -out: return ret; } @@ -262,7 +232,9 @@ int lxc_abstract_unix_send_credential(int fd, void *data, size_t size) struct iovec iov; struct cmsghdr *cmsg; struct ucred cred = { - .pid = lxc_raw_getpid(), .uid = getuid(), .gid = getgid(), + .pid = lxc_raw_getpid(), + .uid = getuid(), + .gid = getgid(), }; char cmsgbuf[CMSG_SPACE(sizeof(cred))] = {0}; char buf[1] = {0}; @@ -309,7 +281,7 @@ int lxc_abstract_unix_rcv_credential(int fd, void *data, size_t size) ret = recvmsg(fd, &msg, 0); if (ret <= 0) - goto out; + return ret; cmsg = CMSG_FIRSTHDR(&msg); @@ -317,15 +289,13 @@ int lxc_abstract_unix_rcv_credential(int fd, void *data, size_t size) cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SCM_CREDENTIALS) { memcpy(&cred, CMSG_DATA(cmsg), sizeof(cred)); - if (cred.uid && - (cred.uid != getuid() || cred.gid != getgid())) { - INFO("Message denied for '%d/%d'", cred.uid, cred.gid); - errno = EACCES; - return -1; - } + + if (cred.uid && (cred.uid != getuid() || cred.gid != getgid())) + return log_error_errno(-1, EACCES, + "Message denied for '%d/%d'", + cred.uid, cred.gid); } -out: return ret; } @@ -364,10 +334,9 @@ int lxc_unix_connect_type(struct sockaddr_un *addr, int type) ssize_t len; fd = socket(AF_UNIX, type | SOCK_CLOEXEC, 0); - if (fd < 0) { - SYSERROR("Failed to open new AF_UNIX socket"); - return -1; - } + if (fd < 0) + return log_error_errno(-1, errno, + "Failed to open new AF_UNIX socket"); if (addr->sun_path[0] == '\0') len = strlen(&addr->sun_path[1]); @@ -376,10 +345,9 @@ int lxc_unix_connect_type(struct sockaddr_un *addr, int type) ret = connect(fd, (struct sockaddr *)addr, offsetof(struct sockaddr_un, sun_path) + len); - if (ret < 0) { - SYSERROR("Failed to bind new AF_UNIX socket"); - return -1; - } + if (ret < 0) + return log_error_errno(-1, errno, + "Failed to bind new AF_UNIX socket"); return move_fd(fd); } From b714b9f29b03e850be1cf0ca06c7337599846701 Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Thu, 5 Mar 2020 10:02:12 +0100 Subject: [PATCH 02/11] api-extensions: document cgroup2_devices and cgroup2 api extensions Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- doc/api-extensions.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/doc/api-extensions.md b/doc/api-extensions.md index 75681a33cd..da7aefa2e5 100644 --- a/doc/api-extensions.md +++ b/doc/api-extensions.md @@ -105,3 +105,16 @@ This introduces the ability to specify a `lxc.net.[i].veth.mode` setting, which In "router" mode static routes are created on the host for the container's IP addresses pointing to the host side veth interface. In addition to the routes, a static IP neighbour proxy is added to the host side veth interface for the IPv4 and IPv6 gateway IPs. + + +# cgroup2\_devices + +This enables `LXC` to make use of the new devices controller in the unified +cgroup hierarchy. `LXC` will now create, load, and attach bpf program to the +cgroup of the container when the controller is available. + +# cgroup2 + +This enables `LXC` to make complete use of the unified cgroup hierarchy. With +this extension it is possible to run `LXC` containers on systems that use +a pure unified cgroup layout. From 65a3fd2b2620b3f5b5d8bb744988a7cc6a0a22d1 Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Thu, 5 Mar 2020 10:03:38 +0100 Subject: [PATCH 03/11] attach: cleanup Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- src/lxc/attach.c | 95 ++++++++++++++++++------------------------------ src/lxc/log.h | 6 +++ 2 files changed, 42 insertions(+), 59 deletions(-) diff --git a/src/lxc/attach.c b/src/lxc/attach.c index 7b07dce440..e9030e3482 100644 --- a/src/lxc/attach.c +++ b/src/lxc/attach.c @@ -58,10 +58,10 @@ static struct lxc_proc_context_info *lxc_proc_get_context_info(pid_t pid) { __do_free char *line = NULL; __do_fclose FILE *proc_file = NULL; + __do_free struct lxc_proc_context_info *info = NULL; int ret; bool found; char proc_fn[LXC_PROC_STATUS_LEN]; - struct lxc_proc_context_info *info; size_t line_bufsz = 0; /* Read capabilities. */ @@ -69,11 +69,9 @@ static struct lxc_proc_context_info *lxc_proc_get_context_info(pid_t pid) if (ret < 0 || ret >= LXC_PROC_STATUS_LEN) return NULL; - proc_file = fopen(proc_fn, "r"); - if (!proc_file) { - SYSERROR("Failed to open %s", proc_fn); - return NULL; - } + proc_file = fopen(proc_fn, "re"); + if (!proc_file) + return log_error_errno(NULL, errno, "Failed to open %s", proc_fn); info = calloc(1, sizeof(*info)); if (!info) @@ -89,17 +87,14 @@ static struct lxc_proc_context_info *lxc_proc_get_context_info(pid_t pid) } } - if (!found) { - ERROR("Could not read capability bounding set from %s", proc_fn); - free(info); - return NULL; - } + if (!found) + return log_error_errno(NULL, ENOENT, "Failed to read capability bounding set from %s", proc_fn); info->lsm_label = lsm_process_label_get(pid); info->ns_inherited = 0; memset(info->ns_fd, -1, sizeof(int) * LXC_NS_MAX); - return info; + return move_ptr(info); } static inline void lxc_proc_close_ns_fd(struct lxc_proc_context_info *ctx) @@ -172,18 +167,17 @@ static int in_same_namespace(pid_t pid1, pid_t pid2, const char *ns) static int lxc_attach_to_ns(pid_t pid, struct lxc_proc_context_info *ctx) { - int i, ret; + for (int i = 0; i < LXC_NS_MAX; i++) { + int ret; - for (i = 0; i < LXC_NS_MAX; i++) { if (ctx->ns_fd[i] < 0) continue; ret = setns(ctx->ns_fd[i], ns_info[i].clone_flag); - if (ret < 0) { - SYSERROR("Failed to attach to %s namespace of %d", - ns_info[i].proc_name, pid); - return -1; - } + if (ret < 0) + return log_error_errno(-1, + errno, "Failed to attach to %s namespace of %d", + ns_info[i].proc_name, pid); DEBUG("Attached to %s namespace of %d", ns_info[i].proc_name, pid); } @@ -196,10 +190,8 @@ int lxc_attach_remount_sys_proc(void) int ret; ret = unshare(CLONE_NEWNS); - if (ret < 0) { - SYSERROR("Failed to unshare mount namespace"); - return -1; - } + if (ret < 0) + return log_error_errno(-1, errno, "Failed to unshare mount namespace"); if (detect_shared_rootfs()) { if (mount(NULL, "/", NULL, MS_SLAVE | MS_REC, NULL)) { @@ -210,50 +202,40 @@ int lxc_attach_remount_sys_proc(void) /* Assume /proc is always mounted, so remount it. */ ret = umount2("/proc", MNT_DETACH); - if (ret < 0) { - SYSERROR("Failed to unmount /proc"); - return -1; - } + if (ret < 0) + return log_error_errno(-1, errno, "Failed to unmount /proc"); ret = mount("none", "/proc", "proc", 0, NULL); - if (ret < 0) { - SYSERROR("Failed to remount /proc"); - return -1; - } + if (ret < 0) + return log_error_errno(-1, errno, "Failed to remount /proc"); - /* Try to umount /sys. If it's not a mount point, we'll get EINVAL, then + /* + * Try to umount /sys. If it's not a mount point, we'll get EINVAL, then * we ignore it because it may not have been mounted in the first place. */ ret = umount2("/sys", MNT_DETACH); - if (ret < 0 && errno != EINVAL) { - SYSERROR("Failed to unmount /sys"); - return -1; - } else if (ret == 0) { - /* Remount it. */ - ret = mount("none", "/sys", "sysfs", 0, NULL); - if (ret < 0) { - SYSERROR("Failed to remount /sys"); - return -1; - } - } + if (ret < 0 && errno != EINVAL) + return log_error_errno(-1, errno, "Failed to unmount /sys"); + + /* Remount it. */ + if (ret == 0 && mount("none", "/sys", "sysfs", 0, NULL)) + return log_error_errno(-1, errno, "Failed to remount /sys"); return 0; } static int lxc_attach_drop_privs(struct lxc_proc_context_info *ctx) { - int cap, last_cap; + int last_cap; last_cap = lxc_caps_last_cap(); - for (cap = 0; cap <= last_cap; cap++) { + for (int cap = 0; cap <= last_cap; cap++) { if (ctx->capability_mask & (1LL << cap)) continue; if (prctl(PR_CAPBSET_DROP, prctl_arg(cap), prctl_arg(0), - prctl_arg(0), prctl_arg(0))) { - SYSERROR("Failed to drop capability %d", cap); - return -1; - } + prctl_arg(0), prctl_arg(0))) + return log_error_errno(-1, errno, "Failed to drop capability %d", cap); TRACE("Dropped capability %d", cap); } @@ -310,8 +292,7 @@ static int lxc_attach_set_environment(struct lxc_proc_context_info *init_ctx, free(extra_keep_store); } - ERROR("Failed to clear environment"); - return -1; + return log_error(-1, "Failed to clear environment"); } if (extra_keep_store) { @@ -343,10 +324,8 @@ static int lxc_attach_set_environment(struct lxc_proc_context_info *init_ctx, } ret = putenv("container=lxc"); - if (ret < 0) { - SYSWARN("Failed to set environment variable"); - return -1; - } + if (ret < 0) + return log_warn(-1, errno, "Failed to set environment variable"); /* Set container environment variables.*/ if (init_ctx && init_ctx->container && init_ctx->container->lxc_conf) { @@ -358,10 +337,8 @@ static int lxc_attach_set_environment(struct lxc_proc_context_info *init_ctx, return -1; ret = putenv(env_tmp); - if (ret < 0) { - SYSERROR("Failed to set environment variable: %s", (char *)iterator->elem); - return -1; - } + if (ret < 0) + return log_error_errno(-1, errno, "Failed to set environment variable: %s", (char *)iterator->elem); } } diff --git a/src/lxc/log.h b/src/lxc/log.h index db4129493d..545626e46f 100644 --- a/src/lxc/log.h +++ b/src/lxc/log.h @@ -510,6 +510,12 @@ __lxc_unused static inline void LXC_##LEVEL(struct lxc_log_locinfo* locinfo, \ __ret__; \ }) +#define log_warn(__ret__, __errno__, format, ...) \ + ({ \ + WARN(format, ##__VA_ARGS__); \ + __ret__; \ + }) + #define log_debug_errno(__ret__, __errno__, format, ...) \ ({ \ errno = __errno__; \ From a17caf07fa4f7abbd0eb55ebe6d6f94049b06e1e Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Mon, 9 Mar 2020 10:05:25 +0100 Subject: [PATCH 04/11] attach: fix fd leak Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- src/lxc/attach.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/lxc/attach.c b/src/lxc/attach.c index e9030e3482..4930397b4c 100644 --- a/src/lxc/attach.c +++ b/src/lxc/attach.c @@ -431,6 +431,13 @@ static char *lxc_attach_getpwshell(uid_t uid) close(pipes[1]); pipe_f = fdopen(pipes[0], "r"); + if (!pipe_f) { + close(pipes[0]); + goto reap_child; + } + /* Transfer ownership of pipes[0] to pipe_f. */ + move_fd(pipes[0]); + while (getline(&line, &line_bufsz, pipe_f) != -1) { int i; long value; @@ -493,6 +500,7 @@ static char *lxc_attach_getpwshell(uid_t uid) found = true; } +reap_child: ret = wait_for_pid(pid); if (ret < 0) { free(result); From b35ffe6898cdbe4b87f5b4fffed137bb1d227f6d Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Mon, 9 Mar 2020 10:06:50 +0100 Subject: [PATCH 05/11] attach: use cleanup macros in lxc_attach_getpwshell() Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- src/lxc/attach.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/lxc/attach.c b/src/lxc/attach.c index 4930397b4c..267b948286 100644 --- a/src/lxc/attach.c +++ b/src/lxc/attach.c @@ -365,14 +365,13 @@ static int lxc_attach_set_environment(struct lxc_proc_context_info *init_ctx, static char *lxc_attach_getpwshell(uid_t uid) { - __do_free char *line = NULL; + __do_free char *line = NULL, *result = NULL; __do_fclose FILE *pipe_f = NULL; int fd, ret; pid_t pid; int pipes[2]; bool found = false; size_t line_bufsz = 0; - char *result = NULL; /* We need to fork off a process that runs the getent program, and we * need to capture its output, so we use a pipe for that purpose. @@ -489,7 +488,7 @@ static char *lxc_attach_getpwshell(uid_t uid) if (!token) continue; - free(result); + free_disarm(result); result = strdup(token); /* Sanity check that there are no fields after that. */ @@ -502,17 +501,13 @@ static char *lxc_attach_getpwshell(uid_t uid) reap_child: ret = wait_for_pid(pid); - if (ret < 0) { - free(result); + if (ret < 0) return NULL; - } - if (!found) { - free(result); + if (!found) return NULL; - } - return result; + return move_ptr(result); } static void lxc_attach_get_init_uidgid(uid_t *init_uid, gid_t *init_gid) From e6d1aff5dd80057465b3d7c91953e0781d0f2611 Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Mon, 9 Mar 2020 10:08:37 +0100 Subject: [PATCH 06/11] attach: use LXC_INVALID_{G,U}ID macros Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- src/lxc/attach.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/lxc/attach.c b/src/lxc/attach.c index 267b948286..07e5217f00 100644 --- a/src/lxc/attach.c +++ b/src/lxc/attach.c @@ -518,8 +518,8 @@ static void lxc_attach_get_init_uidgid(uid_t *init_uid, gid_t *init_gid) int ret; size_t line_bufsz = 0; long value = -1; - uid_t uid = (uid_t)-1; - gid_t gid = (gid_t)-1; + uid_t uid = LXC_INVALID_UID; + gid_t gid = LXC_INVALID_GID; ret = snprintf(proc_fn, LXC_PROC_STATUS_LEN, "/proc/%d/status", 1); if (ret < 0 || ret >= LXC_PROC_STATUS_LEN) @@ -542,15 +542,15 @@ static void lxc_attach_get_init_uidgid(uid_t *init_uid, gid_t *init_gid) gid = (gid_t)value; } - if (uid != (uid_t)-1 && gid != (gid_t)-1) + if (uid != LXC_INVALID_UID && gid != LXC_INVALID_GID) break; } /* Only override arguments if we found something. */ - if (uid != (uid_t)-1) + if (uid != LXC_INVALID_UID) *init_uid = uid; - if (gid != (gid_t)-1) + if (gid != LXC_INVALID_UID) *init_gid = gid; /* TODO: we should also parse supplementary groups and use From 1ede54f5857be66121b975ae7119dfd22b716ff7 Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Mon, 9 Mar 2020 10:12:44 +0100 Subject: [PATCH 07/11] attach: use cleanup macros and logging helpers when fetching seccomp Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- src/lxc/attach.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/lxc/attach.c b/src/lxc/attach.c index 07e5217f00..56a0d9320e 100644 --- a/src/lxc/attach.c +++ b/src/lxc/attach.c @@ -566,8 +566,7 @@ static bool fetch_seccomp(struct lxc_container *c, lxc_attach_options_t *options if (!(options->namespaces & CLONE_NEWNS) || !(options->attach_flags & LXC_ATTACH_LSM)) { - free(c->lxc_conf->seccomp.seccomp); - c->lxc_conf->seccomp.seccomp = NULL; + free_disarm(c->lxc_conf->seccomp.seccomp); return true; } @@ -582,10 +581,8 @@ static bool fetch_seccomp(struct lxc_container *c, lxc_attach_options_t *options INFO("Failed to retrieve lxc.seccomp.profile"); path = c->get_running_config_item(c, "lxc.seccomp"); - if (!path) { - INFO("Failed to retrieve lxc.seccomp"); - return true; - } + if (!path) + return log_info(true, "Failed to retrieve lxc.seccomp"); } /* Copy the value into the new lxc_conf. */ @@ -595,13 +592,10 @@ static bool fetch_seccomp(struct lxc_container *c, lxc_attach_options_t *options /* Attempt to parse the resulting config. */ ret = lxc_read_seccomp_config(c->lxc_conf); - if (ret < 0) { - ERROR("Failed to retrieve seccomp policy"); - return false; - } + if (ret < 0) + return log_error(false, "Failed to retrieve seccomp policy"); - INFO("Retrieved seccomp policy"); - return true; + return log_info(true, "Retrieved seccomp policy"); } static bool no_new_privs(struct lxc_container *c, lxc_attach_options_t *options) From b7ca5d743a20acfc34904bd53b9d9732bc1dd24d Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Mon, 9 Mar 2020 10:14:34 +0100 Subject: [PATCH 08/11] attach: use logging helpers when handling no new privileges Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- src/lxc/attach.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/lxc/attach.c b/src/lxc/attach.c index 56a0d9320e..effc3ff79b 100644 --- a/src/lxc/attach.c +++ b/src/lxc/attach.c @@ -603,17 +603,13 @@ static bool no_new_privs(struct lxc_container *c, lxc_attach_options_t *options) __do_free char *val = NULL; /* Remove current setting. */ - if (!c->set_config_item(c, "lxc.no_new_privs", "")) { - INFO("Failed to unset lxc.no_new_privs"); - return false; - } + if (!c->set_config_item(c, "lxc.no_new_privs", "")) + return log_info(false, "Failed to unset lxc.no_new_privs"); /* Retrieve currently active setting. */ val = c->get_running_config_item(c, "lxc.no_new_privs"); - if (!val) { - INFO("Failed to retrieve lxc.no_new_privs"); - return false; - } + if (!val) + return log_info(false, "Failed to retrieve lxc.no_new_privs"); /* Set currently active setting. */ return c->set_config_item(c, "lxc.no_new_privs", val); From 0b3f761c6cf2ae6114d07b787cd0e5afb11d65a7 Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Mon, 9 Mar 2020 10:17:55 +0100 Subject: [PATCH 09/11] attach: cleanup various helpers Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- src/lxc/attach.c | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/src/lxc/attach.c b/src/lxc/attach.c index effc3ff79b..777be652b6 100644 --- a/src/lxc/attach.c +++ b/src/lxc/attach.c @@ -894,10 +894,8 @@ static int lxc_attach_terminal(struct lxc_conf *conf, lxc_terminal_init(terminal); ret = lxc_terminal_create(terminal); - if (ret < 0) { - ERROR("Failed to create terminal"); - return -1; - } + if (ret < 0) + return log_error(-1, "Failed to create terminal"); /* Shift ttys to container. */ ret = lxc_terminal_map_ids(conf, terminal); @@ -920,16 +918,13 @@ static int lxc_attach_terminal_mainloop_init(struct lxc_terminal *terminal, int ret; ret = lxc_mainloop_open(descr); - if (ret < 0) { - ERROR("Failed to create mainloop"); - return -1; - } + if (ret < 0) + return log_error(-1, "Failed to create mainloop"); ret = lxc_terminal_mainloop_add(descr, terminal); if (ret < 0) { - ERROR("Failed to add handlers to mainloop"); lxc_mainloop_close(descr); - return -1; + return log_error(-1, "Failed to add handlers to mainloop"); } return 0; @@ -971,10 +966,8 @@ int lxc_attach(struct lxc_container *container, lxc_attach_exec_t exec_function, struct attach_clone_payload payload = {0}; ret = access("/proc/self/ns", X_OK); - if (ret) { - SYSERROR("Does this kernel version support namespaces?"); - return -1; - } + if (ret) + return log_error_errno(-1, errno, "Does this kernel version support namespaces?"); if (!container) return ret_set_errno(-1, EINVAL); @@ -990,9 +983,8 @@ int lxc_attach(struct lxc_container *container, lxc_attach_exec_t exec_function, init_pid = lxc_cmd_get_init_pid(name, lxcpath); if (init_pid < 0) { - ERROR("Failed to get init pid"); lxc_container_put(container); - return -1; + return log_error(-1, "Failed to get init pid"); } init_ctx = lxc_proc_get_context_info(init_pid); @@ -1452,8 +1444,7 @@ int lxc_attach_run_command(void *payload) } } - SYSERROR("Failed to exec \"%s\"", cmd->program); - return ret; + return log_error_errno(ret, errno, "Failed to exec \"%s\"", cmd->program); } int lxc_attach_run_shell(void* payload) From 61b356f2614616fd3f2d98cdf53dbb5cce0cf9ad Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Mon, 9 Mar 2020 10:18:55 +0100 Subject: [PATCH 10/11] tree-wide: make files cloexec whenever possible Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- src/lxc/attach.c | 4 +- src/lxc/cgroups/cgfsng.c | 6 +- src/lxc/cmd/lxc_init.c | 4 +- src/lxc/cmd/lxc_user_nic.c | 2 +- src/lxc/cmd/lxc_usernsexec.c | 2 +- src/lxc/conf.c | 14 +++-- src/lxc/confile.c | 18 ++---- src/lxc/criu.c | 8 +-- src/lxc/file_utils.c | 118 +++++++++++++++++++++++------------ src/lxc/file_utils.h | 5 +- src/lxc/lxccontainer.c | 53 +++++++--------- src/lxc/network.c | 32 ++++------ src/lxc/pam/pam_cgfs.c | 8 +-- src/lxc/parse.c | 8 +-- src/lxc/seccomp.c | 17 ++--- src/lxc/utils.c | 90 ++++++++++---------------- src/lxc/utils.h | 16 ++--- 17 files changed, 191 insertions(+), 214 deletions(-) diff --git a/src/lxc/attach.c b/src/lxc/attach.c index 777be652b6..95b44bff69 100644 --- a/src/lxc/attach.c +++ b/src/lxc/attach.c @@ -429,7 +429,7 @@ static char *lxc_attach_getpwshell(uid_t uid) close(pipes[1]); - pipe_f = fdopen(pipes[0], "r"); + pipe_f = fdopen(pipes[0], "re"); if (!pipe_f) { close(pipes[0]); goto reap_child; @@ -525,7 +525,7 @@ static void lxc_attach_get_init_uidgid(uid_t *init_uid, gid_t *init_gid) if (ret < 0 || ret >= LXC_PROC_STATUS_LEN) return; - proc_file = fopen(proc_fn, "r"); + proc_file = fopen(proc_fn, "re"); if (!proc_file) return; diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c index 7677567cbf..4c21a6f29e 100644 --- a/src/lxc/cgroups/cgfsng.c +++ b/src/lxc/cgroups/cgfsng.c @@ -224,7 +224,7 @@ static char *read_file(const char *fnam) char *buf = NULL; size_t len = 0, fulllen = 0; - f = fopen(fnam, "r"); + f = fopen(fnam, "re"); if (!f) return NULL; while ((linelen = getline(&line, &len, f)) != -1) { @@ -902,7 +902,7 @@ static int get_existing_subsystems(char ***klist, char ***nlist) __do_fclose FILE *f = NULL; size_t len = 0; - f = fopen("/proc/self/cgroup", "r"); + f = fopen("/proc/self/cgroup", "re"); if (!f) return -1; @@ -2961,7 +2961,7 @@ static int cg_hybrid_init(struct cgroup_ops *ops, bool relative, bool unprivileg if (ret < 0) return log_error_errno(-1, errno, "Failed to retrieve available legacy cgroup controllers"); - f = fopen("/proc/self/mountinfo", "r"); + f = fopen("/proc/self/mountinfo", "re"); if (!f) return log_error_errno(-1, errno, "Failed to open \"/proc/self/mountinfo\""); diff --git a/src/lxc/cmd/lxc_init.c b/src/lxc/cmd/lxc_init.c index 000cefddb7..10fe64f2bb 100644 --- a/src/lxc/cmd/lxc_init.c +++ b/src/lxc/cmd/lxc_init.c @@ -85,7 +85,7 @@ static void prevent_forking(void) char path[PATH_MAX]; size_t len = 0; - f = fopen("/proc/self/cgroup", "r"); + f = fopen("/proc/self/cgroup", "re"); if (!f) return; @@ -150,7 +150,7 @@ static void kill_children(pid_t pid) return; } - f = fopen(path, "r"); + f = fopen(path, "re"); if (!f) { if (my_args.quiet) fprintf(stderr, "Failed to open %s\n", path); diff --git a/src/lxc/cmd/lxc_user_nic.c b/src/lxc/cmd/lxc_user_nic.c index 43b58d11ce..f2388a5b4c 100644 --- a/src/lxc/cmd/lxc_user_nic.c +++ b/src/lxc/cmd/lxc_user_nic.c @@ -323,7 +323,7 @@ static int get_alloted(char *me, char *intype, char *link, int count = 0; size_t len = 0; - fin = fopen(LXC_USERNIC_CONF, "r"); + fin = fopen(LXC_USERNIC_CONF, "re"); if (!fin) { CMD_SYSERROR("Failed to open \"%s\"\n", LXC_USERNIC_CONF); return -1; diff --git a/src/lxc/cmd/lxc_usernsexec.c b/src/lxc/cmd/lxc_usernsexec.c index 31e6d52cb1..6441fb3c86 100644 --- a/src/lxc/cmd/lxc_usernsexec.c +++ b/src/lxc/cmd/lxc_usernsexec.c @@ -195,7 +195,7 @@ static int read_default_map(char *fnam, int which, char *user) int ret = -1; size_t sz = 0; - fin = fopen(fnam, "r"); + fin = fopen(fnam, "re"); if (!fin) return -1; diff --git a/src/lxc/conf.c b/src/lxc/conf.c index 9678fe7bf2..a8d2ffadfd 100644 --- a/src/lxc/conf.c +++ b/src/lxc/conf.c @@ -1392,7 +1392,7 @@ int lxc_chroot(const struct lxc_rootfs *rootfs) int progress = 0; size_t len = 0; - f = fopen("./proc/self/mountinfo", "r"); + f = fopen("./proc/self/mountinfo", "re"); if (!f) { SYSERROR("Failed to open \"/proc/self/mountinfo\""); return -1; @@ -2366,6 +2366,7 @@ FILE *make_anonymous_mount_file(struct lxc_list *mount, bool include_nesting_helpers) { __do_close_prot_errno int fd = -EBADF; + FILE *f; int ret; char *mount_entry; struct lxc_list *iterator; @@ -2412,7 +2413,10 @@ FILE *make_anonymous_mount_file(struct lxc_list *mount, if (ret < 0) return NULL; - return fdopen(move_fd(fd), "r+"); + f = fdopen(fd, "re+"); + if (f) + move_fd(fd); /* Transfer ownership of fd. */ + return f; } static int setup_mount_entries(const struct lxc_conf *conf, @@ -3304,7 +3308,7 @@ void remount_all_slave(void) return; } - f = fdopen(memfd, "r"); + f = fdopen(memfd, "re"); if (!f) { SYSERROR("Failed to open copy of \"/proc/self/mountinfo\" to mark all shared. Continuing"); return; @@ -4704,7 +4708,7 @@ void suggest_default_idmap(void) if (!gname) return; - subuid_f = fopen(subuidfile, "r"); + subuid_f = fopen(subuidfile, "re"); if (!subuid_f) { ERROR("Your system is not configured with subuids"); return; @@ -4741,7 +4745,7 @@ void suggest_default_idmap(void) WARN("Could not parse UID range"); } - subgid_f = fopen(subgidfile, "r"); + subgid_f = fopen(subgidfile, "re"); if (!subgid_f) { ERROR("Your system is not configured with subgids"); return; diff --git a/src/lxc/confile.c b/src/lxc/confile.c index 8046638683..a24bcff745 100644 --- a/src/lxc/confile.c +++ b/src/lxc/confile.c @@ -150,7 +150,7 @@ lxc_config_define(proc); /* * Important Note: - * If a new config option is added to this table, be aware that + * If a new config option is added to this table, be aware that * the order in which the options are places into the table matters. * That means that more specific options of a namespace have to be * placed above more generic ones. @@ -2464,8 +2464,8 @@ int append_unexp_config_line(const char *line, struct lxc_conf *conf) static int do_includedir(const char *dirp, struct lxc_conf *lxc_conf) { + __do_closedir DIR *dir = NULL; struct dirent *direntp; - DIR *dir; char path[PATH_MAX]; int len; int ret = -1; @@ -2489,21 +2489,15 @@ static int do_includedir(const char *dirp, struct lxc_conf *lxc_conf) continue; len = snprintf(path, PATH_MAX, "%s/%s", dirp, fnam); - if (len < 0 || len >= PATH_MAX) { - ret = -1; - goto out; - } + if (len < 0 || len >= PATH_MAX) + return -1; ret = lxc_config_read(path, lxc_conf, true); if (ret < 0) - goto out; + return -1; } - ret = 0; -out: - closedir(dir); - - return ret; + return 0; } static int set_config_includefiles(const char *key, const char *value, diff --git a/src/lxc/criu.c b/src/lxc/criu.c index a68b4674c1..59e50047b9 100644 --- a/src/lxc/criu.c +++ b/src/lxc/criu.c @@ -89,7 +89,7 @@ static int load_tty_major_minor(char *directory, char *output, int len) return -1; } - f = fopen(path, "r"); + f = fopen(path, "re"); if (!f) { /* This means we're coming from a liblxc which didn't export * the tty info. In this case they had to have lxc.console.path @@ -793,7 +793,7 @@ static bool criu_version_ok(char **version) return false; } - f = fdopen(pipes[0], "r"); + f = fdopen(pipes[0], "re"); if (!f) { close(pipes[0]); return false; @@ -1085,7 +1085,7 @@ static void do_restore(struct lxc_container *c, int status_pipe, struct migrate_ goto out_fini_handler; } - FILE *f = fopen(buf, "r"); + FILE *f = fopen(buf, "re"); if (!f) { SYSERROR("couldn't read restore's children file %s", buf); goto out_fini_handler; @@ -1202,7 +1202,7 @@ static int save_tty_major_minor(char *directory, struct lxc_container *c, char * return -1; } - f = fopen(path, "w"); + f = fopen(path, "we"); if (!f) { SYSERROR("failed to open %s", path); return -1; diff --git a/src/lxc/file_utils.c b/src/lxc/file_utils.c index 009d26d7c8..687a4a7e9a 100644 --- a/src/lxc/file_utils.c +++ b/src/lxc/file_utils.c @@ -235,7 +235,7 @@ int print_to_file(const char *file, const char *content) FILE *f; int ret = 0; - f = fopen(file, "w"); + f = fopen(file, "we"); if (!f) return -1; @@ -395,45 +395,6 @@ ssize_t lxc_sendfile_nointr(int out_fd, int in_fd, off_t *offset, size_t count) return ret; } -char *file_to_buf(char *path, size_t *length) -{ - int fd; - char buf[PATH_MAX]; - char *copy = NULL; - - if (!length) - return NULL; - - fd = open(path, O_RDONLY | O_CLOEXEC); - if (fd < 0) - return NULL; - - *length = 0; - for (;;) { - int n; - char *old = copy; - - n = lxc_read_nointr(fd, buf, sizeof(buf)); - if (n < 0) - goto on_error; - if (!n) - break; - - copy = must_realloc(old, (*length + n) * sizeof(*old)); - memcpy(copy + *length, buf, n); - *length += n; - } - - close(fd); - return copy; - -on_error: - close(fd); - free(copy); - - return NULL; -} - int fd_to_fd(int from, int to) { for (;;) { @@ -463,3 +424,80 @@ int fd_to_fd(int from, int to) return 0; } + +static char *fd_to_buf(int fd, size_t *length) +{ + __do_free char *copy = NULL; + + if (!length) + return NULL; + + *length = 0; + for (;;) { + ssize_t bytes_read; + char buf[4096]; + char *old = copy; + + bytes_read = lxc_read_nointr(fd, buf, sizeof(buf)); + if (bytes_read < 0) + return NULL; + + if (!bytes_read) + break; + + copy = must_realloc(old, (*length + bytes_read) * sizeof(*old)); + memcpy(copy + *length, buf, bytes_read); + *length += bytes_read; + } + + return move_ptr(copy); +} + +char *file_to_buf(const char *path, size_t *length) +{ + __do_close_prot_errno int fd = -EBADF; + + if (!length) + return NULL; + + fd = open(path, O_RDONLY | O_CLOEXEC); + if (fd < 0) + return NULL; + + return fd_to_buf(fd, length); +} + +FILE *fopen_cached(const char *path, const char *mode, void **caller_freed_buffer) +{ + __do_free char *buf = NULL; + size_t len = 0; + FILE *f; + + buf = file_to_buf(path, &len); + if (!buf) + return NULL; + + f = fmemopen(buf, len, mode); + if (!f) + return NULL; + *caller_freed_buffer = move_ptr(buf); + return f; +} + +FILE *fdopen_cached(int fd, const char *mode, void **caller_freed_buffer) +{ + __do_free char *buf = NULL; + size_t len = 0; + FILE *f; + + buf = fd_to_buf(fd, &len); + if (!buf) + return NULL; + + f = fmemopen(buf, len, mode); + if (!f) + return NULL; + + *caller_freed_buffer = move_ptr(buf); + return f; +} diff --git a/src/lxc/file_utils.h b/src/lxc/file_utils.h index 9faf41d36d..8b31b976e6 100644 --- a/src/lxc/file_utils.h +++ b/src/lxc/file_utils.h @@ -48,8 +48,11 @@ extern bool is_fs_type(const struct statfs *fs, fs_type_magic magic_val); extern FILE *fopen_cloexec(const char *path, const char *mode); extern ssize_t lxc_sendfile_nointr(int out_fd, int in_fd, off_t *offset, size_t count); -extern char *file_to_buf(char *path, size_t *length); +extern char *file_to_buf(const char *path, size_t *length); extern int fd_to_fd(int from, int to); extern int lxc_open_dirfd(const char *dir); +extern FILE *fdopen_cached(int fd, const char *mode, void **caller_freed_buffer); +extern FILE *fopen_cached(const char *path, const char *mode, + void **caller_freed_buffer); #endif /* __LXC_FILE_UTILS_H */ diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c index 55e391870a..3bc347f4db 100644 --- a/src/lxc/lxccontainer.c +++ b/src/lxc/lxccontainer.c @@ -719,7 +719,7 @@ WRAP_API_2(bool, lxcapi_wait, const char *, int) static bool am_single_threaded(void) { - DIR *dir; + __do_closedir DIR *dir = NULL; struct dirent *direntp; int count = 0; @@ -738,7 +738,6 @@ static bool am_single_threaded(void) if (count > 1) break; } - closedir(dir); return count == 1; } @@ -1649,7 +1648,7 @@ static bool prepend_lxc_header(char *path, const char *t, char *const argv[]) char *tpath; #endif - f = fopen(path, "r"); + f = fopen(path, "re"); if (f == NULL) return false; @@ -1702,7 +1701,7 @@ static bool prepend_lxc_header(char *path, const char *t, char *const argv[]) free(tpath); #endif - f = fopen(path, "w"); + f = fopen(path, "we"); if (f == NULL) { SYSERROR("Reopening config for writing"); free(contents); @@ -2699,7 +2698,7 @@ static bool mod_rdep(struct lxc_container *c0, struct lxc_container *c, bool inc /* If we find an lxc-snapshot file using the old format only listing the * number of snapshots we will keep using it. */ - f1 = fopen(path, "r"); + f1 = fopen(path, "re"); if (f1) { n = fscanf(f1, "%d", &v); fclose(f1); @@ -2714,7 +2713,7 @@ static bool mod_rdep(struct lxc_container *c0, struct lxc_container *c, bool inc if (n == 1) { v += inc ? 1 : -1; - f1 = fopen(path, "w"); + f1 = fopen(path, "we"); if (!f1) goto out; @@ -2733,7 +2732,7 @@ static bool mod_rdep(struct lxc_container *c0, struct lxc_container *c, bool inc /* Here we know that we have or can use an lxc-snapshot file * using the new format. */ if (inc) { - f1 = fopen(path, "a"); + f1 = fopen(path, "ae"); if (!f1) goto out; @@ -2817,7 +2816,7 @@ void mod_all_rdeps(struct lxc_container *c, bool inc) return; } - f = fopen(path, "r"); + f = fopen(path, "re"); if (f == NULL) return; @@ -2868,7 +2867,7 @@ static bool has_fs_snapshots(struct lxc_container *c) v = fbuf.st_size; if (v != 0) { - f = fopen(path, "r"); + f = fopen(path, "re"); if (!f) goto out; @@ -2887,10 +2886,10 @@ static bool has_fs_snapshots(struct lxc_container *c) static bool has_snapshots(struct lxc_container *c) { + __do_closedir DIR *dir = NULL; char path[PATH_MAX]; struct dirent *direntp; - int count=0; - DIR *dir; + int count = 0; if (!get_snappath_dir(c, path)) return false; @@ -2909,7 +2908,6 @@ static bool has_snapshots(struct lxc_container *c) break; } - closedir(dir); return count > 0; } @@ -3540,7 +3538,7 @@ static bool add_rdepends(struct lxc_container *c, struct lxc_container *c0) if (ret < 0 || ret >= PATH_MAX) return false; - f = fopen(path, "a"); + f = fopen(path, "ae"); if (!f) return false; @@ -3736,7 +3734,7 @@ static int clone_update_rootfs(struct clone_update_data *data) if (!file_exists(path)) return 0; - if (!(fout = fopen(path, "w"))) { + if (!(fout = fopen(path, "we"))) { SYSERROR("unable to open %s: ignoring", path); return 0; } @@ -4216,7 +4214,7 @@ static int do_lxcapi_snapshot(struct lxc_container *c, const char *commentfile) len = strlen(snappath) + 1 + strlen(newname) + 1 + strlen(LXC_TIMESTAMP_FNAME) + 1; dfnam = must_realloc(NULL, len); snprintf(dfnam, len, "%s/%s/%s", snappath, newname, LXC_TIMESTAMP_FNAME); - f = fopen(dfnam, "w"); + f = fopen(dfnam, "we"); if (!f) { ERROR("Failed to open %s", dfnam); return -1; @@ -4284,7 +4282,7 @@ static char *get_timestamp(char* snappath, char *name) if (ret < 0 || ret >= PATH_MAX) return NULL; - fin = fopen(path, "r"); + fin = fopen(path, "re"); if (!fin) return NULL; @@ -4309,11 +4307,11 @@ static char *get_timestamp(char* snappath, char *name) static int do_lxcapi_snapshot_list(struct lxc_container *c, struct lxc_snapshot **ret_snaps) { + __do_closedir DIR *dir = NULL; char snappath[PATH_MAX], path2[PATH_MAX]; int count = 0, ret; struct dirent *direntp; struct lxc_snapshot *snaps =NULL, *nsnaps; - DIR *dir; if (!c || !lxcapi_is_defined(c)) return -1; @@ -4376,17 +4374,12 @@ static int do_lxcapi_snapshot_list(struct lxc_container *c, struct lxc_snapshot out_free: if (snaps) { - int i; - - for (i=0; i<count; i++) + for (int i = 0; i < count; i++) lxcsnap_free(&snaps[i]); free(snaps); } - if (closedir(dir)) - WARN("Failed to close directory"); - return -1; } @@ -4499,7 +4492,7 @@ static bool do_snapshot_destroy(const char *snapname, const char *clonelxcpath) static bool remove_all_snapshots(const char *path) { - DIR *dir; + __do_closedir DIR *dir = NULL; struct dirent *direntp; bool bret = true; @@ -4522,8 +4515,6 @@ static bool remove_all_snapshots(const char *path) } } - closedir(dir); - if (rmdir(path)) SYSERROR("Error removing directory %s", path); @@ -5433,7 +5424,7 @@ int lxc_get_wait_states(const char **states) */ int list_defined_containers(const char *lxcpath, char ***names, struct lxc_container ***cret) { - DIR *dir; + __do_closedir DIR *dir = NULL; int i, cfound = 0, nfound = 0; struct dirent *direntp; struct lxc_container *c; @@ -5504,23 +5495,21 @@ int list_defined_containers(const char *lxcpath, char ***names, struct lxc_conta nfound++; } - closedir(dir); return nfound; free_bad: if (names && *names) { - for (i=0; i<cfound; i++) + for (i = 0; i < cfound; i++) free((*names)[i]); free(*names); } if (cret && *cret) { - for (i=0; i<nfound; i++) + for (i = 0; i < nfound; i++) lxc_container_put((*cret)[i]); free(*cret); } - closedir(dir); return -1; } @@ -5545,7 +5534,7 @@ int list_active_containers(const char *lxcpath, char ***nret, if (nret) *nret = NULL; - FILE *f = fopen("/proc/net/unix", "r"); + FILE *f = fopen("/proc/net/unix", "re"); if (!f) return -1; diff --git a/src/lxc/network.c b/src/lxc/network.c index 7b9ea1f25a..dba90a58e3 100644 --- a/src/lxc/network.c +++ b/src/lxc/network.c @@ -1238,43 +1238,37 @@ int lxc_netdev_move_by_index(int ifindex, pid_t pid, const char *ifname) #define PHYSNAME "/sys/class/net/%s/phy80211/name" char *is_wlan(const char *ifname) { - __do_free char *path = NULL; + __do_fclose FILE *f = NULL; + __do_free char *path = NULL, *physname = NULL; int i, ret; long physlen; size_t len; - FILE *f; - char *physname = NULL; len = strlen(ifname) + strlen(PHYSNAME) - 1; path = must_realloc(NULL, len + 1); ret = snprintf(path, len, PHYSNAME, ifname); if (ret < 0 || (size_t)ret >= len) - goto bad; + return NULL; - f = fopen(path, "r"); + f = fopen(path, "re"); if (!f) - goto bad; + return NULL; /* Feh - sb.st_size is always 4096. */ fseek(f, 0, SEEK_END); physlen = ftell(f); fseek(f, 0, SEEK_SET); - if (physlen < 0) { - fclose(f); - goto bad; - } + if (physlen < 0) + return NULL; physname = malloc(physlen + 1); - if (!physname) { - fclose(f); - goto bad; - } + if (!physname) + return NULL; memset(physname, 0, physlen + 1); ret = fread(physname, 1, physlen, f); - fclose(f); if (ret < 0) - goto bad; + return NULL; for (i = 0; i < physlen; i++) { if (physname[i] == '\n') @@ -1284,11 +1278,7 @@ char *is_wlan(const char *ifname) break; } - return physname; - -bad: - free(physname); - return NULL; + return move_ptr(physname); } static int lxc_netdev_rename_by_name_in_netns(pid_t pid, const char *old, diff --git a/src/lxc/pam/pam_cgfs.c b/src/lxc/pam/pam_cgfs.c index 1624ce1602..0ea6b24066 100644 --- a/src/lxc/pam/pam_cgfs.c +++ b/src/lxc/pam/pam_cgfs.c @@ -486,8 +486,8 @@ static bool write_int(char *path, int v) /* Recursively remove directory and its parents. */ static int recursive_rmdir(char *dirname) { + __do_closedir DIR *dir = NULL; struct dirent *direntp; - DIR *dir; int r = 0; dir = opendir(dirname); @@ -527,12 +527,6 @@ static int recursive_rmdir(char *dirname) r = -1; } - if (closedir(dir) < 0) { - if (!r) - pam_cgfs_debug("Failed to delete %s: %s\n", dirname, strerror(errno)); - r = -1; - } - return r; } diff --git a/src/lxc/parse.c b/src/lxc/parse.c index e6b0460f57..291bf3efc1 100644 --- a/src/lxc/parse.c +++ b/src/lxc/parse.c @@ -141,12 +141,12 @@ int lxc_file_for_each_line_mmap(const char *file, lxc_file_cb callback, void *da int lxc_file_for_each_line(const char *file, lxc_file_cb callback, void *data) { - FILE *f; + __do_fclose FILE *f = NULL; + __do_free char *line = NULL; int err = 0; - char *line = NULL; size_t len = 0; - f = fopen(file, "r"); + f = fopen(file, "re"); if (!f) { SYSERROR("Failed to open \"%s\"", file); return -1; @@ -164,7 +164,5 @@ int lxc_file_for_each_line(const char *file, lxc_file_cb callback, void *data) } } - free(line); - fclose(f); return err; } diff --git a/src/lxc/seccomp.c b/src/lxc/seccomp.c index f1ed813848..0c56ec5caf 100644 --- a/src/lxc/seccomp.c +++ b/src/lxc/seccomp.c @@ -1133,16 +1133,16 @@ static int parse_config(FILE *f, struct lxc_conf *conf) */ static bool use_seccomp(const struct lxc_conf *conf) { + __do_free char *line = NULL; + __do_fclose FILE *f = NULL; int ret, v; - FILE *f; size_t line_bufsz = 0; - char *line = NULL; bool already_enabled = false, found = false; if (conf->seccomp.allow_nesting > 0) return true; - f = fopen("/proc/self/status", "r"); + f = fopen("/proc/self/status", "re"); if (!f) return true; @@ -1157,8 +1157,6 @@ static bool use_seccomp(const struct lxc_conf *conf) break; } } - free(line); - fclose(f); if (!found) { INFO("Seccomp is not enabled in the kernel"); @@ -1175,8 +1173,8 @@ static bool use_seccomp(const struct lxc_conf *conf) int lxc_read_seccomp_config(struct lxc_conf *conf) { + __do_fclose FILE *f = NULL; int ret; - FILE *f; if (!conf->seccomp.seccomp) return 0; @@ -1217,16 +1215,13 @@ int lxc_read_seccomp_config(struct lxc_conf *conf) } #endif - f = fopen(conf->seccomp.seccomp, "r"); + f = fopen(conf->seccomp.seccomp, "re"); if (!f) { SYSERROR("Failed to open seccomp policy file %s", conf->seccomp.seccomp); return -1; } - ret = parse_config(f, conf); - fclose(f); - - return ret; + return parse_config(f, conf); } int lxc_seccomp_load(struct lxc_conf *conf) diff --git a/src/lxc/utils.c b/src/lxc/utils.c index 406a54c0aa..a3495635af 100644 --- a/src/lxc/utils.c +++ b/src/lxc/utils.c @@ -514,19 +514,17 @@ int lxc_pclose(struct lxc_popen_FILE *fp) int randseed(bool srand_it) { - FILE *f; + __do_fclose FILE *f = NULL; /* * srand pre-seed function based on /dev/urandom */ unsigned int seed = time(NULL) + getpid(); - f = fopen("/dev/urandom", "r"); + f = fopen("/dev/urandom", "re"); if (f) { int ret = fread(&seed, sizeof(seed), 1, f); if (ret != 1) SYSDEBUG("Unable to fread /dev/urandom, fallback to time+pid rand seed"); - - fclose(f); } if (srand_it) @@ -537,12 +535,12 @@ int randseed(bool srand_it) uid_t get_ns_uid(uid_t orig) { - char *line = NULL; + __do_free char *line = NULL; + __do_fclose FILE *f = NULL; size_t sz = 0; uid_t nsid, hostid, range; - FILE *f; - f = fopen("/proc/self/uid_map", "r"); + f = fopen("/proc/self/uid_map", "re"); if (!f) { SYSERROR("Failed to open uid_map"); return 0; @@ -552,28 +550,21 @@ uid_t get_ns_uid(uid_t orig) if (sscanf(line, "%u %u %u", &nsid, &hostid, &range) != 3) continue; - if (hostid <= orig && hostid + range > orig) { - nsid += orig - hostid; - goto found; - } + if (hostid <= orig && hostid + range > orig) + return nsid += orig - hostid; } - nsid = LXC_INVALID_UID; - -found: - fclose(f); - free(line); - return nsid; + return LXC_INVALID_UID; } gid_t get_ns_gid(gid_t orig) { - char *line = NULL; + __do_free char *line = NULL; + __do_fclose FILE *f = NULL; size_t sz = 0; gid_t nsid, hostid, range; - FILE *f; - f = fopen("/proc/self/gid_map", "r"); + f = fopen("/proc/self/gid_map", "re"); if (!f) { SYSERROR("Failed to open gid_map"); return 0; @@ -583,18 +574,11 @@ gid_t get_ns_gid(gid_t orig) if (sscanf(line, "%u %u %u", &nsid, &hostid, &range) != 3) continue; - if (hostid <= orig && hostid + range > orig) { - nsid += orig - hostid; - goto found; - } + if (hostid <= orig && hostid + range > orig) + return nsid += orig - hostid; } - nsid = LXC_INVALID_GID; - -found: - fclose(f); - free(line); - return nsid; + return LXC_INVALID_GID; } bool dir_exists(const char *path) @@ -640,7 +624,7 @@ bool is_shared_mountpoint(const char *path) int i; size_t len = 0; - f = fopen("/proc/self/mountinfo", "r"); + f = fopen("/proc/self/mountinfo", "re"); if (!f) return 0; @@ -722,19 +706,19 @@ bool switch_to_ns(pid_t pid, const char *ns) */ bool detect_ramfs_rootfs(void) { - FILE *f; - char *p, *p2; - char *line = NULL; + __do_free char *line = NULL; + __do_free void *fopen_cache = NULL; + __do_fclose FILE *f = NULL; size_t len = 0; - int i; - f = fopen("/proc/self/mountinfo", "r"); - if (!f) { - SYSERROR("Failed to open mountinfo"); + f = fopen_cached("/proc/self/mountinfo", "re", &fopen_cache); + if (!f) return false; - } while (getline(&line, &len, f) != -1) { + int i; + char *p, *p2; + for (p = line, i = 0; p && i < 4; i++) p = strchr(p + 1, ' '); if (!p) @@ -743,22 +727,15 @@ bool detect_ramfs_rootfs(void) p2 = strchr(p + 1, ' '); if (!p2) continue; - *p2 = '\0'; if (strcmp(p + 1, "/") == 0) { /* This is '/'. Is it the ramfs? */ p = strchr(p2 + 1, '-'); - if (p && strncmp(p, "- rootfs rootfs ", 16) == 0) { - free(line); - fclose(f); - INFO("Rootfs is located on ramfs"); + if (p && strncmp(p, "- rootfs rootfs ", 16) == 0) return true; - } } } - free(line); - fclose(f); return false; } @@ -1317,21 +1294,21 @@ int null_stdfds(void) #define __PROC_STATUS_LEN (6 + INTTYPE_TO_STRLEN(pid_t) + 7 + 1) bool task_blocks_signal(pid_t pid, int signal) { + __do_free char *line = NULL; + __do_fclose FILE *f = NULL; int ret; char status[__PROC_STATUS_LEN] = {0}; - FILE *f; uint64_t sigblk = 0, one = 1; size_t n = 0; bool bret = false; - char *line = NULL; ret = snprintf(status, __PROC_STATUS_LEN, "/proc/%d/status", pid); if (ret < 0 || ret >= __PROC_STATUS_LEN) return bret; - f = fopen(status, "r"); + f = fopen(status, "re"); if (!f) - return bret; + return false; while (getline(&line, &n, f) != -1) { char *numstr; @@ -1342,7 +1319,7 @@ bool task_blocks_signal(pid_t pid, int signal) numstr = lxc_trim_whitespace_in_place(line + 7); ret = lxc_safe_uint64(numstr, &sigblk, 16); if (ret < 0) - goto out; + return false; break; } @@ -1350,9 +1327,6 @@ bool task_blocks_signal(pid_t pid, int signal) if (sigblk & (one << (signal - 1))) bret = true; -out: - free(line); - fclose(f); return bret; } @@ -1723,11 +1697,13 @@ static int process_dead(/* takes */ int status_fd) if (fd_cloexec(dupfd, true) < 0) return -1; - /* transfer ownership of fd */ - f = fdopen(move_fd(dupfd), "re"); + f = fdopen(dupfd, "re"); if (!f) return -1; + /* Transfer ownership of fd. */ + move_fd(dupfd); + ret = 0; while (getline(&line, &n, f) != -1) { char *state; diff --git a/src/lxc/utils.h b/src/lxc/utils.h index 24d615de46..18476bb110 100644 --- a/src/lxc/utils.h +++ b/src/lxc/utils.h @@ -24,6 +24,7 @@ #include "file_utils.h" #include "initutils.h" #include "macro.h" +#include "memory_utils.h" #include "raw_syscalls.h" #include "string_utils.h" @@ -93,7 +94,7 @@ inline static bool am_guest_unpriv(void) { /* are we unprivileged with respect to init_user_ns */ inline static bool am_host_unpriv(void) { - FILE *f; + __do_fclose FILE *f = NULL; uid_t user, host, count; int ret; @@ -103,20 +104,15 @@ inline static bool am_host_unpriv(void) /* Now: are we in a user namespace? Because then we're also * unprivileged. */ - f = fopen("/proc/self/uid_map", "r"); - if (!f) { + f = fopen("/proc/self/uid_map", "re"); + if (!f) return false; - } ret = fscanf(f, "%u %u %u", &user, &host, &count); - fclose(f); - if (ret != 3) { + if (ret != 3) return false; - } - if (user != 0 || host != 0 || count != UINT32_MAX) - return true; - return false; + return user != 0 || host != 0 || count != UINT32_MAX; } /* From 4ca7263514dbc2b5792f0e17ef587a8cdf77384a Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Mon, 9 Mar 2020 10:59:14 +0100 Subject: [PATCH 11/11] tree-wide: improve logging Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- src/lxc/attach.c | 2 +- src/lxc/log.h | 8 +- src/lxc/start.c | 217 +++++++++++++++++++---------------------------- 3 files changed, 92 insertions(+), 135 deletions(-) diff --git a/src/lxc/attach.c b/src/lxc/attach.c index 95b44bff69..d943b324b5 100644 --- a/src/lxc/attach.c +++ b/src/lxc/attach.c @@ -325,7 +325,7 @@ static int lxc_attach_set_environment(struct lxc_proc_context_info *init_ctx, ret = putenv("container=lxc"); if (ret < 0) - return log_warn(-1, errno, "Failed to set environment variable"); + return log_warn(-1, "Failed to set environment variable"); /* Set container environment variables.*/ if (init_ctx && init_ctx->container && init_ctx->container->lxc_conf) { diff --git a/src/lxc/log.h b/src/lxc/log.h index 545626e46f..c00638cb74 100644 --- a/src/lxc/log.h +++ b/src/lxc/log.h @@ -510,10 +510,10 @@ __lxc_unused static inline void LXC_##LEVEL(struct lxc_log_locinfo* locinfo, \ __ret__; \ }) -#define log_warn(__ret__, __errno__, format, ...) \ - ({ \ - WARN(format, ##__VA_ARGS__); \ - __ret__; \ +#define log_warn(__ret__, format, ...) \ + ({ \ + WARN(format, ##__VA_ARGS__); \ + __ret__; \ }) #define log_debug_errno(__ret__, __errno__, format, ...) \ diff --git a/src/lxc/start.c b/src/lxc/start.c index b727e3ab9b..9e8069b2a2 100644 --- a/src/lxc/start.c +++ b/src/lxc/start.c @@ -94,9 +94,7 @@ static void print_top_failing_dir(const char *path) ret = access(copy, X_OK); if (ret != 0) { - SYSERROR("Could not access %s. Please grant it x " - "access, or add an ACL for the container " - "root", copy); + SYSERROR("Could not access %s. Please grant it x access, or add an ACL for the container " "root", copy); return; } *p = saved; @@ -105,14 +103,11 @@ static void print_top_failing_dir(const char *path) static void lxc_put_nsfds(struct lxc_handler *handler) { - int i; - - for (i = 0; i < LXC_NS_MAX; i++) { + for (int i = 0; i < LXC_NS_MAX; i++) { if (handler->nsfd[i] < 0) continue; - close(handler->nsfd[i]); - handler->nsfd[i] = -EBADF; + close_prot_errno_disarm(handler->nsfd[i]); } } @@ -122,13 +117,14 @@ static int lxc_try_preserve_ns(const int pid, const char *ns) fd = lxc_preserve_ns(pid, ns); if (fd < 0) { - if (errno != ENOENT) { - SYSERROR("Failed to preserve %s namespace", ns); - return -EINVAL; - } + if (errno != ENOENT) + return log_error_errno(-EINVAL, + errno, "Failed to preserve %s namespace", + ns); - SYSWARN("Kernel does not support preserving %s namespaces", ns); - return -EOPNOTSUPP; + return log_error_errno(-EOPNOTSUPP, + errno, "Kernel does not support preserving %s namespaces", + ns); } return fd; @@ -187,23 +183,18 @@ static bool match_dlog_fds(struct dirent *direntp) int ret; ret = snprintf(path, PATH_MAX, "/proc/self/fd/%s", direntp->d_name); - if (ret < 0 || ret >= PATH_MAX) { - ERROR("Failed to create file descriptor name"); - return false; - } + if (ret < 0 || ret >= PATH_MAX) + return log_error(false, "Failed to create file descriptor name"); linklen = readlink(path, link, PATH_MAX); - if (linklen < 0) { - SYSERROR("Failed to read link path - \"%s\"", path); - return false; - } else if (linklen >= PATH_MAX) { - ERROR("The name of link path is too long - \"%s\"", path); - return false; - } - - if (strcmp(link, "/dev/log_main") == 0 || - strcmp(link, "/dev/log_system") == 0 || - strcmp(link, "/dev/log_radio") == 0) + if (linklen < 0) + return log_error(false, "Failed to read link path - \"%s\"", path); + else if (linklen >= PATH_MAX) + return log_error(false, "The name of link path is too long - \"%s\"", path); + + if (strcmp(link, "/dev/log_main") == 0 || + strcmp(link, "/dev/log_system") == 0 || + strcmp(link, "/dev/log_radio") == 0) return true; return false; @@ -223,10 +214,8 @@ int lxc_check_inherited(struct lxc_conf *conf, bool closeall, restart: dir = opendir("/proc/self/fd"); - if (!dir) { - SYSWARN("Failed to open directory"); - return -1; - } + if (!dir) + return log_warn(-1, "Failed to open directory"); fddir = dirfd(dir); @@ -319,16 +308,14 @@ static int setup_signal_fd(sigset_t *oldmask) } ret = pthread_sigmask(SIG_BLOCK, &mask, oldmask); - if (ret < 0) { - SYSERROR("Failed to set signal mask"); - return -EBADF; - } + if (ret < 0) + return log_error_errno(-EBADF, errno, + "Failed to set signal mask"); ret = signalfd(-1, &mask, SFD_CLOEXEC); - if (ret < 0) { - SYSERROR("Failed to create signal file descriptor"); - return -EBADF; - } + if (ret < 0) + return log_error_errno(-EBADF, + errno, "Failed to create signal file descriptor"); TRACE("Created signal file descriptor %d", ret); @@ -344,15 +331,11 @@ static int signal_handler(int fd, uint32_t events, void *data, struct lxc_handler *hdlr = data; ret = lxc_read_nointr(fd, &siginfo, sizeof(siginfo)); - if (ret < 0) { - ERROR("Failed to read signal info from signal file descriptor %d", fd); - return LXC_MAINLOOP_ERROR; - } + if (ret < 0) + return log_error(LXC_MAINLOOP_ERROR, "Failed to read signal info from signal file descriptor %d", fd); - if (ret != sizeof(siginfo)) { - ERROR("Unexpected size for struct signalfd_siginfo"); - return -EINVAL; - } + if (ret != sizeof(siginfo)) + return log_error(-EINVAL, "Unexpected size for struct signalfd_siginfo"); /* Check whether init is running. */ info.si_pid = 0; @@ -425,9 +408,7 @@ static int signal_handler(int fd, uint32_t events, void *data, : LXC_MAINLOOP_CONTINUE; } - DEBUG("Container init process %d exited", hdlr->pid); - - return LXC_MAINLOOP_CLOSE; + return log_debug(LXC_MAINLOOP_CLOSE, "Container init process %d exited", hdlr->pid); } int lxc_serve_state_clients(const char *name, struct lxc_handler *handler, @@ -445,10 +426,8 @@ int lxc_serve_state_clients(const char *name, struct lxc_handler *handler, TRACE("Set container state to %s", lxc_state2str(state)); - if (lxc_list_empty(&handler->conf->state_clients)) { - TRACE("No state clients registered"); - return 0; - } + if (lxc_list_empty(&handler->conf->state_clients)) + return log_trace(0, "No state clients registered"); retlen = strlcpy(msg.name, name, sizeof(msg.name)); if (retlen >= sizeof(msg.name)) @@ -507,17 +486,14 @@ static int lxc_serve_state_socket_pair(const char *name, return -1; } - if (ret != sizeof(int)) { - ERROR("Message too long : %d", handler->state_socket_pair[1]); - return -1; - } + if (ret != sizeof(int)) + return log_error(-1, "Message too long : %d", handler->state_socket_pair[1]); TRACE("Sent container state \"%s\" to %d", lxc_state2str(state), handler->state_socket_pair[1]); /* Close write end of the socket pair. */ - close(handler->state_socket_pair[1]); - handler->state_socket_pair[1] = -1; + close_prot_errno_disarm(handler->state_socket_pair[1]); return 0; } @@ -528,10 +504,8 @@ int lxc_set_state(const char *name, struct lxc_handler *handler, int ret; ret = lxc_serve_state_socket_pair(name, handler, state); - if (ret < 0) { - ERROR("Failed to synchronize via anonymous pair of unix sockets"); - return -1; - } + if (ret < 0) + return log_error(-1, "Failed to synchronize via anonymous pair of unix sockets"); ret = lxc_serve_state_clients(name, handler, state); if (ret < 0) @@ -636,39 +610,37 @@ int lxc_poll(const char *name, struct lxc_handler *handler) void lxc_zero_handler(struct lxc_handler *handler) { - int i; - memset(handler, 0, sizeof(struct lxc_handler)); - handler->pinfd = -1; + handler->pinfd = -EBADF; handler->pidfd = -EBADF; - handler->sigfd = -1; + handler->sigfd = -EBADF; - for (i = 0; i < LXC_NS_MAX; i++) - handler->nsfd[i] = -1; + for (int i = 0; i < LXC_NS_MAX; i++) + handler->nsfd[i] = -EBADF; - handler->data_sock[0] = -1; - handler->data_sock[1] = -1; + handler->data_sock[0] = -EBADF; + handler->data_sock[1] = -EBADF; - handler->state_socket_pair[0] = -1; - handler->state_socket_pair[1] = -1; + handler->state_socket_pair[0] = -EBADF; + handler->state_socket_pair[1] = -EBADF; - handler->sync_sock[0] = -1; - handler->sync_sock[1] = -1; + handler->sync_sock[0] = -EBADF; + handler->sync_sock[1] = -EBADF; } void lxc_free_handler(struct lxc_handler *handler) { if (handler->pinfd >= 0) - close(handler->pinfd); + close_prot_errno_disarm(handler->pinfd); if (handler->pidfd >= 0) - close(handler->pidfd); + close_prot_errno_disarm(handler->pidfd); if (handler->sigfd >= 0) - close(handler->sigfd); + close_prot_errno_disarm(handler->sigfd); lxc_put_nsfds(handler); @@ -677,26 +649,25 @@ void lxc_free_handler(struct lxc_handler *handler) lxc_abstract_unix_close(handler->conf->maincmd_fd); if (handler->monitor_status_fd >= 0) - close(handler->monitor_status_fd); + close_prot_errno_disarm(handler->monitor_status_fd); if (handler->state_socket_pair[0] >= 0) - close(handler->state_socket_pair[0]); + close_prot_errno_disarm(handler->state_socket_pair[0]); if (handler->state_socket_pair[1] >= 0) - close(handler->state_socket_pair[1]); + close_prot_errno_disarm(handler->state_socket_pair[1]); if (handler->cgroup_ops) cgroup_exit(handler->cgroup_ops); handler->conf = NULL; - free(handler); - handler = NULL; + free_disarm(handler); } struct lxc_handler *lxc_init_handler(const char *name, struct lxc_conf *conf, const char *lxcpath, bool daemonize) { - int i, ret; + int ret; struct lxc_handler *handler; handler = malloc(sizeof(*handler)); @@ -710,20 +681,22 @@ struct lxc_handler *lxc_init_handler(const char *name, struct lxc_conf *conf, * as root so this should be fine. */ handler->am_root = !am_guest_unpriv(); - handler->data_sock[0] = handler->data_sock[1] = -1; handler->conf = conf; handler->lxcpath = lxcpath; + handler->init_died = false; + handler->data_sock[0] = -EBADF; + handler->data_sock[1] = -EBADF; handler->monitor_status_fd = -EBADF; - handler->pinfd = -1; + handler->pinfd = -EBADF; handler->pidfd = -EBADF; handler->sigfd = -EBADF; - handler->init_died = false; - handler->state_socket_pair[0] = handler->state_socket_pair[1] = -1; + handler->state_socket_pair[0] = -EBADF; + handler->state_socket_pair[1] = -EBADF; if (handler->conf->reboot == REBOOT_NONE) lxc_list_init(&handler->conf->state_clients); - for (i = 0; i < LXC_NS_MAX; i++) - handler->nsfd[i] = -1; + for (int i = 0; i < LXC_NS_MAX; i++) + handler->nsfd[i] = -EBADF; handler->name = name; if (daemonize) @@ -807,50 +780,43 @@ int lxc_init(const char *name, struct lxc_handler *handler) if (conf->rcfile) { ret = setenv("LXC_CONFIG_FILE", conf->rcfile, 1); if (ret < 0) - SYSERROR("Failed to set environment variable: " - "LXC_CONFIG_FILE=%s", conf->rcfile); + SYSERROR("Failed to set environment variable: LXC_CONFIG_FILE=%s", conf->rcfile); } if (conf->rootfs.mount) { ret = setenv("LXC_ROOTFS_MOUNT", conf->rootfs.mount, 1); if (ret < 0) - SYSERROR("Failed to set environment variable: " - "LXC_ROOTFS_MOUNT=%s", conf->rootfs.mount); + SYSERROR("Failed to set environment variable: LXC_ROOTFS_MOUNT=%s", conf->rootfs.mount); } if (conf->rootfs.path) { ret = setenv("LXC_ROOTFS_PATH", conf->rootfs.path, 1); if (ret < 0) - SYSERROR("Failed to set environment variable: " - "LXC_ROOTFS_PATH=%s", conf->rootfs.path); + SYSERROR("Failed to set environment variable: LXC_ROOTFS_PATH=%s", conf->rootfs.path); } if (conf->console.path) { ret = setenv("LXC_CONSOLE", conf->console.path, 1); if (ret < 0) - SYSERROR("Failed to set environment variable: " - "LXC_CONSOLE=%s", conf->console.path); + SYSERROR("Failed to set environment variable: LXC_CONSOLE=%s", conf->console.path); } if (conf->console.log_path) { ret = setenv("LXC_CONSOLE_LOGPATH", conf->console.log_path, 1); if (ret < 0) - SYSERROR("Failed to set environment variable: " - "LXC_CONSOLE_LOGPATH=%s", conf->console.log_path); + SYSERROR("Failed to set environment variable: LXC_CONSOLE_LOGPATH=%s", conf->console.log_path); } if (cgns_supported()) { ret = setenv("LXC_CGNS_AWARE", "1", 1); if (ret < 0) - SYSERROR("Failed to set environment variable " - "LXC_CGNS_AWARE=1"); + SYSERROR("Failed to set environment variable LXC_CGNS_AWARE=1"); } loglevel = lxc_log_priority_to_string(lxc_log_get_level()); ret = setenv("LXC_LOG_LEVEL", loglevel, 1); if (ret < 0) - SYSERROR("Set environment variable LXC_LOG_LEVEL=%s", - loglevel); + SYSERROR("Set environment variable LXC_LOG_LEVEL=%s", loglevel); if (conf->hooks_version == 0) ret = setenv("LXC_HOOK_VERSION", "0", 1); @@ -933,7 +899,7 @@ int lxc_init(const char *name, struct lxc_handler *handler) void lxc_fini(const char *name, struct lxc_handler *handler) { - int i, ret; + int ret; pid_t self; struct lxc_list *cur, *next; char *namespaces[LXC_NS_MAX + 1]; @@ -946,7 +912,7 @@ void lxc_fini(const char *name, struct lxc_handler *handler) lxc_set_state(name, handler, STOPPING); self = lxc_raw_getpid(); - for (i = 0; i < LXC_NS_MAX; i++) { + for (int i = 0; i < LXC_NS_MAX; i++) { if (handler->nsfd[i] < 0) continue; @@ -957,7 +923,7 @@ void lxc_fini(const char *name, struct lxc_handler *handler) else ret = asprintf(&namespaces[namespace_count], "/proc/%d/fd/%d", self, handler->nsfd[i]); - if (ret == -1) { + if (ret < 0) { SYSERROR("Failed to allocate memory"); break; } @@ -982,15 +948,13 @@ void lxc_fini(const char *name, struct lxc_handler *handler) if (handler->conf->reboot > REBOOT_NONE) { ret = setenv("LXC_TARGET", "reboot", 1); if (ret < 0) - SYSERROR("Failed to set environment variable: " - "LXC_TARGET=reboot"); + SYSERROR("Failed to set environment variable: LXC_TARGET=reboot"); } if (handler->conf->reboot == REBOOT_NONE) { ret = setenv("LXC_TARGET", "stop", 1); if (ret < 0) - SYSERROR("Failed to set environment variable: " - "LXC_TARGET=stop"); + SYSERROR("Failed to set environment variable: LXC_TARGET=stop"); } if (handler->conf->hooks_version == 0) @@ -1018,7 +982,7 @@ void lxc_fini(const char *name, struct lxc_handler *handler) * because we haven't yet closed the command socket. */ lxc_abstract_unix_close(handler->conf->maincmd_fd); - handler->conf->maincmd_fd = -1; + handler->conf->maincmd_fd = -EBADF; TRACE("Closed command socket"); /* This function will try to connect to the legacy lxc-monitord @@ -1047,8 +1011,7 @@ void lxc_fini(const char *name, struct lxc_handler *handler) ret = setenv("LXC_TARGET", "stop", 1); if (ret < 0) - WARN("Failed to set environment variable: " - "LXC_TARGET=stop"); + WARN("Failed to set environment variable: LXC_TARGET=stop"); } } @@ -1251,9 +1214,7 @@ static int do_start(void *data) if (devnull_fd < 0) goto out_warn_father; - WARN("Using /dev/null from the host for container " - "init's standard file descriptors. Migration will " - "not work"); + WARN("Using /dev/null from the host for container init's standard file descriptors. Migration will not work"); } } @@ -1321,12 +1282,10 @@ static int do_start(void *data) ret = prctl(PR_SET_NO_NEW_PRIVS, prctl_arg(1), prctl_arg(0), prctl_arg(0), prctl_arg(0)); if (ret < 0) { - SYSERROR("Could not set PR_SET_NO_NEW_PRIVS to block " - "execve() gainable privileges"); + SYSERROR("Could not set PR_SET_NO_NEW_PRIVS to block execve() gainable privileges"); goto out_warn_father; } - DEBUG("Set PR_SET_NO_NEW_PRIVS to block execve() gainable " - "privileges"); + DEBUG("Set PR_SET_NO_NEW_PRIVS to block execve() gainable privileges"); } /* Some init's such as busybox will set sane tty settings on stdin, @@ -1341,8 +1300,8 @@ static int do_start(void *data) else ret = lxc_terminal_set_stdfds(handler->conf->console.slave); if (ret < 0) { - ERROR("Failed to redirect std{in,out,err} to pty file " - "descriptor %d", handler->conf->console.slave); + ERROR("Failed to redirect std{in,out,err} to pty file descriptor %d", + handler->conf->console.slave); goto out_warn_father; } } @@ -1522,8 +1481,7 @@ static int lxc_recv_ttys_from_child(struct lxc_handler *handler) tty->busy = -1; tty->master = ttyfds[0]; tty->slave = ttyfds[1]; - TRACE("Received pty with master fd %d and slave fd %d from " - "child", tty->master, tty->slave); + TRACE("Received pty with master fd %d and slave fd %d from child", tty->master, tty->slave); } if (ret < 0) @@ -2072,8 +2030,7 @@ int __lxc_start(const char *name, struct lxc_handler *handler, ret = lxc_restore_phys_nics_to_netns(handler); if (ret < 0) - ERROR("Failed to move physical network devices back to parent " - "network namespace"); + ERROR("Failed to move physical network devices back to parent network namespace"); if (handler->pinfd >= 0) { close(handler->pinfd);
_______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel