commands: rework lxc_cmd_rsp_recv() to make it more obvious

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;
 }
 
 /*

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__;                             \
 	})