Project

General

Profile

Actions

Bug #9858

open

timespecadd() timer calculations can overflow

Added by Robert Mustacchi almost 3 years ago.

Status:
New
Priority:
Normal
Assignee:
-
Category:
-
Start date:
2018-09-27
Due date:
% Done:

0%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage
Gerrit CR:

Description

Alexander Pyhalov had a C program that would end up causing a timer to fire erroneously immediately when compiled as a 64-bit program:

#include <stdio.h>
#include <signal.h>
#include <time.h>
#include <unistd.h>
#include <limits.h>

static void
cleanup (int sig)
{
  printf ("Got signal %d\n", sig);
}

int main()
{

  struct timespec ts ;

  ts.tv_sec=LONG_MAX;
  ts.tv_nsec=1000000000 - 1;
  printf("ts.tv_sec=%ld\n", ts.tv_sec);
  struct itimerspec its = { {0, 0}, ts };
  timer_t timerid;

  struct sigaction sa;

  sigemptyset (&sa.sa_mask);
  sa.sa_handler = cleanup;

  sigaction (SIGALRM, &sa, NULL); /* our timeout.  */

  if (timer_create (CLOCK_REALTIME, NULL, &timerid) == 0)
    {
      if (timer_settime (timerid, 0, &its, NULL) == 0)
         ;
      else
        {
          perror ("warning: timer_settime");
          timer_delete (timerid);
        }
    }

   sleep(10);

   return 0;
}

When looking at this, I suspected that we might be seeing a timer overflow and put together the following DTrace script one liner:

# dtrace -n 'fbt::timespecadd:entry/execname == "a.out"/{ stack(); print(*args[0]); self->t = args[0]; }' -n 'fbt::timespecadd:return/self->t/{ print(*self->t); self->t = NULL; }' -c '/var/tmp/a.out'
dtrace: description 'fbt::timespecadd:entry' matched 1 probe
dtrace: description 'fbt::timespecadd:return' matched 1 probe
ts.tv_sec=9223372036854775807
Got signal 14
dtrace: pid 82423 has exited
CPU     ID                    FUNCTION:NAME
  4  22511                timespecadd:entry 
              genunix`clock_realtime_timer_settime+0x104
              genunix`timer_settime+0xf4
              unix`sys_syscall+0x19f
timespec_t {
    time_t tv_sec = 0x7fffffffffffffff
    long tv_nsec = 0x3b9ac9ff
}
  4  22512               timespecadd:return timespec_t {
    time_t tv_sec = 0x800000005bad24f7
    long tv_nsec = 0xa78c767
}
  4  22511                timespecadd:entry 
              genunix`nanosleep+0x15f
              unix`sys_syscall+0x19f
timespec_t {
    time_t tv_sec = 0xa
    long tv_nsec = 0
}
  4  22512               timespecadd:return timespec_t {
    time_t tv_sec = 0x5bad2501
    long tv_nsec = 0xa78eba6
}

From the first firing we can see that the time_t for tv_sec ends up transforming into a negative number. Fundamentally timespecadd() doesn't check at all for overflow of the tv_sec field. From a brief survey of other systems it appears that Linux, FreeBSD, and NetBSD instead cause this to wait at the maximum time. We should probably look at doing the same and fixing up the timespecadd() function to take care of that problem in general. This high resolution clock does have some checking of the interval logic to make sure that what we have is ultimately representable at first. We may want to adopt some of that into the various callers of timespecadd(). There's a good chance that this should be done in timespecadd(), but I haven't audited all the callers to understand the implications of that yet.

No data to display

Actions

Also available in: Atom PDF