[lttng-dev] [PATCH lttng-ust 1/2] Force tracked fd to be bigger than STDERR_FILENO
Jonathan Rajotte
jonathan.rajotte-julien at efficios.com
Thu Feb 1 22:28:11 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