Hi,
On 06-03-18 07:09, selva.n...@gmail.com wrote:
> From: Selva Nair <selva.n...@gmail.com>
>
> Time interval arithmetic can overflow especially when user
> defined intervals are involved. E.g., see Trac #922.
>
> Avoid this by reordering the arithmetic operation in
> event_timeout_trigger(). Also avoid unnecessary casting of time
> variable to int.
>
> Time until wakeup is now calculated like:
>
> time_t wakeup = (last - now) + delay
>
> Here delay is of type int, but is +ve by construction. Time backtrack
> protection in OpenVPN ensures (last - now) <= 0. Then the above
> expression cannot overflow (provided time_t is at least as large
> as int).
>
> A similar expression in interval.h is also changed.
>
> (This patch grew out of patch 168 by Steffan Karger.)
>
> Signed-off-by: Selva Nair <selva.n...@gmail.com>
> ---
> src/openvpn/interval.c | 8 +++++---
> src/openvpn/interval.h | 2 +-
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/src/openvpn/interval.c b/src/openvpn/interval.c
> index 00ee627..b728560 100644
> --- a/src/openvpn/interval.c
> +++ b/src/openvpn/interval.c
> @@ -51,11 +51,12 @@ event_timeout_trigger(struct event_timeout *et,
>
> if (et->defined)
> {
> - int wakeup = (int) et->last + et->n - local_now;
> + time_t wakeup = et->last - local_now + et->n;
I would have preferred braces to not have any doubt about associativity,
but can live with this. (I always forget, so had to look up what C did
again.)
> if (wakeup <= 0)
> {
> #if INTERVAL_DEBUG
> - dmsg(D_INTERVAL, "EVENT event_timeout_trigger (%d) etcr=%d",
> et->n, et_const_retry);
> + dmsg(D_INTERVAL, "EVENT event_timeout_trigger (%d) etcr=%d",
> et->n,
> + et_const_retry);
> #endif
> if (et_const_retry < 0)
> {
> @@ -72,7 +73,8 @@ event_timeout_trigger(struct event_timeout *et,
> if (tv && wakeup < tv->tv_sec)
> {
> #if INTERVAL_DEBUG
> - dmsg(D_INTERVAL, "EVENT event_timeout_wakeup (%d/%d) etcr=%d",
> wakeup, et->n, et_const_retry);
> + dmsg(D_INTERVAL, "EVENT event_timeout_wakeup (%d/%d) etcr=%d",
> + (int) wakeup, et->n, et_const_retry);
> #endif
> tv->tv_sec = wakeup;
> tv->tv_usec = 0;
> diff --git a/src/openvpn/interval.h b/src/openvpn/interval.h
> index 826a08b..5623f3a 100644
> --- a/src/openvpn/interval.h
> +++ b/src/openvpn/interval.h
> @@ -196,7 +196,7 @@ event_timeout_modify_wakeup(struct event_timeout *et,
> interval_t n)
> static inline interval_t
> event_timeout_remaining(struct event_timeout *et)
> {
> - return (int) et->last + et->n - now;
> + return (interval_t) (et->last - now + et->n);
> }
>
> /*
>
As discussed, an elegant way of solving the issue (verified manually
with -fsanitize=undefined).
Acked-by: Steffan Karger <stef...@karger.me>
-Steffan
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
