Re: [PATCH] timing.cc integer overflow

[David Fifield <david () bamsoftware com>]

On Mon, Jan 26, 2009 at 06:56:57PM +0100, Henri Doreau wrote:
> Thanks for your reply. Here are further elements.
>
> > Can you find a set of values that cause an overflow, to demonstrate that
> > the overflow is fixed in at least one case? I mean values for begin,
> > now, and perc_done.
>     - At 20% of a scan
>     - Scan started 10 days ago
>     Output :
>         Real current time : Mon Jan 26 14:00:43 2009
>         Assuming this pseudo scan started at Fri Jan 16 14:00:43 2009
>         UDP scan Timing: About 20.00% done; ETC: 20:57 (-233:-2:-47 remaining)

I get different results from you with an elapsed time of 10 days and
0.20 perc_done. I spliced this code into nmap_main:

  struct tm t;
  t.tm_sec = 0;
  t.tm_min = 0;
  t.tm_hour = 0;
  t.tm_mon = 0;
  t.tm_year = 109;
  time_t tt = mktime(&t);
  ScanProgressMeter spm("TEST");
  spm.begin.tv_sec = tt;
  spm.begin.tv_usec = 0;
  struct timeval tv;
  for (int days = 0; days <= 50; days += 10) {
    tv = spm.begin;
    TIMEVAL_MSEC_ADD(tv, tv, days * 86400UL * 1000);
    printf("%d days elapsed\n", days);
    spm.printStats(0.00, &tv);
    spm.printStats(0.10, &tv);
    spm.printStats(0.20, &tv);
    spm.printStats(0.30, &tv);
    spm.printStats(0.40, &tv);
    spm.printStats(0.50, &tv);
    spm.printStats(0.60, &tv);
    spm.printStats(0.70, &tv);
    spm.printStats(0.80, &tv);
    spm.printStats(0.90, &tv);
    spm.printStats(1.00, &tv);
  }
  exit(1);

Part of the output is

10 days elapsed
TEST Timing: About 0.00% done
TEST Timing: About 10.00% done; ETC: 20:31 (356:31:23 remaining)
TEST Timing: About 20.00% done; ETC: 20:31 (356:31:23 remaining)
TEST Timing: About 30.00% done; ETC: 20:31 (356:31:23 remaining)
TEST Timing: About 40.00% done; ETC: 20:31 (356:31:23 remaining)
TEST Timing: About 50.00% done; ETC: 00:00 (240:00:00 remaining)
TEST Timing: About 60.00% done; ETC: 16:00 (160:00:00 remaining)
TEST Timing: About 70.00% done; ETC: 06:51 (102:51:25 remaining)
TEST Timing: About 80.00% done; ETC: 11:59 (59:59:59 remaining)
TEST Timing: About 90.00% done; ETC: 02:39 (26:39:59 remaining)
TEST Timing: About 100.00% done; ETC: 00:00 (0:00:00 remaining)

I do get the "(-233:-2:-47 remaining)" but it's at 40 days and 50%:

40 days elapsed
TEST Timing: About 0.00% done
TEST Timing: About 10.00% done; ETC: 20:31 (-363:-28:-36 remaining)
TEST Timing: About 20.00% done; ETC: 20:31 (-363:-28:-36 remaining)
TEST Timing: About 30.00% done; ETC: 20:31 (-363:-28:-36 remaining)
TEST Timing: About 40.00% done; ETC: 10:25 (-349:-34:-10 remaining)
TEST Timing: About 50.00% done; ETC: 06:57 (-233:-2:-47 remaining)
TEST Timing: About 60.00% done; ETC: 12:38 (-155:-21:-51 remaining)
TEST Timing: About 70.00% done; ETC: 20:07 (-99:-52:-37 remaining)
TEST Timing: About 80.00% done; ETC: 13:44 (-58:-15:-41 remaining)
TEST Timing: About 90.00% done; ETC: 22:06 (-25:-53:-38 remaining)
TEST Timing: About 100.00% done; ETC: 00:00 (0:00:00 remaining)

I don't think your patch is quite correct, because after applying it I
get huge values for the time remaining:

10 days elapsed
TEST Timing: About 0.00% done
TEST Timing: About 10.00% done; ETC: 06:57 (1831415424 day(s), 250072:40:32 remaining)
TEST Timing: About 20.00% done; ETC: 23:59 (2245570176 day(s), 998021:58:24 remaining)
TEST Timing: About 30.00% done; ETC: 08:00 (2383708160 day(s), 1162149:11:28 remaining)
TEST Timing: About 40.00% done; ETC: 23:59 (305163904 day(s), 178950:09:04 remaining)
TEST Timing: About 50.00% done; ETC: 00:00 (1635155968 day(s), 327628:43:44 remaining)
TEST Timing: About 60.00% done; ETC: 16:00 (2521759744 day(s), 1013783:28:00 remaining)
TEST Timing: About 70.00% done; ETC: 06:51 (1927852928 day(s), 258073:29:18 remaining)
TEST Timing: About 80.00% done; ETC: 11:59 (1482444416 day(s), 365937:28:32 remaining)
TEST Timing: About 90.00% done; ETC: 02:39 (3999346304 day(s), 1049517:26:24 remaining)
TEST Timing: About 100.00% done; ETC: 00:00 (0 day(s), 0:00:00 remaining)

Did I do something wrong in the test code, or make a mistake applying
your patch? Do you get the same thing if you run the test code above? I
attached the two output files time.orig and time.henri.

I think doing all the calculations with doubles is the right approach.
If you're doing that, there's no reason to do calculations in
milliseconds; just do everything in seconds. There shouldn't be any
problems with overflow using doubles. Please send your next patch
without the "day(s)" code and keep everything in the hours like before.
Showing days is a good idea but that belongs in a separate commit.

David Fifield

Attachment: time.orig
Description:

Attachment: time.henri
Description:

_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://SecLists.Org