From b3f35e021305afc1a40b9f3637c77595f38d6df2 Mon Sep 17 00:00:00 2001 From: Francis Deslauriers Date: Tue, 27 Jun 2017 17:21:35 -0400 Subject: [PATCH] Fix: kernel adds creds on recv with SO_PASSCRED unix socket option MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit When a Unix socket is configured with the SO_PASSCRED option, the caller of recv() will receive a credential control message even if the sender did not manually include it. This caused problem with the old implementation of the lttcomm_recv_fds_unix_sock function since it was expecting only one control message for the fd passing. With the SO_PASSCRED, the kernel will add a credential(SCM_CREDENTIALS) control message before the fd passing(SCM_RIGHTS) control message (function scm_recv() [1]). As a fix, make the receiver have a large enough before to include both types of message and ignore the credential control message. [1]: include/net/scm.h:111 Signed-off-by: Francis Deslauriers Signed-off-by: Jérémie Galarneau --- configure.ac | 4 +- src/common/Makefile.am | 56 +++++++++++++++------------ src/common/unix-stub.c | 86 ++++++++++++++++++++++++++++++++++++++++++ src/common/unix.c | 82 +++++++++++++++++++++++++++++----------- 4 files changed, 180 insertions(+), 48 deletions(-) create mode 100644 src/common/unix-stub.c diff --git a/configure.ac b/configure.ac index f5501cebd..b7688eb7e 100644 --- a/configure.ac +++ b/configure.ac @@ -867,6 +867,7 @@ build_lib_compat=no build_lib_consumer=no build_lib_hashtable=no build_lib_health=no +build_lib_unix=no build_lib_index=no build_lib_kernel_consumer=no build_lib_kernel_ctl=no @@ -923,7 +924,7 @@ AS_IF([test x$enable_bin_lttng_sessiond != xno], build_lib_relayd=yes build_lib_testpoint=yes build_lib_health=yes - build_lib_health=yes + build_lib_unix=yes ] ) @@ -1032,6 +1033,7 @@ AM_CONDITIONAL([BUILD_LIB_CONFIG], [test x$build_lib_config = xyes]) AM_CONDITIONAL([BUILD_LIB_CONSUMER], [test x$build_lib_consumer = xyes]) AM_CONDITIONAL([BUILD_LIB_HASHTABLE], [test x$build_lib_hashtable = xyes]) AM_CONDITIONAL([BUILD_LIB_HEALTH], [test x$build_lib_health = xyes]) +AM_CONDITIONAL([BUILD_LIB_UNIX], [test x$build_lib_unix = xyes]) AM_CONDITIONAL([BUILD_LIB_INDEX], [test x$build_lib_index = xyes]) AM_CONDITIONAL([BUILD_LIB_KERNEL_CONSUMER], [test x$build_lib_kernel_consumer = xyes]) AM_CONDITIONAL([BUILD_LIB_KERNEL_CTL], [test x$build_lib_kernel_ctl = xyes]) diff --git a/src/common/Makefile.am b/src/common/Makefile.am index 0b8043506..171748f43 100644 --- a/src/common/Makefile.am +++ b/src/common/Makefile.am @@ -7,6 +7,37 @@ SUBDIRS = string-utils DIST_SUBDIRS = compat health hashtable kernel-ctl sessiond-comm relayd \ kernel-consumer ust-consumer testpoint index config consumer \ string-utils +# +# Common library +noinst_LTLIBRARIES = libcommon.la +EXTRA_DIST = mi-lttng-3.0.xsd + +libcommon_la_SOURCES = error.h error.c utils.c utils.h runas.h runas.c \ + common.h futex.c futex.h uri.c uri.h defaults.c \ + pipe.c pipe.h readwrite.c readwrite.h \ + mi-lttng.h mi-lttng.c \ + daemonize.c daemonize.h \ + unix.h \ + filter.c filter.h context.c context.h \ + action.c notify.c condition.c buffer-usage.c \ + session-consumed-size.c \ + session-rotation.c \ + evaluation.c notification.c trigger.c endpoint.c \ + dynamic-buffer.h dynamic-buffer.c \ + buffer-view.h buffer-view.c \ + location.c \ + waiter.h waiter.c \ + userspace-probe.c + +libcommon_la_LIBADD = \ + $(top_builddir)/src/common/config/libconfig.la \ + $(UUID_LIBS) + +if BUILD_LIB_UNIX +libcommon_la_SOURCES += unix.c +else +libcommon_la_SOURCES += unix-stub.c +endif if BUILD_LIB_COMPAT SUBDIRS += compat @@ -60,31 +91,6 @@ noinst_HEADERS = lttng-kernel.h defaults.h macros.h error.h futex.h \ uri.h utils.h lttng-kernel-old.h \ align.h bitfield.h bug.h time.h -# Common library -noinst_LTLIBRARIES = libcommon.la -EXTRA_DIST = mi-lttng-3.0.xsd - -libcommon_la_SOURCES = error.h error.c utils.c utils.h runas.c runas.h \ - common.h futex.c futex.h uri.c uri.h defaults.c \ - pipe.c pipe.h readwrite.c readwrite.h \ - mi-lttng.h mi-lttng.c \ - daemonize.c daemonize.h \ - unix.c unix.h \ - filter.c filter.h context.c context.h \ - action.c notify.c condition.c buffer-usage.c \ - session-consumed-size.c \ - session-rotation.c \ - evaluation.c notification.c trigger.c endpoint.c \ - dynamic-buffer.h dynamic-buffer.c \ - buffer-view.h buffer-view.c \ - location.c \ - waiter.h waiter.c \ - userspace-probe.c - -libcommon_la_LIBADD = \ - $(top_builddir)/src/common/config/libconfig.la \ - $(UUID_LIBS) - all-local: @if [ x"$(srcdir)" != x"$(builddir)" ]; then \ for script in $(EXTRA_DIST); do \ diff --git a/src/common/unix-stub.c b/src/common/unix-stub.c new file mode 100644 index 000000000..5cc629645 --- /dev/null +++ b/src/common/unix-stub.c @@ -0,0 +1,86 @@ +/* + * Copyright (C) 2018 Francis Deslauriers + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License, version 2 only, as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program; if not, write to the Free Software Foundation, Inc., 51 + * Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#ifndef _UNIX_STUB_H +#define _UNIX_STUB_H + +#include +#include +#include + +int lttcomm_create_unix_sock(const char *pathname) +{ + return -1; +} +int lttcomm_create_anon_unix_socketpair(int *fds) +{ + return -1; +} +int lttcomm_connect_unix_sock(const char *pathname) +{ + return -1; +} +int lttcomm_accept_unix_sock(int sock) +{ + return -1; +} +int lttcomm_listen_unix_sock(int sock) +{ + return -1; +} +int lttcomm_close_unix_sock(int sock) +{ + return -1; +} +ssize_t lttcomm_send_fds_unix_sock(int sock, const int *fds, size_t nb_fd) +{ + return -1; +} +ssize_t lttcomm_recv_fds_unix_sock(int sock, int *fds, size_t nb_fd) +{ + return -1; +} +ssize_t lttcomm_recv_unix_sock(int sock, void *buf, size_t len) +{ + return -1; +} +ssize_t lttcomm_recv_unix_sock_non_block(int sock, void *buf, size_t len) +{ + return -1; +} +ssize_t lttcomm_send_unix_sock(int sock, const void *buf, size_t len) +{ + return -1; +} +ssize_t lttcomm_send_unix_sock_non_block(int sock, const void *buf, size_t len) +{ + return -1; +} +ssize_t lttcomm_send_creds_unix_sock(int sock, void *buf, size_t len) +{ + return -1; +} +ssize_t lttcomm_recv_creds_unix_sock(int sock, void *buf, size_t len, + lttng_sock_cred *creds) +{ + return -1; +} +int lttcomm_setsockopt_creds_unix_sock(int sock) +{ + return -1; +} +#endif /* _UNIX_STUB_H */ diff --git a/src/common/unix.c b/src/common/unix.c index 41525b691..3490af6e7 100644 --- a/src/common/unix.c +++ b/src/common/unix.c @@ -385,7 +385,7 @@ ssize_t lttcomm_send_fds_unix_sock(int sock, const int *fds, size_t nb_fd) char dummy = 0; memset(&msg, 0, sizeof(msg)); - memset(tmp, 0, CMSG_SPACE(sizeof_fds) * sizeof(char)); + memset(tmp, 0, sizeof(tmp)); if (nb_fd > LTTCOMM_MAX_SEND_FDS) return -EINVAL; @@ -397,6 +397,7 @@ ssize_t lttcomm_send_fds_unix_sock(int sock, const int *fds, size_t nb_fd) if (!cmptr) { return -1; } + cmptr->cmsg_level = SOL_SOCKET; cmptr->cmsg_type = SCM_RIGHTS; cmptr->cmsg_len = CMSG_LEN(sizeof_fds); @@ -439,7 +440,9 @@ ssize_t lttcomm_recv_fds_unix_sock(int sock, int *fds, size_t nb_fd) ssize_t ret = 0; struct cmsghdr *cmsg; size_t sizeof_fds = nb_fd * sizeof(int); - char recv_fd[CMSG_SPACE(sizeof_fds)]; + + /* Account for the struct ucred cmsg in the buffer size */ + char recv_buf[CMSG_SPACE(sizeof_fds) + CMSG_SPACE(sizeof(struct ucred))]; struct msghdr msg; char dummy; @@ -450,8 +453,15 @@ ssize_t lttcomm_recv_fds_unix_sock(int sock, int *fds, size_t nb_fd) iov[0].iov_len = 1; msg.msg_iov = iov; msg.msg_iovlen = 1; - msg.msg_control = recv_fd; - msg.msg_controllen = sizeof(recv_fd); + + cmsg = (struct cmsghdr *) recv_buf; + cmsg->cmsg_len = CMSG_LEN(sizeof_fds); + cmsg->cmsg_level = SOL_SOCKET; + cmsg->cmsg_type = SCM_RIGHTS; + + msg.msg_control = cmsg; + msg.msg_controllen = CMSG_LEN(sizeof(recv_buf)); + msg.msg_flags = 0; do { ret = recvmsg(sock, &msg, 0); @@ -460,35 +470,63 @@ ssize_t lttcomm_recv_fds_unix_sock(int sock, int *fds, size_t nb_fd) PERROR("recvmsg fds"); goto end; } + if (ret != 1) { fprintf(stderr, "Error: Received %zd bytes, expected %d\n", ret, 1); goto end; } + if (msg.msg_flags & MSG_CTRUNC) { fprintf(stderr, "Error: Control message truncated.\n"); ret = -1; goto end; } - cmsg = CMSG_FIRSTHDR(&msg); - if (!cmsg) { - fprintf(stderr, "Error: Invalid control message header\n"); - ret = -1; - goto end; - } - if (cmsg->cmsg_level != SOL_SOCKET || cmsg->cmsg_type != SCM_RIGHTS) { - fprintf(stderr, "Didn't received any fd\n"); - ret = -1; - goto end; - } - if (cmsg->cmsg_len != CMSG_LEN(sizeof_fds)) { - fprintf(stderr, "Error: Received %zu bytes of ancillary data, expected %zu\n", - (size_t) cmsg->cmsg_len, (size_t) CMSG_LEN(sizeof_fds)); - ret = -1; - goto end; + + /* + * If the socket was configured with SO_PASSCRED, the kernel will add a + * control message (cmsg) to the ancillary data of the unix socket. We + * need to expect a cmsg of the SCM_CREDENTIALS as the first control + * message. + */ + for (cmsg = CMSG_FIRSTHDR(&msg); cmsg != NULL; cmsg = CMSG_NXTHDR(&msg, cmsg)) { + if (!cmsg) { + fprintf(stderr, "Error: Invalid control message header\n"); + ret = -1; + goto end; + } + if (cmsg->cmsg_level != SOL_SOCKET) { + fprintf(stderr, "Error: The socket needs to be of type SOL_SOCKET\n"); + ret = -1; + goto end; + } + if (cmsg->cmsg_type == SCM_RIGHTS) { + /* + * We found the controle message for file descriptors, + * now copy the fds to the fds ptr and return success. + */ + if (cmsg->cmsg_len != CMSG_LEN(sizeof_fds)) { + fprintf(stderr, "Error: Received %zu bytes of" + "ancillary data for FDs, expected %zu\n", + (size_t) cmsg->cmsg_len, + (size_t) CMSG_LEN(sizeof_fds)); + ret = -1; + goto end; + } + memcpy(fds, CMSG_DATA(cmsg), sizeof_fds); + ret = sizeof_fds; + goto end; + } + if (cmsg->cmsg_type == SCM_CREDENTIALS) { + /* + * Expect credentials to be sent when expecting fds even + * if no credential were include in the send(). The + * kernel adds them... + */ + fprintf(stderr, "Received creds... continuing\n"); + ret = -1; + } } - memcpy(fds, CMSG_DATA(cmsg), sizeof_fds); - ret = sizeof_fds; end: return ret; } -- 2.34.1