Force tracked fd to be bigger than STDERR_FILENO
authorJonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Mon, 5 Feb 2018 22:58:18 +0000 (17:58 -0500)
committerMathieu Desnoyers <mathieu.desnoyers@efficios.com>
Tue, 6 Feb 2018 13:25:27 +0000 (08:25 -0500)
This allow ust to be proactive regarding std* fd manipulation done by
external source.

A good example of this is the "daemon" function that can dup2 statically
the std* fd and close them silently if the were already used.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
include/ust-fd.h
liblttng-ust-comm/lttng-ust-comm.c
liblttng-ust-comm/lttng-ust-fd-tracker.c
liblttng-ust/lttng-ust-comm.c
liblttng-ust/lttng-ust-elf.c

index 032cac538412d620b379cd77bfcaf6f76ea06885..a0155251f4a55a2918e078880e3682dc92a47dc8 100644 (file)
@@ -27,7 +27,7 @@
 #include <stdio.h>
 
 void lttng_ust_init_fd_tracker(void);
-void lttng_ust_add_fd_to_tracker(int fd);
+int lttng_ust_add_fd_to_tracker(int fd);
 void lttng_ust_delete_fd_from_tracker(int fd);
 void lttng_ust_lock_fd_tracker(void);
 void lttng_ust_unlock_fd_tracker(void);
index 9fe6d288d633a2ec67c8d78a82cf90d2e52dc2c2..3d1c65038295062559db6dd415fe2b03304d5ec2 100644 (file)
@@ -598,7 +598,7 @@ ssize_t ustcomm_recv_channel_from_sessiond(int sock,
 {
        void *chan_data;
        ssize_t len, nr_fd;
-       int wakeup_fd;
+       int wakeup_fd, ret;
 
        if (var_len > LTTNG_UST_CHANNEL_DATA_MAX_LEN) {
                len = -EINVAL;
@@ -627,9 +627,21 @@ ssize_t ustcomm_recv_channel_from_sessiond(int sock,
                        goto error_recv;
                }
        }
-       *_wakeup_fd = wakeup_fd;
-       lttng_ust_add_fd_to_tracker(wakeup_fd);
+
+       ret = lttng_ust_add_fd_to_tracker(wakeup_fd);
+       if (ret < 0) {
+               lttng_ust_unlock_fd_tracker();
+               ret = close(wakeup_fd);
+               if (ret) {
+                       PERROR("close on wakeup_fd");
+               }
+               len = -EIO;
+               goto error_recv;
+       }
+
+       *_wakeup_fd = ret;
        lttng_ust_unlock_fd_tracker();
+
        *_chan_data = chan_data;
        return len;
 
@@ -661,10 +673,35 @@ int ustcomm_recv_stream_from_sessiond(int sock,
                        goto error;
                }
        }
-       *shm_fd = fds[0];
-       *wakeup_fd = fds[1];
-       lttng_ust_add_fd_to_tracker(fds[0]);
-       lttng_ust_add_fd_to_tracker(fds[1]);
+
+       ret = lttng_ust_add_fd_to_tracker(fds[0]);
+       if (ret < 0) {
+               lttng_ust_unlock_fd_tracker();
+               ret = close(fds[0]);
+               if (ret) {
+                       PERROR("close on received shm_fd");
+               }
+               ret = -EIO;
+               goto error;
+       }
+       *shm_fd = ret;
+
+       ret = lttng_ust_add_fd_to_tracker(fds[1]);
+       if (ret < 0) {
+               lttng_ust_unlock_fd_tracker();
+               ret = close(*shm_fd);
+               if (ret) {
+                       PERROR("close on shm_fd");
+               }
+               *shm_fd = -1;
+               ret = close(fds[1]);
+               if (ret) {
+                       PERROR("close on received wakeup_fd");
+               }
+               ret = -EIO;
+               goto error;
+       }
+       *wakeup_fd = ret;
        lttng_ust_unlock_fd_tracker();
        return 0;
 
index 8d2acb69f55f5cb32b7841ba7dafcdb96064326b..329339f98a038736fe528bc856bdeff1737b4c2b 100644 (file)
@@ -47,6 +47,7 @@
 #define IS_FD_VALID(fd)                        ((fd) >= 0 && (fd) < lttng_ust_max_fd)
 #define GET_FD_SET_FOR_FD(fd, fd_sets) (&((fd_sets)[(fd) / FD_SETSIZE]))
 #define CALC_INDEX_TO_SET(fd)          ((fd) % FD_SETSIZE)
+#define IS_FD_STD(fd)                  (IS_FD_VALID(fd) && (fd) <= STDERR_FILENO)
 
 /* Check fd validity before calling these. */
 #define ADD_FD_TO_SET(fd, fd_sets)     \
@@ -143,25 +144,112 @@ void lttng_ust_unlock_fd_tracker(void)
        URCU_TLS(thread_fd_tracking) = 0;
 }
 
+static int dup_std_fd(int fd)
+{
+       int ret;
+       int fd_to_close[STDERR_FILENO + 1];
+       int fd_to_close_count = 0;
+       int dup_cmd = F_DUPFD; /* Default command */
+       int fd_valid = -1;
+
+       if (!(IS_FD_STD(fd))) {
+               /* Should not be here */
+               ret = -1;
+               goto error;
+       }
+
+       /* Check for FD_CLOEXEC flag */
+       ret = fcntl(fd, F_GETFD);
+       if (ret < 0) {
+               PERROR("fcntl on f_getfd");
+               ret = -1;
+               goto error;
+       }
+
+       if (ret & FD_CLOEXEC) {
+               dup_cmd = F_DUPFD_CLOEXEC;
+       }
+
+       /* Perform dup */
+       for (int i = 0; i < STDERR_FILENO + 1; i++) {
+               ret = fcntl(fd, dup_cmd, 0);
+               if (ret < 0) {
+                       PERROR("fcntl dup fd");
+                       goto error;
+               }
+
+               if (!(IS_FD_STD(ret))) {
+                       /* fd is outside of STD range, use it. */
+                       fd_valid = ret;
+                       /* Close fd received as argument. */
+                       fd_to_close[i] = fd;
+                       fd_to_close_count++;
+                       break;
+               }
+
+               fd_to_close[i] = ret;
+               fd_to_close_count++;
+       }
+
+       /* Close intermediary fds */
+       for (int i = 0; i < fd_to_close_count; i++) {
+               ret = close(fd_to_close[i]);
+               if (ret) {
+                       PERROR("close on temporary fd: %d.", fd_to_close[i]);
+                       /*
+                        * Not using an abort here would yield a complicated
+                        * error handling for the caller. If a failure occurs
+                        * here, the system is already in a bad state.
+                        */
+                       abort();
+               }
+       }
+
+       ret = fd_valid;
+error:
+       return ret;
+}
+
 /*
  * Needs to be called with ust_safe_guard_fd_mutex held when opening the fd.
  * Has strict checking of fd validity.
+ *
+ * If fd <= 2, dup the fd until fd > 2. This enables us to bypass
+ * problems that can be encountered if UST uses stdin, stdout, stderr
+ * fds for internal use (daemon etc.). This can happen if the
+ * application closes either of those file descriptors. Intermediary fds
+ * are closed as needed.
+ *
+ * Return -1 on error.
+ *
  */
-void lttng_ust_add_fd_to_tracker(int fd)
+int lttng_ust_add_fd_to_tracker(int fd)
 {
+       int ret;
        /*
         * Ensure the tracker is initialized when called from
         * constructors.
         */
        lttng_ust_init_fd_tracker();
-
        assert(URCU_TLS(thread_fd_tracking));
+
+       if (IS_FD_STD(fd)) {
+               ret = dup_std_fd(fd);
+               if (ret < 0) {
+                       goto error;
+               }
+               fd = ret;
+       }
+
        /* Trying to add an fd which we can not accommodate. */
        assert(IS_FD_VALID(fd));
        /* Setting an fd thats already set. */
        assert(!IS_FD_SET(fd, lttng_fd_set));
 
        ADD_FD_TO_SET(fd, lttng_fd_set);
+       return fd;
+error:
+       return ret;
 }
 
 /*
index 776ad33290b2aa8b93660acf686d40a51e8c712d..e92bb870d4d58abcd2874ab80e9e7e56525a591d 100644 (file)
@@ -1247,7 +1247,18 @@ char *get_map_shm(struct sock_info *sock_info)
                lttng_ust_unlock_fd_tracker();
                goto error;
        }
-       lttng_ust_add_fd_to_tracker(wait_shm_fd);
+
+       ret = lttng_ust_add_fd_to_tracker(wait_shm_fd);
+       if (ret < 0) {
+               ret = close(wait_shm_fd);
+               if (!ret) {
+                       PERROR("Error closing fd");
+               }
+               lttng_ust_unlock_fd_tracker();
+               goto error;
+       }
+
+       wait_shm_fd = ret;
        lttng_ust_unlock_fd_tracker();
 
        wait_shm_mmap = mmap(NULL, page_size, PROT_READ,
@@ -1339,7 +1350,7 @@ static
 void *ust_listener_thread(void *arg)
 {
        struct sock_info *sock_info = arg;
-       int sock, ret, prev_connect_failed = 0, has_waited = 0;
+       int sock, ret, prev_connect_failed = 0, has_waited = 0, fd;
        long timeout;
 
        lttng_ust_fixup_tls();
@@ -1419,9 +1430,21 @@ restart:
                ust_unlock();
                goto restart;
        }
-       lttng_ust_add_fd_to_tracker(ret);
-       lttng_ust_unlock_fd_tracker();
+       fd = ret;
+       ret = lttng_ust_add_fd_to_tracker(fd);
+       if (ret < 0) {
+               ret = close(fd);
+               if (ret) {
+                       PERROR("close on sock_info->socket");
+               }
+               ret = -1;
+               lttng_ust_unlock_fd_tracker();
+               ust_unlock();
+               goto quit;
+       }
+
        sock_info->socket = ret;
+       lttng_ust_unlock_fd_tracker();
 
        ust_unlock();
        /*
@@ -1490,9 +1513,22 @@ restart:
                ust_unlock();
                goto restart;
        }
-       lttng_ust_add_fd_to_tracker(ret);
-       lttng_ust_unlock_fd_tracker();
+
+       fd = ret;
+       ret = lttng_ust_add_fd_to_tracker(fd);
+       if (ret < 0) {
+               ret = close(fd);
+               if (ret) {
+                       PERROR("close on sock_info->notify_socket");
+               }
+               ret = -1;
+               lttng_ust_unlock_fd_tracker();
+               ust_unlock();
+               goto quit;
+       }
+
        sock_info->notify_socket = ret;
+       lttng_ust_unlock_fd_tracker();
 
        ust_unlock();
        /*
index f2c098297acab8955df876f01f97d2a738dc0a81..c073e7a545b64d349d3d7bb56932a24b9025f657 100644 (file)
@@ -243,6 +243,7 @@ struct lttng_ust_elf *lttng_ust_elf_create(const char *path)
        uint8_t e_ident[EI_NIDENT];
        struct lttng_ust_elf_shdr *section_names_shdr;
        struct lttng_ust_elf *elf = NULL;
+       int ret, fd;
 
        elf = zmalloc(sizeof(struct lttng_ust_elf));
        if (!elf) {
@@ -256,12 +257,23 @@ struct lttng_ust_elf *lttng_ust_elf_create(const char *path)
        }
 
        lttng_ust_lock_fd_tracker();
-       elf->fd = open(elf->path, O_RDONLY | O_CLOEXEC);
-       if (elf->fd < 0) {
+       fd = open(elf->path, O_RDONLY | O_CLOEXEC);
+       if (fd < 0) {
                lttng_ust_unlock_fd_tracker();
                goto error;
        }
-       lttng_ust_add_fd_to_tracker(elf->fd);
+
+       ret = lttng_ust_add_fd_to_tracker(fd);
+       if (ret < 0) {
+               ret = close(fd);
+               if (ret) {
+                       PERROR("close on elf->fd");
+               }
+               ret = -1;
+               lttng_ust_unlock_fd_tracker();
+               goto error;
+       }
+       elf->fd = ret;
        lttng_ust_unlock_fd_tracker();
 
        if (lttng_ust_read(elf->fd, e_ident, EI_NIDENT) < EI_NIDENT) {
This page took 0.030934 seconds and 4 git commands to generate.