[lxc-devel] [lxc/master] tree-wide: fixes
brauner on Github
lxc-bot at linuxcontainers.org
Fri Mar 27 14:43:28 UTC 2020
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 364 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200327/416f57a2/attachment.bin>
-------------- next part --------------
From 8bc2b675f279cf201b7e5d45627c96be0a85c3c0 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Fri, 27 Mar 2020 11:52:44 +0100
Subject: [PATCH 1/4] attach: use close_prot_errno_disarm()
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/attach.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/lxc/attach.c b/src/lxc/attach.c
index ca1fd2bd64..15cc5f3793 100644
--- a/src/lxc/attach.c
+++ b/src/lxc/attach.c
@@ -1326,8 +1326,7 @@ int lxc_attach(struct lxc_container *container, lxc_attach_exec_t exec_function,
}
/* close unneeded file descriptors */
- close(ipc_sockets[0]);
- ipc_sockets[0] = -EBADF;
+ close_prot_errno_disarm(ipc_sockets[0]);
if (options->attach_flags & LXC_ATTACH_TERMINAL) {
lxc_attach_terminal_close_master(&terminal);
From 0d113b16f0d3ad998f45e0bca188d03c6646f9cb Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Fri, 27 Mar 2020 12:00:22 +0100
Subject: [PATCH 2/4] cgroups: remove unused variable
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/cgroups/cgfsng.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index 4bcdf0c925..42cef8831a 100644
--- a/src/lxc/cgroups/cgfsng.c
+++ b/src/lxc/cgroups/cgfsng.c
@@ -2118,7 +2118,6 @@ struct userns_exec_unified_attach_data {
const struct lxc_conf *conf;
int unified_fd;
pid_t pid;
- uid_t origuid;
};
static int cgroup_unified_attach_wrapper(void *data)
From cea5da2c88fbc58f1e654358bc92bc3d58688ede Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Fri, 27 Mar 2020 15:38:27 +0100
Subject: [PATCH 3/4] cgroups: fix unified cgroup attach
There's a fundamental problem with futexes and setid calls and the go runtime.
POSIX requires that when one thread setids all threas must setids and it uses
futexes and signals to synchronize the state across threads. This causes
deadlocks which means we can't use the pretty solution I first implemented.
Instead we need to chown after we create the directory. I might come up with
something smarter later but for now this will do.
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/cgroups/cgfsng.c | 100 +++++++++++++++------------------------
1 file changed, 37 insertions(+), 63 deletions(-)
diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index 42cef8831a..f335b836c8 100644
--- a/src/lxc/cgroups/cgfsng.c
+++ b/src/lxc/cgroups/cgfsng.c
@@ -2057,8 +2057,11 @@ static inline char *build_full_cgpath_from_monitorpath(struct hierarchy *h,
return must_make_path(h->mountpoint, inpath, filename, NULL);
}
-static int cgroup_attach_leaf(const struct lxc_conf *conf, int unified_fd, pid_t pid)
+#define LXC_UNIFIED_ATTACH_CGROUP_LEN STRLITERALLEN("/lxc-1000/cgroup.procs")
+static int cgroup_attach_leaf(const struct lxc_conf *conf, char *unified_path,
+ int unified_fd, pid_t pid)
{
+ __do_free char *path = NULL;
int idx = 1;
int ret;
char pidstr[INTTYPE_TO_STRLEN(int64_t) + 1];
@@ -2069,6 +2072,11 @@ static int cgroup_attach_leaf(const struct lxc_conf *conf, int unified_fd, pid_t
if (ret < 0 && errno != EEXIST)
return log_error_errno(-1, errno, "Failed to create leaf cgroup \"lxc\"");
+ path = must_make_path(unified_path, "lxc", NULL);
+ ret = chown_mapped_root(path, conf);
+ if (ret < 0)
+ return log_error_errno(-1, errno, "Failed to chown \"%s\"", path);
+
pidstr_len = sprintf(pidstr, INT64_FMT, (int64_t)pid);
ret = lxc_writeat(unified_fd, "lxc/cgroup.procs", pidstr, pidstr_len);
if (ret < 0)
@@ -2080,9 +2088,10 @@ static int cgroup_attach_leaf(const struct lxc_conf *conf, int unified_fd, pid_t
if (errno != EBUSY)
return log_error_errno(-1, errno, "Failed to attach to unified cgroup");
+ free_disarm(path);
do {
bool rm = false;
- char attach_cgroup[STRLITERALLEN("lxc-1000/cgroup.procs") + 1];
+ char attach_cgroup[LXC_UNIFIED_ATTACH_CGROUP_LEN + 1];
char *slash;
sprintf(attach_cgroup, "lxc-%d/cgroup.procs", idx);
@@ -2097,6 +2106,12 @@ static int cgroup_attach_leaf(const struct lxc_conf *conf, int unified_fd, pid_t
*slash = '/';
+ path = must_make_path(unified_path, "attach_cgroup", NULL);
+ ret = chown_mapped_root(path, conf);
+ if (ret < 0)
+ return log_error_errno(-1, errno, "Failed to chown \"%s\"", path);
+ free_disarm(path);
+
ret = lxc_writeat(unified_fd, attach_cgroup, pidstr, pidstr_len);
if (ret == 0)
return 0;
@@ -2114,46 +2129,14 @@ static int cgroup_attach_leaf(const struct lxc_conf *conf, int unified_fd, pid_t
return log_error_errno(-1, errno, "Failed to attach to unified cgroup");
}
-struct userns_exec_unified_attach_data {
- const struct lxc_conf *conf;
- int unified_fd;
- pid_t pid;
-};
-
-static int cgroup_unified_attach_wrapper(void *data)
-{
- struct userns_exec_unified_attach_data *args = data;
- uid_t nsuid;
- gid_t nsgid;
- int ret;
-
- if (!args->conf || args->unified_fd < 0 || args->pid <= 0)
- return ret_errno(EINVAL);
-
- if (!lxc_setgroups(0, NULL) && errno != EPERM)
- return log_error_errno(-1, errno, "Failed to setgroups(0, NULL)");
-
- nsuid = (args->conf->root_nsuid_map != NULL) ? 0 : args->conf->init_uid;
- nsgid = (args->conf->root_nsgid_map != NULL) ? 0 : args->conf->init_gid;
-
- ret = setresgid(nsgid, nsgid, nsgid);
- if (ret < 0)
- return log_error_errno(-1, errno, "Failed to setresgid(%d, %d, %d)",
- (int)nsgid, (int)nsgid, (int)nsgid);
-
- ret = setresuid(nsuid, nsuid, nsuid);
- if (ret < 0)
- return log_error_errno(-1, errno, "Failed to setresuid(%d, %d, %d)",
- (int)nsuid, (int)nsuid, (int)nsuid);
-
- return cgroup_attach_leaf(args->conf, args->unified_fd, args->pid);
-}
-
int cgroup_attach(const struct lxc_conf *conf, const char *name,
const char *lxcpath, pid_t pid)
{
__do_close int unified_fd = -EBADF;
+ __do_free char *buf = NULL;
int ret;
+ ssize_t len;
+ struct stat st;
if (!conf || !name || !lxcpath || pid <= 0)
return ret_errno(EINVAL);
@@ -2162,20 +2145,24 @@ int cgroup_attach(const struct lxc_conf *conf, const char *name,
if (unified_fd < 0)
return ret_errno(EBADF);
- if (!lxc_list_empty(&conf->id_map)) {
- struct userns_exec_unified_attach_data args = {
- .conf = conf,
- .unified_fd = unified_fd,
- .pid = pid,
- };
+ ret = fstatat(unified_fd, "", &st, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW);
+ if (ret < 0)
+ return -errno;
- ret = userns_exec_1(conf, cgroup_unified_attach_wrapper, &args,
- "cgroup_unified_attach_wrapper");
- } else {
- ret = cgroup_attach_leaf(conf, unified_fd, pid);
- }
+ if (st.st_size == 0)
+ return ret_errno(EINVAL);
- return ret;
+ buf = zalloc(st.st_size);
+ if (!buf)
+ return ret_errno(ENOMEM);
+
+ len = readlinkat(unified_fd, "", buf, st.st_size);
+ if (len < 0)
+ return -errno;
+ if (len >= st.st_size)
+ return ret_errno(E2BIG);
+
+ return cgroup_attach_leaf(conf, buf, unified_fd, pid);
}
/* Technically, we're always at a delegation boundary here (This is especially
@@ -2217,20 +2204,7 @@ static int __cg_unified_attach(const struct hierarchy *h,
if (unified_fd < 0)
return ret_errno(EBADF);
- if (!lxc_list_empty(&conf->id_map)) {
- struct userns_exec_unified_attach_data args = {
- .conf = conf,
- .unified_fd = unified_fd,
- .pid = pid,
- };
-
- ret = userns_exec_1(conf, cgroup_unified_attach_wrapper, &args,
- "cgroup_unified_attach_wrapper");
- } else {
- ret = cgroup_attach_leaf(conf, unified_fd, pid);
- }
-
- return ret;
+ return cgroup_attach_leaf(conf, path, unified_fd, pid);
}
__cgfsng_ops static bool cgfsng_attach(struct cgroup_ops *ops,
From c56568a2d42852536ac871b893e7970ab0d056c9 Mon Sep 17 00:00:00 2001
From: Wolfgang Bumiller <w.bumiller at proxmox.com>
Date: Fri, 27 Mar 2020 14:15:12 +0100
Subject: [PATCH 4/4] fixup i/o handler return values
Particularly important for lxc_cmd_handler() handles client
input and should not be capable of canceling the main loop,
some syscall return values leaked through overlapping with
LXC_MAINLOOP_ERROR, causing unauthorized clients connecting
to the command socket to shutdown the main loop.
In turn, signal_handler() receiving unexpected
`signalfd_siginfo` struct sizes seems like a reason to bail
(since it's a kernel interface).
Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/commands.c | 9 ++-------
src/lxc/seccomp.c | 4 +---
src/lxc/start.c | 2 +-
3 files changed, 4 insertions(+), 11 deletions(-)
diff --git a/src/lxc/commands.c b/src/lxc/commands.c
index 20890a719a..8b2d0e0b7a 100644
--- a/src/lxc/commands.c
+++ b/src/lxc/commands.c
@@ -1450,7 +1450,7 @@ static int lxc_cmd_handler(int fd, uint32_t events, void *data,
if (errno == EACCES) {
/* We don't care for the peer, just send and close. */
struct lxc_cmd_rsp rsp = {
- .ret = ret,
+ .ret = -EPERM,
};
lxc_cmd_rsp_send(fd, &rsp);
@@ -1464,14 +1464,11 @@ static int lxc_cmd_handler(int fd, uint32_t events, void *data,
if (ret != sizeof(req)) {
WARN("Failed to receive full command request. Ignoring request for \"%s\"", lxc_cmd_str(req.cmd));
- ret = -1;
goto out_close;
}
if ((req.datalen > LXC_CMD_DATA_MAX) && (req.cmd != LXC_CMD_CONSOLE_LOG)) {
ERROR("Received command data length %d is too large for command \"%s\"", req.datalen, lxc_cmd_str(req.cmd));
- errno = EFBIG;
- ret = -EFBIG;
goto out_close;
}
@@ -1480,7 +1477,6 @@ static int lxc_cmd_handler(int fd, uint32_t events, void *data,
ret = lxc_recv_nointr(fd, reqdata, req.datalen, 0);
if (ret != req.datalen) {
WARN("Failed to receive full command request. Ignoring request for \"%s\"", lxc_cmd_str(req.cmd));
- ret = LXC_MAINLOOP_ERROR;
goto out_close;
}
@@ -1490,12 +1486,11 @@ static int lxc_cmd_handler(int fd, uint32_t events, void *data,
ret = lxc_cmd_process(fd, &req, handler, descr);
if (ret) {
/* This is not an error, but only a request to close fd. */
- ret = LXC_MAINLOOP_CONTINUE;
goto out_close;
}
out:
- return ret;
+ return LXC_MAINLOOP_CONTINUE;
out_close:
lxc_cmd_fd_cleanup(fd, handler, descr, req.cmd);
diff --git a/src/lxc/seccomp.c b/src/lxc/seccomp.c
index 916b1aa1a8..081d315ab5 100644
--- a/src/lxc/seccomp.c
+++ b/src/lxc/seccomp.c
@@ -1478,10 +1478,8 @@ int seccomp_notify_handler(int fd, uint32_t events, void *data,
SYSERROR("Failed to send seccomp notification");
out:
- return 0;
-#else
- return -ENOSYS;
#endif
+ return LXC_MAINLOOP_CONTINUE;
}
void seccomp_conf_init(struct lxc_conf *conf)
diff --git a/src/lxc/start.c b/src/lxc/start.c
index 62152a6f60..c8ebe77265 100644
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -335,7 +335,7 @@ static int signal_handler(int fd, uint32_t events, void *data,
return log_error(LXC_MAINLOOP_ERROR, "Failed to read signal info from signal file descriptor %d", fd);
if (ret != sizeof(siginfo))
- return log_error(-EINVAL, "Unexpected size for struct signalfd_siginfo");
+ return log_error(LXC_MAINLOOP_ERROR, "Unexpected size for struct signalfd_siginfo");
/* Check whether init is running. */
info.si_pid = 0;
More information about the lxc-devel
mailing list