From 8a8c2117fb26a42b19975b8745d4333c7aa1a061 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Wed, 9 Mar 2022 11:54:33 -0500 Subject: [PATCH] Fix: concurrent exec(2) file descriptor leak If exec(2) is executed by the application concurrently with LTTng-UST listener threads between the creation of a file descriptor with socket(2), recvmsg(2), or pipe(2) and call to fcntl(3) FD_CLOEXEC, those file descriptors will stay open after the exec, which is not intended. As a consequence, shared memory files for ring buffers can stay present on the file system for long-running traced processes. Use: - pipe2(2) O_CLOEXEC (supported since Linux 2.6.27, and by FreeBSD), - socket(2) SOCK_CLOEXEC (supported since Linux 2.6.27, and by FreeBSD), - recvmsg(2) MSG_CMSG_CLOEXEC (supported since Linux 2.6.23 and by FreeBSD), rather than fcntl(2) FD_CLOEXEC to make sure the file descriptors are closed on exec immediately upon their creation. Signed-off-by: Mathieu Desnoyers Change-Id: Id2167cf99d7cb8a8425fc0dc13745f023a504562 --- src/common/ringbuffer/shm.c | 23 ++--------------------- src/common/ustcomm.c | 22 ++-------------------- 2 files changed, 4 insertions(+), 41 deletions(-) diff --git a/src/common/ringbuffer/shm.c b/src/common/ringbuffer/shm.c index a3235b69..a1ef3d69 100644 --- a/src/common/ringbuffer/shm.c +++ b/src/common/ringbuffer/shm.c @@ -97,18 +97,11 @@ struct shm_object *_shm_object_table_alloc_shm(struct shm_object_table *table, obj = &table->objects[table->allocated_len]; /* wait_fd: create pipe */ - ret = pipe(waitfd); + ret = pipe2(waitfd, O_CLOEXEC); if (ret < 0) { PERROR("pipe"); goto error_pipe; } - for (i = 0; i < 2; i++) { - ret = fcntl(waitfd[i], F_SETFD, FD_CLOEXEC); - if (ret < 0) { - PERROR("fcntl"); - goto error_fcntl; - } - } /* The write end of the pipe needs to be non-blocking */ ret = fcntl(waitfd[1], F_SETFL, O_NONBLOCK); if (ret < 0) { @@ -200,18 +193,11 @@ struct shm_object *_shm_object_table_alloc_mem(struct shm_object_table *table, goto alloc_error; /* wait_fd: create pipe */ - ret = pipe(waitfd); + ret = pipe2(waitfd, O_CLOEXEC); if (ret < 0) { PERROR("pipe"); goto error_pipe; } - for (i = 0; i < 2; i++) { - ret = fcntl(waitfd[i], F_SETFD, FD_CLOEXEC); - if (ret < 0) { - PERROR("fcntl"); - goto error_fcntl; - } - } /* The write end of the pipe needs to be non-blocking */ ret = fcntl(waitfd[1], F_SETFL, O_NONBLOCK); if (ret < 0) { @@ -380,11 +366,6 @@ struct shm_object *shm_object_table_append_mem(struct shm_object_table *table, obj->shm_fd = -1; obj->shm_fd_ownership = 0; - ret = fcntl(obj->wait_fd[1], F_SETFD, FD_CLOEXEC); - if (ret < 0) { - PERROR("fcntl"); - goto error_fcntl; - } /* The write end of the pipe needs to be non-blocking */ ret = fcntl(obj->wait_fd[1], F_SETFL, O_NONBLOCK); if (ret < 0) { diff --git a/src/common/ustcomm.c b/src/common/ustcomm.c index 4c1fb0ef..f391399f 100644 --- a/src/common/ustcomm.c +++ b/src/common/ustcomm.c @@ -60,9 +60,8 @@ int ustcomm_connect_unix_sock(const char *pathname, long timeout) /* * libust threads require the close-on-exec flag for all * resources so it does not leak file descriptors upon exec. - * SOCK_CLOEXEC is not used since it is linux specific. */ - fd = socket(PF_UNIX, SOCK_STREAM, 0); + fd = socket(PF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0); if (fd < 0) { PERROR("socket"); ret = -errno; @@ -77,12 +76,6 @@ int ustcomm_connect_unix_sock(const char *pathname, long timeout) WARN("Error setting connect socket send timeout"); } } - ret = fcntl(fd, F_SETFD, FD_CLOEXEC); - if (ret < 0) { - PERROR("fcntl"); - ret = -errno; - goto error_fcntl; - } memset(&sun, 0, sizeof(sun)); sun.sun_family = AF_UNIX; @@ -110,7 +103,6 @@ int ustcomm_connect_unix_sock(const char *pathname, long timeout) return fd; error_connect: -error_fcntl: { int closeret; @@ -416,7 +408,6 @@ ssize_t ustcomm_recv_fds_unix_sock(int sock, int *fds, size_t nb_fd) char recv_fd[CMSG_SPACE(sizeof_fds)]; struct msghdr msg; char dummy; - int i; memset(&msg, 0, sizeof(msg)); @@ -429,7 +420,7 @@ ssize_t ustcomm_recv_fds_unix_sock(int sock, int *fds, size_t nb_fd) msg.msg_controllen = sizeof(recv_fd); do { - ret = recvmsg(sock, &msg, 0); + ret = recvmsg(sock, &msg, MSG_CMSG_CLOEXEC); } while (ret < 0 && errno == EINTR); if (ret < 0) { if (errno != EPIPE && errno != ECONNRESET) { @@ -475,15 +466,6 @@ ssize_t ustcomm_recv_fds_unix_sock(int sock, int *fds, size_t nb_fd) memcpy(fds, CMSG_DATA(cmsg), sizeof_fds); - /* Set FD_CLOEXEC */ - for (i = 0; i < nb_fd; i++) { - ret = fcntl(fds[i], F_SETFD, FD_CLOEXEC); - if (ret < 0) { - PERROR("fcntl failed to set FD_CLOEXEC on fd %d", - fds[i]); - } - } - ret = nb_fd; end: return ret; -- 2.34.1