From: Christian Brauner Date: Thu, 25 Feb 2021 09:48:14 +0000 (+0100) Subject: commands: rework lxc_cmd_rsp_recv() to make it more obvious X-Git-Tag: lxc-5.0.0~267^2~5 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3a9daa046cf3bab85122844a97bf076f16cb9f5d;p=thirdparty%2Flxc.git commands: rework lxc_cmd_rsp_recv() to make it more obvious Signed-off-by: Christian Brauner --- diff --git a/src/lxc/commands.c b/src/lxc/commands.c index 984f13550..9090a8b9a 100644 --- a/src/lxc/commands.c +++ b/src/lxc/commands.c @@ -118,6 +118,38 @@ static int __transfer_cgroup_fd(struct unix_fds *fds, struct cgroup_fd *fd) return 0; } +static ssize_t lxc_cmd_rsp_recv_fds(int fd_sock, struct unix_fds *fds, + struct lxc_cmd_rsp *rsp, + const char *cur_cmdstr) +{ + ssize_t ret; + + ret = lxc_abstract_unix_recv_fds(fd_sock, fds, rsp, sizeof(*rsp)); + if (ret < 0) + return ret; + + /* + * If we end up here with fewer or more file descriptors the caller + * must have set flags to indicate that they are fine with this. + * Otherwise the call would have failed. + */ + + if (fds->flags & UNIX_FDS_RECEIVED_EXACT) + return log_info(ret, "Received exact number of file descriptors %u == %u", + fds->fd_count_max, fds->fd_count_ret); + + if (fds->flags & UNIX_FDS_RECEIVED_LESS) + return log_info(ret, "Received less file descriptors %u < %u", + fds->fd_count_ret, fds->fd_count_max); + + if (fds->flags & UNIX_FDS_RECEIVED_MORE) + return log_info(ret, "Received more file descriptors (excessive fds were automatically closed) %u > %u", + fds->fd_count_ret, fds->fd_count_max); + + INFO("Command \"%s\" received response", cur_cmdstr); + return ret; +} + /* * lxc_cmd_rsp_recv: Receive a response to a command * @@ -134,15 +166,17 @@ static int __transfer_cgroup_fd(struct unix_fds *fds, struct cgroup_fd *fd) * As a special case, the response for LXC_CMD_GET_TTY_FD is created here as * it contains an fd for the ptx pty passed through the unix socket. */ -static int lxc_cmd_rsp_recv(int sock, struct lxc_cmd_rr *cmd) +static ssize_t lxc_cmd_rsp_recv(int sock, struct lxc_cmd_rr *cmd) { __do_free void *__private_ptr = NULL; struct lxc_cmd_tty_rsp_data *data_console = NULL; - call_cleaner(put_unix_fds) struct unix_fds *fds = &(struct unix_fds){}; + call_cleaner(put_unix_fds) struct unix_fds *fds = &(struct unix_fds){ + .fd[0 ... KERNEL_SCM_MAX_FD - 1] = -EBADF, + }; struct lxc_cmd_rsp *rsp = &cmd->rsp; - int cur_cmd = cmd->req.cmd, fret = 0; + int cur_cmd = cmd->req.cmd; const char *cur_cmdstr; - int ret; + ssize_t bytes_recv; /* * Determine whether this command will receive file descriptors and how @@ -163,12 +197,24 @@ static int lxc_cmd_rsp_recv(int sock, struct lxc_cmd_rr *cmd) case LXC_CMD_GET_SECCOMP_NOTIFY_FD: __fallthrough; case LXC_CMD_GET_DEVPTS_FD: - __fallthrough; + fds->fd_count_max = 1; + /* + * The kernel might not support the required features or the + * server might be too old. + */ + fds->flags = UNIX_FDS_ACCEPT_EXACT | UNIX_FDS_ACCEPT_NONE; + break; case LXC_CMD_GET_TTY_FD: + /* + * The requested terminal can be busy so it's perfectly fine + * for LXC_CMD_GET_TTY to receive no file descriptor. + */ fds->fd_count_max = 1; + fds->flags = UNIX_FDS_ACCEPT_EXACT | UNIX_FDS_ACCEPT_NONE; break; case LXC_CMD_GET_CGROUP_CTX: fds->fd_count_max = CGROUP_CTX_MAX_FD; + fds->flags |= UNIX_FDS_ACCEPT_LESS; break; default: fds->fd_count_max = 0; @@ -176,22 +222,9 @@ static int lxc_cmd_rsp_recv(int sock, struct lxc_cmd_rr *cmd) } /* Receive the first response including file descriptors if any. */ - ret = lxc_abstract_unix_recv_fds(sock, fds, rsp, sizeof(*rsp)); - if (ret < 0) - return syserrno(ret, "Failed to receive response for command \"%s\"", cur_cmdstr); - - /* - * Verify that we actually received any file descriptors if the command - * expects to do so. - */ - if (fds->fd_count_max == 0) { - WARN("Command \"%s\" received response", cur_cmdstr); - } else if (fds->fd_count_ret == 0) { - TRACE("Command \"%s\" received response without any of the expected %u file descriptors", cur_cmdstr, fds->fd_count_max); - fret = -EBADF; - } else { - TRACE("Command \"%s\" received response with %u of %u expected file descriptors", cur_cmdstr, fds->fd_count_ret, fds->fd_count_max); - } + bytes_recv = lxc_cmd_rsp_recv_fds(sock, fds, rsp, cur_cmdstr); + if (bytes_recv < 0) + return bytes_recv; /* * Ensure that no excessive data is sent unless someone retrieves the @@ -199,7 +232,7 @@ static int lxc_cmd_rsp_recv(int sock, struct lxc_cmd_rr *cmd) */ if ((rsp->datalen > LXC_CMD_DATA_MAX) && (cur_cmd != LXC_CMD_CONSOLE_LOG)) - return syserrno_set(fret ?: -E2BIG, "Response data for command \"%s\" is too long: %d bytes > %d", + return syserrno_set(-E2BIG, "Response data for command \"%s\" is too long: %d bytes > %d", cur_cmdstr, rsp->datalen, LXC_CMD_DATA_MAX); /* @@ -216,23 +249,20 @@ static int lxc_cmd_rsp_recv(int sock, struct lxc_cmd_rr *cmd) case LXC_CMD_GET_DEVPTS_FD: /* no data */ __fallthrough; case LXC_CMD_GET_SECCOMP_NOTIFY_FD: /* no data */ - if (!fret) - rsp->data = INT_TO_PTR(move_fd(fds->fd[0])); - - /* Return for any command that doesn't expect additional data. */ - return log_debug(fret ?: ret, "Finished processing \"%s\" with file descriptor %d", cur_cmdstr, PTR_TO_INT(rsp->data)); + rsp->data = INT_TO_PTR(move_fd(fds->fd[0])); + return log_debug(0, "Finished processing \"%s\" with file descriptor %d", cur_cmdstr, PTR_TO_INT(rsp->data)); case LXC_CMD_GET_CGROUP_FD: /* data */ __fallthrough; case LXC_CMD_GET_LIMIT_CGROUP_FD: /* data */ if (rsp->datalen > sizeof(struct cgroup_fd)) - return syserrno_set(fret ?: -EINVAL, "Invalid response size from server for \"%s\"", cur_cmdstr); + return syserrno_set(-EINVAL, "Invalid response size from server for \"%s\"", cur_cmdstr); /* Don't pointlessly allocate. */ rsp->data = (void *)cmd->req.data; break; case LXC_CMD_GET_CGROUP_CTX: /* data */ if (rsp->datalen > sizeof(struct cgroup_ctx)) - return syserrno_set(fret ?: -EINVAL, "Invalid response size from server for \"%s\"", cur_cmdstr); + return syserrno_set(-EINVAL, "Invalid response size from server for \"%s\"", cur_cmdstr); /* Don't pointlessly allocate. */ rsp->data = (void *)cmd->req.data; @@ -240,14 +270,14 @@ static int lxc_cmd_rsp_recv(int sock, struct lxc_cmd_rr *cmd) case LXC_CMD_GET_TTY_FD: /* data */ /* * recv() returns 0 bytes when a tty cannot be allocated, - * rsp->ret is < 0 when the peer permission check failed + * rsp->ret is < 0 when the peer permission check failed. */ - if (ret == 0 || rsp->ret < 0) + if (bytes_recv == 0 || rsp->ret < 0) return 0; __private_ptr = malloc(sizeof(struct lxc_cmd_tty_rsp_data)); if (!__private_ptr) - return syserrno_set(fret ?: -ENOMEM, "Failed to receive response for command \"%s\"", cur_cmdstr); + return syserrno_set(-ENOMEM, "Failed to receive response for command \"%s\"", cur_cmdstr); data_console = (struct lxc_cmd_tty_rsp_data *)__private_ptr; data_console->ptxfd = move_fd(fds->fd[0]); data_console->ttynum = PTR_TO_INT(rsp->data); @@ -267,48 +297,40 @@ static int lxc_cmd_rsp_recv(int sock, struct lxc_cmd_rr *cmd) break; } - if (rsp->datalen == 0) { - DEBUG("Command \"%s\" requested no additional data", cur_cmdstr); + if (rsp->datalen > 0) { + int err; + /* - * Note that LXC_CMD_GET_TTY_FD historically allocates memory - * to return info to the caller. That's why we jump to no_data - * so we ensure that the allocated data is wiped if we return - * early here. + * All commands ending up here expect data so rsp->data must be valid. + * Either static or allocated memory. */ - goto no_data; - } - - /* - * All commands ending up here expect data so rsp->data must be valid. - * Either static or allocated memory. - */ - if (!rsp->data) - return syserrno_set(fret ?: -ENOMEM, "Failed to prepare 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\": %d != %d", cur_cmdstr, ret, rsp->datalen); - - switch (cur_cmd) { - case LXC_CMD_GET_CGROUP_CTX: - if (!fret) - ret = __transfer_cgroup_ctx_fds(fds, rsp->data); - /* Make sure any received fds are wiped by us. */ - break; - case LXC_CMD_GET_CGROUP_FD: - __fallthrough; - case LXC_CMD_GET_LIMIT_CGROUP_FD: - if (!fret) - ret = __transfer_cgroup_fd(fds, rsp->data); - /* Make sure any received fds are wiped by us. */ - break; + if (!rsp->data) + return syserrno_set(-ENOMEM, "Failed to prepare response buffer for command \"%s\"", + cur_cmdstr); + + bytes_recv = lxc_recv_nointr(sock, rsp->data, rsp->datalen, 0); + if (bytes_recv != rsp->datalen) + return syserrno(-errno, "Failed to receive response data for command \"%s\": %zd != %d", + cur_cmdstr, bytes_recv, rsp->datalen); + + switch (cur_cmd) { + case LXC_CMD_GET_CGROUP_CTX: + err = __transfer_cgroup_ctx_fds(fds, rsp->data); + break; + case LXC_CMD_GET_CGROUP_FD: + __fallthrough; + case LXC_CMD_GET_LIMIT_CGROUP_FD: + err = __transfer_cgroup_fd(fds, rsp->data); + break; + default: + err = 0; + } + if (err < 0) + return syserrno(err, "Failed to transfer file descriptors for command \"%s\"", cur_cmdstr); } -no_data: - if (!fret && ret >= 0) - move_ptr(__private_ptr); - - return fret ?: ret; + move_ptr(__private_ptr); + return bytes_recv; } /* diff --git a/src/lxc/log.h b/src/lxc/log.h index 3e3dafa9a..2c536aa98 100644 --- a/src/lxc/log.h +++ b/src/lxc/log.h @@ -515,6 +515,13 @@ __lxc_unused static inline void LXC_##LEVEL(struct lxc_log_locinfo* locinfo, \ __internal_ret__; \ }) +#define systrace(__ret__, format, ...) \ + ({ \ + typeof(__ret__) __internal_ret__ = (__ret__); \ + SYSTRACE(format, ##__VA_ARGS__); \ + __internal_ret__; \ + }) + #define sysinfo(__ret__, format, ...) \ ({ \ typeof(__ret__) __internal_ret__ = (__ret__); \ @@ -525,7 +532,7 @@ __lxc_unused static inline void LXC_##LEVEL(struct lxc_log_locinfo* locinfo, \ #define syserrno_set(__ret__, format, ...) \ ({ \ typeof(__ret__) __internal_ret__ = (__ret__); \ - errno = abs(__ret__); \ + errno = labs(__ret__); \ SYSERROR(format, ##__VA_ARGS__); \ __internal_ret__; \ }) @@ -533,7 +540,7 @@ __lxc_unused static inline void LXC_##LEVEL(struct lxc_log_locinfo* locinfo, \ #define syswarn_set(__ret__, format, ...) \ ({ \ typeof(__ret__) __internal_ret__ = (__ret__); \ - errno = abs(__ret__); \ + errno = labs(__ret__); \ SYSWARN(format, ##__VA_ARGS__); \ __internal_ret__; \ })