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/src/common/ringbuffer/shm.c b/src/common/ringbuffer/shm.c
index a3235b6961ad6e0b6c13677c84bd78bb5207b954..a1ef3d69f0602544f9335a9f1d59843e14208c55 100644 (file)
--- 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 4c1fb0efb1da750a549af9e19f5382e14e6178f6..f391399ff3f1b46aedf2ff958c0d3ea101563803 100644 (file)
--- 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;