From 1454e5d9a07c597e208e586b41a689a7a963f352 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Tue, 23 Feb 2021 22:07:11 +0100 Subject: [PATCH 1/3] commands: only deref once Fixes: Coverity 1473308 Signed-off-by: Christian Brauner --- src/lxc/commands.c | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/src/lxc/commands.c b/src/lxc/commands.c index 7564c70c85..82c3d32857 100644 --- a/src/lxc/commands.c +++ b/src/lxc/commands.c @@ -131,11 +131,13 @@ static int lxc_cmd_rsp_recv(int sock, struct lxc_cmd_rr *cmd) { call_cleaner(put_unix_fds) struct unix_fds *fds = &(struct unix_fds){}; struct lxc_cmd_rsp *rsp = &cmd->rsp; - const char *reqstr = lxc_cmd_str(cmd->req.cmd); + int cur_cmd = cmd->req.cmd; + const char *cur_cmdstr; int fret = 0; int ret; - switch (cmd->req.cmd) { + cur_cmdstr = lxc_cmd_str(cur_cmd); + switch (cur_cmd) { case LXC_CMD_GET_CGROUP2_FD: __fallthrough; case LXC_CMD_GET_LIMITING_CGROUP2_FD: @@ -158,16 +160,16 @@ static int lxc_cmd_rsp_recv(int sock, struct lxc_cmd_rr *cmd) } ret = lxc_abstract_unix_recv_fds(sock, fds, rsp, sizeof(*rsp)); if (ret < 0) - return syserrno(ret, "Failed to receive response for command \"%s\"", reqstr); + return syserrno(ret, "Failed to receive response for command \"%s\"", cur_cmdstr); if (fds->fd_count_max == 0) { - TRACE("Command \"%s\" received response with %u file descriptors", reqstr, fds->fd_count_ret); + TRACE("Command \"%s\" received response with %u file descriptors", cur_cmdstr, fds->fd_count_ret); } else if (fds->fd_count_ret == 0) { - WARN("Command \"%s\" received response without expected file descriptors", reqstr); + WARN("Command \"%s\" received response without expected file descriptors", cur_cmdstr); fret = -EBADF; } - if (cmd->req.cmd == LXC_CMD_CONSOLE) { + if (cur_cmd == LXC_CMD_CONSOLE) { struct lxc_cmd_console_rsp_data *rspdata; /* recv() returns 0 bytes when a tty cannot be allocated, @@ -178,14 +180,14 @@ static int lxc_cmd_rsp_recv(int sock, struct lxc_cmd_rr *cmd) rspdata = malloc(sizeof(*rspdata)); if (!rspdata) - return syserrno_set(-ENOMEM, "Failed to receive response for command \"%s\"", reqstr); + return syserrno_set(-ENOMEM, "Failed to receive response for command \"%s\"", cur_cmdstr); rspdata->ptxfd = move_fd(fds->fd[0]); rspdata->ttynum = PTR_TO_INT(rsp->data); rsp->data = rspdata; } - switch (cmd->req.cmd) { + switch (cur_cmd) { case LXC_CMD_GET_CGROUP2_FD: __fallthrough; case LXC_CMD_GET_LIMITING_CGROUP2_FD: @@ -196,10 +198,10 @@ static int lxc_cmd_rsp_recv(int sock, struct lxc_cmd_rr *cmd) __fallthrough; case LXC_CMD_GET_SECCOMP_NOTIFY_FD: rsp->data = INT_TO_PTR(move_fd(fds->fd[0])); - return log_debug(fret ?: ret, "Finished processing \"%s\"", reqstr); + return log_debug(fret ?: ret, "Finished processing \"%s\"", cur_cmdstr); case LXC_CMD_GET_CGROUP_CTX: if ((rsp->datalen == 0) || (rsp->datalen > sizeof(struct cgroup_ctx))) - return syserrno_set(fret ?: -EINVAL, "Invalid response size from server for \"%s\"", reqstr); + return syserrno_set(fret ?: -EINVAL, "Invalid response size from server for \"%s\"", cur_cmdstr); /* Don't pointlessly allocate. */ rsp->data = (void *)cmd->req.data; @@ -209,28 +211,28 @@ static int lxc_cmd_rsp_recv(int sock, struct lxc_cmd_rr *cmd) } if (rsp->datalen == 0) - return log_debug(fret ?: ret, "Response data length for command \"%s\" is 0", reqstr); + return log_debug(fret ?: ret, "Response data length for command \"%s\" is 0", cur_cmdstr); if ((rsp->datalen > LXC_CMD_DATA_MAX) && - (cmd->req.cmd != LXC_CMD_CONSOLE_LOG)) + (cur_cmd != LXC_CMD_CONSOLE_LOG)) return syserrno_set(-E2BIG, "Response data for command \"%s\" is too long: %d bytes > %d", - reqstr, rsp->datalen, LXC_CMD_DATA_MAX); + cur_cmdstr, rsp->datalen, LXC_CMD_DATA_MAX); - if (cmd->req.cmd == LXC_CMD_CONSOLE_LOG) + if (cur_cmd == LXC_CMD_CONSOLE_LOG) rsp->data = zalloc(rsp->datalen + 1); - else if (cmd->req.cmd != LXC_CMD_GET_CGROUP_CTX) + else if (cur_cmd != LXC_CMD_GET_CGROUP_CTX) rsp->data = malloc(rsp->datalen); if (!rsp->data) - return syserrno_set(-ENOMEM, "Failed to allocate response buffer for command \"%s\"", reqstr); + return syserrno_set(-ENOMEM, "Failed to allocate response buffer for command \"%s\"", cur_cmdstr); ret = lxc_recv_nointr(sock, rsp->data, rsp->datalen, 0); if (ret != rsp->datalen) - return syserrno(-errno, "Failed to receive response data for command \"%s\"", reqstr); + return syserrno(-errno, "Failed to receive response data for command \"%s\"", cur_cmdstr); - if (cmd->req.cmd == LXC_CMD_GET_CGROUP_CTX) { + if (cur_cmd == LXC_CMD_GET_CGROUP_CTX) { ret = __transfer_cgroup_ctx_fds(fds, rsp->data); if (ret < 0) - return syserrno(ret, "Failed to transfer file descriptors for \"%s\"", reqstr); + return syserrno(ret, "Failed to transfer file descriptors for \"%s\"", cur_cmdstr); } return fret ?: ret; From 92fea74bfe5186184d26c5bc89bd2d8f1895574b Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Tue, 23 Feb 2021 22:08:48 +0100 Subject: [PATCH 2/3] af_unix: prevent oob writes Fixes: Coverity 1473309 Signed-off-by: Christian Brauner --- src/lxc/af_unix.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lxc/af_unix.c b/src/lxc/af_unix.c index 526e38ad02..747e688205 100644 --- a/src/lxc/af_unix.c +++ b/src/lxc/af_unix.c @@ -218,7 +218,7 @@ static ssize_t lxc_abstract_unix_recv_fds_iov(int fd, * which exceeds the kernel limit we know about so * close them and return an error. */ - if (num_raw > KERNEL_SCM_MAX_FD) { + if (num_raw >= KERNEL_SCM_MAX_FD) { for (idx = 0; idx < num_raw; idx++) close(fds_raw[idx]); From 2d8b9ab865ea42ba5d72a3231b7d1a98c8999fc1 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Tue, 23 Feb 2021 22:10:56 +0100 Subject: [PATCH 3/3] cgroups: fix error checking Fixes: Coverity 1473310 Signed-off-by: Christian Brauner --- src/lxc/cgroups/cgfsng.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c index 5d4275c013..283b0dbe1d 100644 --- a/src/lxc/cgroups/cgfsng.c +++ b/src/lxc/cgroups/cgfsng.c @@ -3432,7 +3432,7 @@ int cgroup_attach(const struct lxc_conf *conf, const char *name, ret = __cgroup_attach_many(conf, name, lxcpath, pid); if (ret < 0) { - if (ret != ENOSYS) + if (ret != -ENOSYS) return ret; ret = __cgroup_attach_unified(conf, name, lxcpath, pid);