From 69657d9abefd819ea0e80cbb1dc3c4e135589a03 Mon Sep 17 00:00:00 2001
From: cheloha <cheloha@openbsd.org>
Date: Thu, 9 Jul 2020 02:17:07 +0000
Subject: adjfreq(2): limit adjustment to [-500000, +500000] ppm

When we recompute the scaling factor during tc_windup() there is an
opportunity for arithmetic overflow if the active timecounter's
adjfreq(2) adjustment is too large.  If we limit the adjustment to
[-500000, +500000] ppm the statement in question cannot overflow.

In particular, we are concerned with the following bit of code:

	scale = (u_int64_t)1 << 63;
	scale += \
	    ((th->th_adjustment + th->th_counter->tc_freq_adj) / 1024) * 2199;
	scale /= th->th_counter->tc_frequency;
	th->th_scale = scale * 2;

where scale is an int64_t.  Overflow when we do:

	scale += (...) / 1024 * 2199;

as th->th_counter->tc_freq_adj is currently unbounded.
th->th_adjustment is limited to [-5000ppm, 5000ppm].

To see that overflow is prevented with the new bounds, consider the
new edge case where th->th_counter->tc_freq_adj is 500000ppm and
th->th_adjustment is 5000ppm.  Both are of type int64_t.  We have:

   int64_t th_adjustment = (5000 * 1000) << 32;	/* 21474836480000000 */
   int64_t tc_freq_adj = 500000000LL << 32;	/* 2147483648000000000 */


   scale = (u_int64_t)1 << 63;			/* 9223372036854775808 */
   scale += (th_adjustment + tc_freq_adj) / 1024 * 2199;
/* scale += 2168958484480000000 / 1024 * 2199; */
/* scale += 4657753620480000000; */

9223372036854775808 + 4657753620480000000 = 13881125657334775808,
which less than 18446744073709551616, so we don't have overflow.

On the opposite end, if th->th_counter->tc_freq_adj is -500000ppm and
th->th_adjustment is -5000ppm we would have -4657753620480000000.

9223372036854775808 - 4657753620480000000 = 4565618416374775808.
Again, no overflow.

500000ppm and -500000ppm are extreme adjustments.  otto@ says ntpd(8)
would never arrive at them naturally, so we are not at risk of breaking
a working setup by imposing these restrictions.

Documentation input from kettenis@.

No complaints from otto@.
---
 lib/libc/sys/adjfreq.2 | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

(limited to 'lib/libc')

diff --git a/lib/libc/sys/adjfreq.2 b/lib/libc/sys/adjfreq.2
index 8bc38a8e5c5..183e65c6f08 100644
--- a/lib/libc/sys/adjfreq.2
+++ b/lib/libc/sys/adjfreq.2
@@ -1,4 +1,4 @@
-.\"	$OpenBSD: adjfreq.2,v 1.7 2015/09/10 17:55:21 schwarze Exp $
+.\"	$OpenBSD: adjfreq.2,v 1.8 2020/07/09 02:17:07 cheloha Exp $
 .\"
 .\" Copyright (c) 2006 Otto Moerbeek
 .\"
@@ -23,7 +23,7 @@
 .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 .\" SUCH DAMAGE.
 .\"
-.Dd $Mdocdate: September 10 2015 $
+.Dd $Mdocdate: July 9 2020 $
 .Dt ADJFREQ 2
 .Os
 .Sh NAME
@@ -60,6 +60,9 @@ The
 .Fa freq
 argument is non-null and the process's effective user ID is not that
 of the superuser.
+.It Bq Er EINVAL
+.Fa freq
+is less than -500000 ppm or greater than 500000 ppm.
 .El
 .Sh SEE ALSO
 .Xr date 1 ,
-- 
cgit v1.2.3-59-g8ed1b