]> git.proxmox.com Git - mirror_lxc.git/commitdiff
commands: rework lxc_cmd_rsp_recv() to make it more obvious
authorChristian Brauner <christian.brauner@ubuntu.com>
Thu, 25 Feb 2021 09:48:14 +0000 (10:48 +0100)
committerChristian Brauner <christian.brauner@ubuntu.com>
Thu, 25 Feb 2021 20:28:44 +0000 (21:28 +0100)
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
src/lxc/commands.c
src/lxc/log.h

index 984f13550d1e174e97fcb0e5dcd4c20030c9d1fa..9090a8b9a61c1d3611e51dd9b7f12bd29d042faa 100644 (file)
@@ -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;
 }
 
 /*
index 3e3dafa9afd33f3fef489a4de5de0bd66e33aa3b..2c536aa98ce46a1261a2467b2cca8dc342888e36 100644 (file)
@@ -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__;                             \
        })