[lttng-dev] [PATCH lttng-ust 1/2] WIP: Force tracked fd to be bigger than STDERR_FILENO

Jonathan Rajotte jonathan.rajotte-julien at efficios.com
Thu Feb 1 20:55:23 UTC 2018


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 at efficios.com>
---
 include/ust-fd.h                         |  2 +-
 liblttng-ust-comm/lttng-ust-comm.c       | 51 +++++++++++++++---
 liblttng-ust-comm/lttng-ust-fd-tracker.c | 90 +++++++++++++++++++++++++++++++-
 liblttng-ust/lttng-ust-comm.c            | 48 ++++++++++++++---
 liblttng-ust/lttng-ust-elf.c             | 18 +++++--
 5 files changed, 190 insertions(+), 19 deletions(-)

diff --git a/include/ust-fd.h b/include/ust-fd.h
index 032cac5..a015525 100644
--- a/include/ust-fd.h
+++ b/include/ust-fd.h
@@ -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);
diff --git a/liblttng-ust-comm/lttng-ust-comm.c b/liblttng-ust-comm/lttng-ust-comm.c
index 9fe6d28..3d1c650 100644
--- a/liblttng-ust-comm/lttng-ust-comm.c
+++ b/liblttng-ust-comm/lttng-ust-comm.c
@@ -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;
 
diff --git a/liblttng-ust-comm/lttng-ust-fd-tracker.c b/liblttng-ust-comm/lttng-ust-fd-tracker.c
index 8d2acb6..c2b3e3d 100644
--- a/liblttng-ust-comm/lttng-ust-fd-tracker.c
+++ b/liblttng-ust-comm/lttng-ust-fd-tracker.c
@@ -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,110 @@ 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 enable us to bypass problems that
+ * can be encounter if UST uses stdin, stdout, stderr fds for internal use
+ * (daemon etc.). 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;
 }
 
 /*
diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c
index 511b9cf..32ba3c1 100644
--- a/liblttng-ust/lttng-ust-comm.c
+++ b/liblttng-ust/lttng-ust-comm.c
@@ -1261,7 +1261,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,
@@ -1353,7 +1364,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();
@@ -1433,9 +1444,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();
 	/*
@@ -1504,9 +1527,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();
 	/*
diff --git a/liblttng-ust/lttng-ust-elf.c b/liblttng-ust/lttng-ust-elf.c
index f2c0982..c073e7a 100644
--- a/liblttng-ust/lttng-ust-elf.c
+++ b/liblttng-ust/lttng-ust-elf.c
@@ -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) {
-- 
2.7.4



More information about the lttng-dev mailing list