From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Date: Wed, 9 Mar 2022 16:54:33 +0000 (-0500)
Subject: Fix: concurrent exec(2) file descriptor leak
X-Git-Tag: v2.12.4~2
X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=37cdae11082d4fcaa10488f4dba452273113cad3;p=lttng-ust.git

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 <mathieu.desnoyers@efficios.com>
Change-Id: Id2167cf99d7cb8a8425fc0dc13745f023a504562
---

diff --git a/liblttng-ust-comm/lttng-ust-comm.c b/liblttng-ust-comm/lttng-ust-comm.c
index 47b58cc4..5976f633 100644
--- a/liblttng-ust-comm/lttng-ust-comm.c
+++ b/liblttng-ust-comm/lttng-ust-comm.c
@@ -108,9 +108,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;
@@ -125,12 +124,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;
@@ -158,7 +151,6 @@ int ustcomm_connect_unix_sock(const char *pathname, long timeout)
 	return fd;
 
 error_connect:
-error_fcntl:
 	{
 		int closeret;
 
@@ -454,7 +446,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));
 
@@ -467,7 +458,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) {
@@ -513,15 +504,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;
diff --git a/libringbuffer/shm.c b/libringbuffer/shm.c
index 7a41d4de..00b3d129 100644
--- a/libringbuffer/shm.c
+++ b/libringbuffer/shm.c
@@ -108,18 +108,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) {
@@ -201,18 +194,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) {
@@ -373,11 +359,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) {