[lxc-devel] [lxc/master] 2020 03 04/improvements
brauner on Github
lxc-bot at linuxcontainers.org
Mon Mar 9 12:17:18 UTC 2020
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 301 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200309/21474236/attachment-0001.bin>
-------------- next part --------------
From 55171a21aff5db21b5307b4732739e05bff82eb8 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Wed, 4 Mar 2020 15:21:18 +0100
Subject: [PATCH 01/11] af_unix: cleanup
Signed-off-by: Christian Brauner <christian.brauner at 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.brauner at 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.brauner at 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.brauner at ubuntu.com>
Date: Thu, 5 Mar 2020 10:03:38 +0100
Subject: [PATCH 03/11] attach: cleanup
Signed-off-by: Christian Brauner <christian.brauner at 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.brauner at 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.brauner at 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.brauner at 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.brauner at 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.brauner at 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.brauner at 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.brauner at 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.brauner at 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.brauner at 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.brauner at 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.brauner at 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.brauner at 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.brauner at 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.brauner at 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.brauner at 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.brauner at 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);
More information about the lxc-devel
mailing list