From 1d4551a98d145db0efa43f7532c3700a5b949f0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Mon, 17 Jan 2022 16:55:02 +0100 Subject: [PATCH] pipewire: module-protocol-native: avoid file descriptor leaks At the moment, file descriptors may be leaked due to a malicious/buggy client: 1. If the control messages have been truncated, some file descriptors may still have been successfully transferred. Currently, seeing the MSG_CTRUNC bit causes `refill_buffer()` to immediately return -EPROTO without doing anything with the control messages, which may contain file descriptors. 2. When there is no truncation, it is still possible that the current batch of file descriptors causes the total file descriptor count to go over the maximum number of fds for the given buffer (currently 1024). In this case, too, `refill_buffer()` immediately returns -EPROTO without closing the file descriptors that can not be saved. Fix both of these cases by closing all file descriptors in all remaining cmsgs when one of the mentioned conditions occur. --- .../module-protocol-native/connection.c | 44 ++++++++++++++++--- 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/src/modules/module-protocol-native/connection.c b/src/modules/module-protocol-native/connection.c index 5df3ad031..77b3a8e33 100644 --- a/src/modules/module-protocol-native/connection.c +++ b/src/modules/module-protocol-native/connection.c @@ -175,10 +175,37 @@ static void handle_connection_error(struct pw_protocol_native_connection *conn, pw_log_error("connection %p: could not recvmsg on fd:%d: %s", conn, conn->fd, strerror(res)); } +static size_t cmsg_data_length(const struct cmsghdr *cmsg) +{ + const void *begin = CMSG_DATA(cmsg); + const void *end = SPA_PTROFF(cmsg, cmsg->cmsg_len, void); + + spa_assert(begin <= end); + + return SPA_PTRDIFF(end, begin); +} + +static void close_all_fds(struct msghdr *msg, struct cmsghdr *from) +{ + for (; from != NULL; from = CMSG_NXTHDR(msg, from)) { + if (from->cmsg_level != SOL_SOCKET || from->cmsg_type != SCM_RIGHTS) + continue; + + size_t n_fds = cmsg_data_length(from) / sizeof(int); + for (size_t i = 0; i < n_fds; i++) { + const void *p = SPA_PTROFF(CMSG_DATA(from), sizeof(int) * i, void); + int fd; + + memcpy(&fd, p, sizeof(fd)); + close(fd); + } + } +} + static int refill_buffer(struct pw_protocol_native_connection *conn, struct buffer *buf) { ssize_t len; - struct cmsghdr *cmsg; + struct cmsghdr *cmsg = NULL; struct msghdr msg = { 0 }; struct iovec iov[1]; char cmsgbuf[CMSG_SPACE(MAX_FDS_MSG * sizeof(int))]; @@ -198,7 +225,7 @@ static int refill_buffer(struct pw_protocol_native_connection *conn, struct buff while (true) { len = recvmsg(conn->fd, &msg, msg.msg_flags); if (msg.msg_flags & MSG_CTRUNC) - return -EPROTO; + goto cmsgs_truncated; if (len == 0 && avail != 0) return -EPIPE; else if (len < 0) { @@ -218,10 +245,9 @@ static int refill_buffer(struct pw_protocol_native_connection *conn, struct buff if (cmsg->cmsg_level != SOL_SOCKET || cmsg->cmsg_type != SCM_RIGHTS) continue; - n_fds = - (cmsg->cmsg_len - ((char *) CMSG_DATA(cmsg) - (char *) cmsg)) / sizeof(int); + n_fds = cmsg_data_length(cmsg) / sizeof(int); if (n_fds + buf->n_fds > MAX_FDS) - return -EPROTO; + goto too_many_fds; memcpy(&buf->fds[buf->n_fds], CMSG_DATA(cmsg), n_fds * sizeof(int)); buf->n_fds += n_fds; } @@ -234,6 +260,14 @@ static int refill_buffer(struct pw_protocol_native_connection *conn, struct buff recv_error: handle_connection_error(conn, errno); return -errno; + +cmsgs_truncated: + close_all_fds(&msg, CMSG_FIRSTHDR(&msg)); + return -EPROTO; + +too_many_fds: + close_all_fds(&msg, cmsg); + return -EPROTO; } static void clear_buffer(struct buffer *buf, bool fds)