From b4202c917a1e59dba2790463995959a2c2da745a Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Fri, 1 Apr 2016 13:27:03 +0100 Subject: [PATCH] Fix a potential time-wraparound issue in pinger.c. A compiler warning drew my attention to the fact that 'next' in pinger_schedule() was an int, not the unsigned long it should have been. And looking at the code that handles it, it was also taking no care with integer wraparound when checking whether an existing scheduled ping should be moved forward. So now I do something a bit more robust, by remembering what time it _was_ when we set pinger->next, and checking if the new time value falls in the interval between those two times. --- pinger.c | 8 +++++--- putty.h | 1 + timing.c | 11 +++++++++++ 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/pinger.c b/pinger.c index 3f533ae6..d8f110ac 100644 --- a/pinger.c +++ b/pinger.c @@ -8,7 +8,7 @@ struct pinger_tag { int interval; int pending; - unsigned long next; + unsigned long when_set, next; Backend *back; void *backhandle; }; @@ -28,7 +28,7 @@ static void pinger_timer(void *ctx, unsigned long now) static void pinger_schedule(Pinger pinger) { - int next; + unsigned long next; if (!pinger->interval) { pinger->pending = FALSE; /* cancel any pending ping */ @@ -37,8 +37,10 @@ static void pinger_schedule(Pinger pinger) next = schedule_timer(pinger->interval * TICKSPERSEC, pinger_timer, pinger); - if (!pinger->pending || next < pinger->next) { + if (!pinger->pending || + (next - pinger->when_set) < (pinger->next - pinger->when_set)) { pinger->next = next; + pinger->when_set = timing_last_clock(); pinger->pending = TRUE; } } diff --git a/putty.h b/putty.h index 73bfd9e4..a149edb1 100644 --- a/putty.h +++ b/putty.h @@ -1450,6 +1450,7 @@ unsigned long schedule_timer(int ticks, timer_fn_t fn, void *ctx); void expire_timer_context(void *ctx); int run_timers(unsigned long now, unsigned long *next); void timer_change_notify(unsigned long next); +unsigned long timing_last_clock(void); /* * Exports from callback.c. diff --git a/timing.c b/timing.c index ccd76cd6..696c1e1d 100644 --- a/timing.c +++ b/timing.c @@ -148,6 +148,17 @@ unsigned long schedule_timer(int ticks, timer_fn_t fn, void *ctx) return when; } +unsigned long timing_last_clock(void) +{ + /* + * Return the last value we stored in 'now'. In particular, + * calling this just after schedule_timer returns the value of + * 'now' that was used to decide when the timer you just set would + * go off. + */ + return now; +} + /* * Call to run any timers whose time has reached the present. * Returns the time (in ticks) expected until the next timer after