* [PATCH lttng-tools 1/2] Fix: Connect timeout arithmetic in inet/inet6
@ 2018-11-09 22:27 Mathieu Desnoyers
0 siblings, 0 replies; 3+ messages in thread
From: Mathieu Desnoyers @ 2018-11-09 22:27 UTC (permalink / raw)
To: jgalar, jonathan.rajotte-julien; +Cc: lttng-dev
The nanoseconds part of the timespec struct time_a is not always
bigger than time_b since it wrap around each seconds.
Use 64-bit arithmetic to perform 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>
---
src/common/common.h | 5 +++++
src/common/sessiond-comm/inet.c | 28 ++++++++--------------------
src/common/sessiond-comm/inet6.c | 28 ++++++++--------------------
src/common/utils.c | 29 +++++++++++++++++++++++++++++
4 files changed, 50 insertions(+), 40 deletions(-)
diff --git a/src/common/common.h b/src/common/common.h
index 41eb0361..48f39e0e 100644
--- a/src/common/common.h
+++ b/src/common/common.h
@@ -19,9 +19,14 @@
#ifndef _COMMON_H
#define _COMMON_H
+#include <time.h>
+
#include "error.h"
#include "macros.h"
#include "runas.h"
#include "readwrite.h"
+int timespec_to_ms(struct timespec ts, unsigned long *ms);
+struct timespec timespec_abs_diff(struct timespec ts_a, struct timespec ts_b);
+
#endif /* _COMMON_H */
diff --git a/src/common/sessiond-comm/inet.c b/src/common/sessiond-comm/inet.c
index e0b3e7a9..76a3b7ff 100644
--- 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,13 @@ 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");
+ errno = EOVERFLOW;
+ 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 dfb5fc5d..5c1ac2e0 100644
--- 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,13 @@ 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");
+ errno = EOVERFLOW;
+ connect_ret = -1;
+ goto error;
+ }
+ } while (diff_ms < timeout);
/* Timeout */
errno = ETIMEDOUT;
diff --git a/src/common/utils.c b/src/common/utils.c
index 3442bef8..c4759f68 100644
--- a/src/common/utils.c
+++ b/src/common/utils.c
@@ -31,6 +31,7 @@
#include <pwd.h>
#include <sys/file.h>
#include <unistd.h>
+#include <stdint.h>
#include <common/common.h>
#include <common/runas.h>
@@ -41,6 +42,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 +1647,30 @@ 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;
+
+ if (ts.tv_sec + 1 > ULONG_MAX / MSEC_PER_SEC) {
+ return -1;
+ }
+ res = ts.tv_sec * MSEC_PER_SEC;
+ 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 * INT64_C(1000000000) + (uint64_t) t1.tv_nsec;
+ uint64_t ts2 = (uint64_t) t2.tv_sec * INT64_C(1000000000) + (uint64_t) t2.tv_nsec;
+ uint64_t diff = max(ts1, ts2) - min(ts1, ts2);
+ struct timespec res;
+
+ res.tv_sec = diff / INT64_C(1000000000);
+ res.tv_nsec = diff % INT64_C(1000000000);
+ return res;
+}
--
2.11.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH lttng-tools 1/2] Fix: Connect timeout arithmetic in inet/inet6
[not found] ` <20181109223407.GA27438@joraj-alpa>
@ 2018-11-09 22:41 ` Mathieu Desnoyers
0 siblings, 0 replies; 3+ messages in thread
From: Mathieu Desnoyers @ 2018-11-09 22:41 UTC (permalink / raw)
To: Jonathan Rajotte; +Cc: lttng-dev, Jeremie Galarneau
----- On Nov 9, 2018, at 5:34 PM, Jonathan Rajotte jonathan.rajotte-julien@efficios.com wrote:
[...]
>> +
>> +LTTNG_HIDDEN
>> +struct timespec timespec_abs_diff(struct timespec t1, struct timespec t2)
>> +{
>> + uint64_t ts1 = (uint64_t) t1.tv_sec * INT64_C(1000000000) + (uint64_t)
>> t1.tv_nsec;
>> + uint64_t ts2 = (uint64_t) t2.tv_sec * INT64_C(1000000000) + (uint64_t)
>> t2.tv_nsec;
>
> Use NSEC_PER_SEC here.
>
>> + uint64_t diff = max(ts1, ts2) - min(ts1, ts2);
>> + struct timespec res;
>> +
>> + res.tv_sec = diff / INT64_C(1000000000);
>
> Use NSEC_PER_SEC here.
>
>> + res.tv_nsec = diff % INT64_C(1000000000);
>
> Use NSEC_PER_SEC here.
Fixing in v2, thanks,
Mathieu
>
>> + return res;
>> +}
>> --
>> 2.11.0
>>
>
> --
> Jonathan Rajotte-Julien
> EfficiOS
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH lttng-tools 1/2] Fix: Connect timeout arithmetic in inet/inet6
[not found] <20181109222749.27267-1-mathieu.desnoyers@efficios.com>
@ 2018-11-09 22:34 ` Jonathan Rajotte-Julien
[not found] ` <20181109223407.GA27438@joraj-alpa>
1 sibling, 0 replies; 3+ messages in thread
From: Jonathan Rajotte-Julien @ 2018-11-09 22:34 UTC (permalink / raw)
To: Mathieu Desnoyers; +Cc: lttng-dev, jgalar
On Fri, Nov 09, 2018 at 05:27:48PM -0500, Mathieu Desnoyers wrote:
> The nanoseconds part of the timespec struct time_a is not always
> bigger than time_b since it wrap around each seconds.
>
> Use 64-bit arithmetic to perform 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>
> ---
> src/common/common.h | 5 +++++
> src/common/sessiond-comm/inet.c | 28 ++++++++--------------------
> src/common/sessiond-comm/inet6.c | 28 ++++++++--------------------
> src/common/utils.c | 29 +++++++++++++++++++++++++++++
> 4 files changed, 50 insertions(+), 40 deletions(-)
>
> diff --git a/src/common/common.h b/src/common/common.h
> index 41eb0361..48f39e0e 100644
> --- a/src/common/common.h
> +++ b/src/common/common.h
> @@ -19,9 +19,14 @@
> #ifndef _COMMON_H
> #define _COMMON_H
>
> +#include <time.h>
> +
> #include "error.h"
> #include "macros.h"
> #include "runas.h"
> #include "readwrite.h"
>
> +int timespec_to_ms(struct timespec ts, unsigned long *ms);
> +struct timespec timespec_abs_diff(struct timespec ts_a, struct timespec ts_b);
> +
> #endif /* _COMMON_H */
> diff --git a/src/common/sessiond-comm/inet.c b/src/common/sessiond-comm/inet.c
> index e0b3e7a9..76a3b7ff 100644
> --- 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,13 @@ 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");
> + errno = EOVERFLOW;
> + 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 dfb5fc5d..5c1ac2e0 100644
> --- 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,13 @@ 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");
> + errno = EOVERFLOW;
> + connect_ret = -1;
> + goto error;
> + }
> + } while (diff_ms < timeout);
>
> /* Timeout */
> errno = ETIMEDOUT;
> diff --git a/src/common/utils.c b/src/common/utils.c
> index 3442bef8..c4759f68 100644
> --- a/src/common/utils.c
> +++ b/src/common/utils.c
> @@ -31,6 +31,7 @@
> #include <pwd.h>
> #include <sys/file.h>
> #include <unistd.h>
> +#include <stdint.h>
>
> #include <common/common.h>
> #include <common/runas.h>
> @@ -41,6 +42,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 +1647,30 @@ 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;
> +
> + if (ts.tv_sec + 1 > ULONG_MAX / MSEC_PER_SEC) {
> + return -1;
> + }
> + res = ts.tv_sec * MSEC_PER_SEC;
> + 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 * INT64_C(1000000000) + (uint64_t) t1.tv_nsec;
> + uint64_t ts2 = (uint64_t) t2.tv_sec * INT64_C(1000000000) + (uint64_t) t2.tv_nsec;
Use NSEC_PER_SEC here.
> + uint64_t diff = max(ts1, ts2) - min(ts1, ts2);
> + struct timespec res;
> +
> + res.tv_sec = diff / INT64_C(1000000000);
Use NSEC_PER_SEC here.
> + res.tv_nsec = diff % INT64_C(1000000000);
Use NSEC_PER_SEC here.
> + return res;
> +}
> --
> 2.11.0
>
--
Jonathan Rajotte-Julien
EfficiOS
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-11-09 22:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-09 22:27 [PATCH lttng-tools 1/2] Fix: Connect timeout arithmetic in inet/inet6 Mathieu Desnoyers
[not found] <20181109222749.27267-1-mathieu.desnoyers@efficios.com>
2018-11-09 22:34 ` Jonathan Rajotte-Julien
[not found] ` <20181109223407.GA27438@joraj-alpa>
2018-11-09 22:41 ` Mathieu Desnoyers
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.