From 780215cf0d9d02c7d022f18c1bf7aaed509d5835 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Thu, 25 Feb 2021 11:18:09 +0100 Subject: [PATCH] af_unix: allow caller and callee to negotiate expectations and reality Signed-off-by: Christian Brauner --- src/lxc/af_unix.c | 57 +++++++++++++++++++++++++++++++--- src/lxc/af_unix.h | 79 +++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 128 insertions(+), 8 deletions(-) diff --git a/src/lxc/af_unix.c b/src/lxc/af_unix.c index 747e68820..f1d36c5dd 100644 --- a/src/lxc/af_unix.c +++ b/src/lxc/af_unix.c @@ -113,7 +113,7 @@ int lxc_abstract_unix_connect(const char *path) } int lxc_abstract_unix_send_fds_iov(int fd, const int *sendfds, int num_sendfds, - struct iovec *iov, size_t iovlen) + struct iovec *const iov, size_t iovlen) { __do_free char *cmsgbuf = NULL; int ret; @@ -176,6 +176,12 @@ static ssize_t lxc_abstract_unix_recv_fds_iov(int fd, size_t cmsgbufsize = CMSG_SPACE(sizeof(struct ucred)) + CMSG_SPACE(ret_fds->fd_count_max * sizeof(int)); + if (ret_fds->flags & ~UNIX_FDS_ACCEPT_MASK) + return ret_errno(EINVAL); + + if (hweight32((ret_fds->flags & ~UNIX_FDS_ACCEPT_NONE)) > 1) + return ret_errno(EINVAL); + cmsgbuf = zalloc(cmsgbufsize); if (!cmsgbuf) return ret_errno(ENOMEM); @@ -202,7 +208,7 @@ again: if (cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SCM_RIGHTS) { __u32 idx; /* - * This causes some compilers to complaing about + * This causes some compilers to complain about * increased alignment requirements but I haven't found * a better way to deal with this yet. Suggestions * welcome! @@ -225,7 +231,22 @@ again: return syserrno_set(-EFBIG, "Received excessive number of file descriptors"); } + if (msg.msg_flags & MSG_CTRUNC) { + for (idx = 0; idx < num_raw; idx++) + close(fds_raw[idx]); + + return syserrno_set(-EFBIG, "Control message was truncated; closing all fds and rejecting incomplete message"); + } + if (ret_fds->fd_count_max > num_raw) { + if (!(ret_fds->flags & UNIX_FDS_ACCEPT_LESS)) { + for (idx = 0; idx < num_raw; idx++) + close(fds_raw[idx]); + + return syserrno_set(-EINVAL, "Received fewer file descriptors than we expected %u != %u", + ret_fds->fd_count_max, num_raw); + } + /* * Make sure any excess entries in the fd array * are set to -EBADF so our cleanup functions @@ -234,16 +255,33 @@ again: for (idx = num_raw; idx < ret_fds->fd_count_max; idx++) ret_fds->fd[idx] = -EBADF; - WARN("Received fewer file descriptors than we expected %u != %u", ret_fds->fd_count_max, num_raw); + ret_fds->flags |= UNIX_FDS_RECEIVED_LESS; } else if (ret_fds->fd_count_max < num_raw) { + if (!(ret_fds->flags & UNIX_FDS_ACCEPT_MORE)) { + for (idx = 0; idx < num_raw; idx++) + close(fds_raw[idx]); + + return syserrno_set(-EINVAL, "Received more file descriptors than we expected %u != %u", + ret_fds->fd_count_max, num_raw); + } + /* Make sure we close any excess fds we received. */ for (idx = ret_fds->fd_count_max; idx < num_raw; idx++) close(fds_raw[idx]); - WARN("Received more file descriptors than we expected %u != %u", ret_fds->fd_count_max, num_raw); - /* Cap the number of received file descriptors. */ num_raw = ret_fds->fd_count_max; + ret_fds->flags |= UNIX_FDS_RECEIVED_MORE; + } else { + ret_fds->flags |= UNIX_FDS_RECEIVED_EXACT; + } + + if (hweight32((ret_fds->flags & ~UNIX_FDS_ACCEPT_MASK)) > 1) { + for (idx = 0; idx < num_raw; idx++) + close(fds_raw[idx]); + + return syserrno_set(-EINVAL, "Invalid flag combination; closing to not risk leaking fds %u != %u", + ret_fds->fd_count_max, num_raw); } memcpy(ret_fds->fd, CMSG_DATA(cmsg), num_raw * sizeof(int)); @@ -252,6 +290,15 @@ again: } } + if (ret_fds->fd_count_ret == 0) { + ret_fds->flags |= UNIX_FDS_RECEIVED_NONE; + + /* We expected to receive file descriptors. */ + if ((ret_fds->flags & UNIX_FDS_ACCEPT_MASK) && + !(ret_fds->flags & UNIX_FDS_ACCEPT_NONE)) + return syserrno_set(-EINVAL, "Received no file descriptors"); + } + return ret; } diff --git a/src/lxc/af_unix.h b/src/lxc/af_unix.h index d8a9ad9ca..7b9793743 100644 --- a/src/lxc/af_unix.h +++ b/src/lxc/af_unix.h @@ -12,15 +12,88 @@ #include "macro.h" #include "memory_utils.h" +#define KERNEL_SCM_MAX_FD 253 + +/* Allow the caller to set expectations. */ + +/* + * UNIX_FDS_ACCEPT_EXACT will only succeed if the exact amount of fds has been + * received (unless combined with UNIX_FDS_ACCEPT_NONE). + */ +#define UNIX_FDS_ACCEPT_EXACT ((__u32)(1 << 0)) /* default */ + +/* + * UNIX_FDS_ACCEPT_LESS will also succeed if less than the requested number of + * fd has been received. If the UNIX_FDS_ACCEPT_NONE flag is not raised than at + * least one fd must be received. + * */ +#define UNIX_FDS_ACCEPT_LESS ((__u32)(1 << 1)) + +/* + * UNIX_FDS_ACCEPT_MORE will also succeed if more than the requested number of + * fds have been received. Any additional fds will be silently closed. If the + * UNIX_FDS_ACCEPT_NONE flag is not raised than at least one fd must be + * received. + */ +#define UNIX_FDS_ACCEPT_MORE ((__u32)(1 << 2)) /* wipe any extra fds */ + /* - * Technically 253 is the kernel limit but we want to the struct to be a - * multiple of 8. + * UNIX_FDS_ACCEPT_NONE can be specified with any of the above flags and + * indicates that the caller will accept no file descriptors to be received. */ -#define KERNEL_SCM_MAX_FD 252 +#define UNIX_FDS_ACCEPT_NONE ((__u32)(1 << 3)) +/* UNIX_FDS_ACCEPT_MASK is the value of all the above flags or-ed together. */ +#define UNIX_FDS_ACCEPT_MASK (UNIX_FDS_ACCEPT_EXACT | UNIX_FDS_ACCEPT_LESS | UNIX_FDS_ACCEPT_MORE | UNIX_FDS_ACCEPT_NONE) + +/* Allow the callee to communicate reality. */ + +/* UNIX_FDS_RECEIVED_EXACT indicates that the exact number of fds was received. */ +#define UNIX_FDS_RECEIVED_EXACT ((__u32)(1 << 16)) + +/* + * UNIX_FDS_RECEIVED_LESS indicates that less than the requested number of fd + * has been received. + */ +#define UNIX_FDS_RECEIVED_LESS ((__u32)(1 << 17)) + +/* + * UNIX_FDS_RECEIVED_MORE indicates that more than the requested number of fd + * has been received. + */ +#define UNIX_FDS_RECEIVED_MORE ((__u32)(1 << 18)) + +/* UNIX_FDS_RECEIVED_NONE indicates that no fds have been received. */ +#define UNIX_FDS_RECEIVED_NONE ((__u32)(1 << 19)) + +/** + * Defines a generic struct to receive file descriptors from unix sockets. + * @fd_count_max : Either the exact or maximum number of file descriptors the + * caller is willing to accept. Must be smaller than + * KERNEL_SCM_MAX_FDs; larger values will be rejected. + * Filled in by the caller. + * @fd_count_ret : The actually received number of file descriptors. + * Filled in by the callee. + * @flags : Flags to negotiate expectations about the number of file + * descriptors to receive. + * Filled in by the caller and callee. The caller's flag space + * is UNIX_FDS_ACCEPT_* other values will be rejected. The + * caller may only set one of {EXACT, LESS, MORE}. In addition + * they can raise the NONE flag. Any combination of {EXACT, + * LESS, MORE} will be rejected. + * The callee's flag space is UNIX_FDS_RECEIVED_*. Only ever + * one of those values will be set. + * @fd : Array to store received file descriptors into. Filled by the + * callee on success. If less file descriptors are received + * than requested in @fd_count_max the callee will ensure that + * all additional slots will be set to -EBADF. Nonetheless, the + * caller should only ever use @fd_count_ret to iterate through + * @fd after a successful receive. + */ struct unix_fds { __u32 fd_count_max; __u32 fd_count_ret; + __u32 flags; __s32 fd[KERNEL_SCM_MAX_FD]; } __attribute__((aligned(8))); -- 2.47.2