summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorcheloha <cheloha@openbsd.org>2020-07-09 02:17:07 +0000
committercheloha <cheloha@openbsd.org>2020-07-09 02:17:07 +0000
commit69657d9abefd819ea0e80cbb1dc3c4e135589a03 (patch)
tree4c412870bdc076b8fb75688097e2c25223b5ed7e
parentNew regression tests for integral type conversions (diff)
downloadwireguard-openbsd-69657d9abefd819ea0e80cbb1dc3c4e135589a03.tar.xz
wireguard-openbsd-69657d9abefd819ea0e80cbb1dc3c4e135589a03.zip
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@.
-rw-r--r--lib/libc/sys/adjfreq.27
-rw-r--r--sys/kern/kern_time.c7
2 files changed, 11 insertions, 3 deletions
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 ,
diff --git a/sys/kern/kern_time.c b/sys/kern/kern_time.c
index 741bc2b9650..872480aa314 100644
--- a/sys/kern/kern_time.c
+++ b/sys/kern/kern_time.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: kern_time.c,v 1.131 2020/06/22 18:25:57 cheloha Exp $ */
+/* $OpenBSD: kern_time.c,v 1.132 2020/07/09 02:17:07 cheloha Exp $ */
/* $NetBSD: kern_time.c,v 1.20 1996/02/18 11:57:06 fvdl Exp $ */
/*
@@ -391,6 +391,9 @@ sys_settimeofday(struct proc *p, void *v, register_t *retval)
return (0);
}
+#define ADJFREQ_MAX (500000000LL << 32)
+#define ADJFREQ_MIN (-500000000LL << 32)
+
int
sys_adjfreq(struct proc *p, void *v, register_t *retval)
{
@@ -408,6 +411,8 @@ sys_adjfreq(struct proc *p, void *v, register_t *retval)
return (error);
if ((error = copyin(freq, &f, sizeof(f))))
return (error);
+ if (f < ADJFREQ_MIN || f > ADJFREQ_MAX)
+ return (EINVAL);
}
rw_enter(&tc_lock, (freq == NULL) ? RW_READ : RW_WRITE);