Skip to content

Commit

Permalink
Fix a potential time-wraparound issue in pinger.c.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sgtatham committed Apr 2, 2016
1 parent 4605102 commit b4202c9
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 3 deletions.
8 changes: 5 additions & 3 deletions pinger.c
Expand Up @@ -8,7 +8,7 @@
struct pinger_tag {
int interval;
int pending;
unsigned long next;
unsigned long when_set, next;
Backend *back;
void *backhandle;
};
Expand All @@ -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 */
Expand All @@ -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;
}
}
Expand Down
1 change: 1 addition & 0 deletions putty.h
Expand Up @@ -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.
Expand Down
11 changes: 11 additions & 0 deletions timing.c
Expand Up @@ -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
Expand Down

0 comments on commit b4202c9

Please sign in to comment.