]> asedeno.scripts.mit.edu Git - PuTTY.git/commitdiff
Fix a potential time-wraparound issue in pinger.c.
authorSimon Tatham <anakin@pobox.com>
Fri, 1 Apr 2016 12:27:03 +0000 (13:27 +0100)
committerSimon Tatham <anakin@pobox.com>
Sat, 2 Apr 2016 06:51:24 +0000 (07:51 +0100)
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
putty.h
timing.c

index 3f533ae6e0868c3721dda70ecb5b0b7cb8618e89..d8f110ac7fad6cee8a73ba7ae0c5d9cd3e1031e2 100644 (file)
--- 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 73bfd9e4820629777d0a88cdd3dfb7341ded4105..a149edb17a3cc101a9d1668eff761811c2083b67 100644 (file)
--- 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.
index ccd76cd66d88f7f4916500ceac9de43c0aa5021a..696c1e1d7ce58281052299dbc9c8bb2e3e49d67c 100644 (file)
--- 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