From owner-freebsd-current  Fri Dec 19 00:33:53 1997
Return-Path: <owner-freebsd-current>
Received: (from root@localhost)
          by hub.freebsd.org (8.8.7/8.8.7) id AAA03515
          for current-outgoing; Fri, 19 Dec 1997 00:33:53 -0800 (PST)
          (envelope-from owner-freebsd-current)
Received: from godzilla.zeta.org.au (godzilla.zeta.org.au [203.2.228.19])
          by hub.freebsd.org (8.8.7/8.8.7) with ESMTP id AAA03432;
          Fri, 19 Dec 1997 00:32:00 -0800 (PST)
          (envelope-from bde@zeta.org.au)
Received: (from bde@localhost) by godzilla.zeta.org.au (8.8.7/8.6.9) id TAA03871; Fri, 19 Dec 1997 19:26:35 +1100
Date: Fri, 19 Dec 1997 19:26:35 +1100
From: Bruce Evans <bde@zeta.org.au>
Message-Id: <199712190826.TAA03871@godzilla.zeta.org.au>
To: jmb@FreeBSD.ORG, lars@fredriks-1.pr.mcs.net
Subject: Re: EDOM and hz
Cc: current@FreeBSD.ORG
Sender: owner-freebsd-current@FreeBSD.ORG
X-Loop: FreeBSD.org
Precedence: bulk

>>                         if (tv->tv_sec > SHRT_MAX / hz - hz) {
>>                                 error = EDOM;
>>                                 goto bad;
>>                         }
>>...
>	<hmmm....better grab stevens v2....ah, let's see on p 543,4>
>
>Rich Stevens recommends:
>
>	if (tv->tv_sec*hz + tv_usec/tick > SHRT_MAX) {
>		error = EDOM;
>		goto bad;
>	}
>
>
>	where tick = 1,000,000 / hz
>	and   SHRT_MAX = 32767

As pointed out in Stevens, the original code should be changed because
it is wrong.  I had comments in my mailbox about this bug, but seem to
have lost the mail, and I thought that -current has a fix from Lite2,
but it doesn't.

The suggested change is wronger.  It fails when tv_sec*hz overflows,
and when the addition overflows.

Checking for overflow of the multiplication is not easy:

Method 1:
Convert everything to u_long and check that the values don't change, so
that the multiplication can't trap.  Then after doing the multiplication
(m = a * b), check that ((m / a) == b && (m % a) == 0), except when a
== 0.  This would be slower than doing the division in the original code,
especially if hz is constant.

Method 2:
Avoid possibile overflow by checking that (tv->tv_sec < LONG_MAX / hz).
When combined with other checks, this gives a check much like the one
in the original code.

A more correct version of the original code:

	if (tv->tv_sec > SHRT_MAX / hz - 1) {
		error = EDOM;
		goto bad;
	}

The -1 term is unnecessary in some cases, e.g., when tv->tv_usec == 0,
but it's probably not worth doing extra checks to avoid these cases.
The -1 term needs to be -2 in some cases where hz doesn't divide
1000000 evenly and tick = 1000000 / hz is rounded in a certain way.
The rounding becomes more significant if hz is a large fraction of
1000000 and can't work right if hz > 1000000.

Perhaps the best way is to combine the checks:

	u_long val;
	...
	/* assert(hz > 0); */
	if (tv->tv_sec < 0 || tv->tv_usec < 0 || tv->tv_usec >= 1000000 ||
	    tv->tv_sec > SHRT_MAX / hz) {
		error = EDOM;
		goto bad;
	}
	/* assert(tick > 0); */
	/* assert(ULONG_MAX - SHRT_MAX >= 1000000); */
	val = (u_long)(tv->tv_sec * hz) + tv_usec / tick;
	if (val > SHRT_MAX) {
		error = EDOM;
		goto bad;
	}

Bruce