Fix: Connect timeout arithmetic in inet/inet6 (v4)

The nanoseconds part of the timespec struct time_a is not always
bigger than time_b since it wraps around each second.

Use 64-bit arithmetic to compute the difference.

Merge/move duplicated code into utils.c.

This function is really doing two things. Split it into
timespec_to_ms() and timespec_abs_diff().

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

diff --git a/src/common/common.h b/src/common/common.h
index 41eb03613c26c80b8bc77b07d4bb8dc348ad7c66..9168a24585c901784cc5d6b724451ddfad948210 100644 (file)
--- a/src/common/common.h
+++ b/src/common/common.h
@@ -23,5 +23,6 @@
 #include "macros.h"
 #include "runas.h"
 #include "readwrite.h"
+#include "time.h"
 
 #endif /* _COMMON_H */

diff --git a/src/common/sessiond-comm/inet.c b/src/common/sessiond-comm/inet.c
index e0b3e7a96ffb1d13324ae0bbb9f7b58238df93be..ac450dbc8b31b9e394eac7062d326edefcc26748 100644 (file)
--- a/src/common/sessiond-comm/inet.c
+++ b/src/common/sessiond-comm/inet.c
@@ -112,31 +112,13 @@ int connect_no_timeout(struct lttcomm_sock *sock)
                        sizeof(sock->sockaddr.addr.sin));
 }
 
-/*
- * Return time_a - time_b  in milliseconds.
- */
-static
-unsigned long time_diff_ms(struct timespec *time_a,
-               struct timespec *time_b)
-{
-       time_t sec_diff;
-       long nsec_diff;
-       unsigned long result_ms;
-
-       sec_diff = time_a->tv_sec - time_b->tv_sec;
-       nsec_diff = time_a->tv_nsec - time_b->tv_nsec;
-
-       result_ms = sec_diff * MSEC_PER_SEC;
-       result_ms += nsec_diff / NSEC_PER_MSEC;
-       return result_ms;
-}
-
 static
 int connect_with_timeout(struct lttcomm_sock *sock)
 {
        unsigned long timeout = lttcomm_get_network_timeout();
        int ret, flags, connect_ret;
        struct timespec orig_time, cur_time;
+       unsigned long diff_ms;
 
        ret = fcntl(sock->fd, F_GETFL, 0);
        if (ret == -1) {
@@ -217,7 +199,12 @@ int connect_with_timeout(struct lttcomm_sock *sock)
                        connect_ret = ret;
                        goto error;
                }
-       } while (time_diff_ms(&cur_time, &orig_time) < timeout);
+               if (timespec_to_ms(timespec_abs_diff(cur_time, orig_time), &diff_ms) < 0) {
+                       ERR("timespec_to_ms input overflows milliseconds output");
+                       connect_ret = -1;
+                       goto error;
+               }
+       } while (diff_ms < timeout);
 
        /* Timeout */
        errno = ETIMEDOUT;

diff --git a/src/common/sessiond-comm/inet6.c b/src/common/sessiond-comm/inet6.c
index dfb5fc5d1238c1b5cc6aa1b55cd0536b65ffec85..03b6627d1dce45188d28d81d234cb2c208c062eb 100644 (file)
--- a/src/common/sessiond-comm/inet6.c
+++ b/src/common/sessiond-comm/inet6.c
@@ -110,31 +110,13 @@ int connect_no_timeout(struct lttcomm_sock *sock)
                        sizeof(sock->sockaddr.addr.sin6));
 }
 
-/*
- * Return time_a - time_b  in milliseconds.
- */
-static
-unsigned long time_diff_ms(struct timespec *time_a,
-               struct timespec *time_b)
-{
-       time_t sec_diff;
-       long nsec_diff;
-       unsigned long result_ms;
-
-       sec_diff = time_a->tv_sec - time_b->tv_sec;
-       nsec_diff = time_a->tv_nsec - time_b->tv_nsec;
-
-       result_ms = sec_diff * MSEC_PER_SEC;
-       result_ms += nsec_diff / NSEC_PER_MSEC;
-       return result_ms;
-}
-
 static
 int connect_with_timeout(struct lttcomm_sock *sock)
 {
        unsigned long timeout = lttcomm_get_network_timeout();
        int ret, flags, connect_ret;
        struct timespec orig_time, cur_time;
+       unsigned long diff_ms;
 
        ret = fcntl(sock->fd, F_GETFL, 0);
        if (ret == -1) {
@@ -216,7 +198,12 @@ int connect_with_timeout(struct lttcomm_sock *sock)
                        connect_ret = ret;
                        goto error;
                }
-       } while (time_diff_ms(&cur_time, &orig_time) < timeout);
+               if (timespec_to_ms(timespec_abs_diff(cur_time, orig_time), &diff_ms) < 0) {
+                       ERR("timespec_to_ms input overflows milliseconds output");
+                       connect_ret = -1;
+                       goto error;
+               }
+       } while (diff_ms < timeout);
 
        /* Timeout */
        errno = ETIMEDOUT;

diff --git a/src/common/time.h b/src/common/time.h
index 81770779bdc90a898b1fefe1bbb8f88ef2d604fd..8a7dd958ffeeb6b86ef4a1ce1b11cf1c10a6a42b 100644 (file)
--- a/src/common/time.h
+++ b/src/common/time.h
@@ -19,9 +19,24 @@
 #ifndef LTTNG_TIME_H
 #define LTTNG_TIME_H
 
+#include <time.h>
+
 #define MSEC_PER_SEC   1000ULL
 #define NSEC_PER_SEC   1000000000ULL
 #define NSEC_PER_MSEC  1000000ULL
 #define NSEC_PER_USEC  1000ULL
 
+/*
+ * timespec_to_ms: Convert timespec to milliseconds.
+ *
+ * Returns 0 on success, else -1 on error. errno is set to EOVERFLOW if
+ * input would overflow the output in milliseconds.
+ */
+int timespec_to_ms(struct timespec ts, unsigned long *ms);
+
+/*
+ * timespec_abs_diff: Absolute difference between timespec.
+ */
+struct timespec timespec_abs_diff(struct timespec ts_a, struct timespec ts_b);
+
 #endif /* LTTNG_TIME_H */

diff --git a/src/common/utils.c b/src/common/utils.c
index 3442bef8922a276120e70cab43d3b46ca97ec05f..08139e5e2cb23d691c92c6800cef841e2362bc57 100644 (file)
--- a/src/common/utils.c
+++ b/src/common/utils.c
@@ -41,6 +41,7 @@
 
 #include "utils.h"
 #include "defaults.h"
+#include "time.h"
 
 /*
  * Return a partial realpath(3) of the path even if the full path does not
@@ -1645,3 +1646,38 @@ int utils_show_help(int section, const char *page_name,
 end:
        return ret;
 }
+
+LTTNG_HIDDEN
+int timespec_to_ms(struct timespec ts, unsigned long *ms)
+{
+       unsigned long res, remain_ms;
+
+       if (ts.tv_sec > ULONG_MAX / MSEC_PER_SEC) {
+               errno = EOVERFLOW;
+               return -1;      /* multiplication overflow */
+       }
+       res = ts.tv_sec * MSEC_PER_SEC;
+       remain_ms = ULONG_MAX - res;
+       if (ts.tv_nsec / NSEC_PER_MSEC > remain_ms) {
+               errno = EOVERFLOW;
+               return -1;      /* addition overflow */
+       }
+       res += ts.tv_nsec / NSEC_PER_MSEC;
+       *ms = res;
+       return 0;
+}
+
+LTTNG_HIDDEN
+struct timespec timespec_abs_diff(struct timespec t1, struct timespec t2)
+{
+       uint64_t ts1 = (uint64_t) t1.tv_sec * (uint64_t) NSEC_PER_SEC +
+                       (uint64_t) t1.tv_nsec;
+       uint64_t ts2 = (uint64_t) t2.tv_sec * (uint64_t) NSEC_PER_SEC +
+                       (uint64_t) t2.tv_nsec;
+       uint64_t diff = max(ts1, ts2) - min(ts1, ts2);
+       struct timespec res;
+
+       res.tv_sec = diff / (uint64_t) NSEC_PER_SEC;
+       res.tv_nsec = diff % (uint64_t) NSEC_PER_SEC;
+       return res;
+}