From owner-svn-src-head@freebsd.org Wed Nov 15 10:57:59 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id AEF58DD937E; Wed, 15 Nov 2017 10:57:59 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail106.syd.optusnet.com.au (mail106.syd.optusnet.com.au [211.29.132.42]) by mx1.freebsd.org (Postfix) with ESMTP id 439476618D; Wed, 15 Nov 2017 10:57:58 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from [192.168.0.103] (c110-21-101-228.carlnfd1.nsw.optusnet.com.au [110.21.101.228]) by mail106.syd.optusnet.com.au (Postfix) with ESMTPS id 7CD4B3C5CD7; Wed, 15 Nov 2017 21:23:49 +1100 (AEDT) Date: Wed, 15 Nov 2017 21:23:49 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Ed Maste cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r325825 - head/sys/kern In-Reply-To: <201711141818.vAEIIILV078187@repo.freebsd.org> Message-ID: <20171115171430.P1572@besplex.bde.org> References: <201711141818.vAEIIILV078187@repo.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.2 cv=KeqiiUQD c=1 sm=1 tr=0 a=PalzARQSbocsUSjMRkwAPg==:117 a=PalzARQSbocsUSjMRkwAPg==:17 a=kj9zAlcOel0A:10 a=pGLkceISAAAA:8 a=NEAV23lmAAAA:8 a=gItIPT6LeQ0ia8_EdOcA:9 a=CjuIK1q_8ugA:10 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 15 Nov 2017 10:57:59 -0000 On Tue, 14 Nov 2017, Ed Maste wrote: > Log: > disallow clock_settime too far in the future to avoid panic > > clock_ts_to_ct has a KASSERT that the converted year fits into four > digits. By default (sysctl debug.allow_insane_settime is 0) the kernel > disallows a time too far in the future, using a value of 9999 366-day > years. However, clock_settime is epoch-relative and the assertion will > fail with a tv_sec corresponding to some 8030 years. > > Avoid trying to be too clever, and just use a limit of 8000 365-day > years past the epoch. > > Submitted by: Heqing Yan > Reported by: Syzkaller (https://github.com/google/syzkaller) > MFC after: 1 week > Sponsored by: The FreeBSD Foundation I reported this panic and several others in reply to previous attempted fixes. > Modified: head/sys/kern/kern_time.c > ============================================================================== > --- head/sys/kern/kern_time.c Tue Nov 14 18:17:23 2017 (r325824) > +++ head/sys/kern/kern_time.c Tue Nov 14 18:18:18 2017 (r325825) > @@ -408,7 +408,7 @@ kern_clock_settime(struct thread *td, clockid_t clock_ > if (ats->tv_nsec < 0 || ats->tv_nsec >= 1000000000 || > ats->tv_sec < 0) > return (EINVAL); > - if (!allow_insane_settime && ats->tv_sec > 9999ULL * 366 * 24 * 60 * 60) > + if (!allow_insane_settime && ats->tv_sec > 8000ULL * 365 * 24 * 60 * 60) > return (EINVAL); > /* XXX Don't convert nsec->usec and back */ > TIMESPEC_TO_TIMEVAL(&atv, ats); Panics still occur when year 10K is reached via: tv_sec = 8000ULL * 365 * 24 * 60 * 60 - 1 (approx. 7994.5 years) POSIX Epoch offset = 1970 years utc_offset() <= (approx. 7994.5 + 1970) - 100000 = approx. -36.5 years (utc_offset() in seconds is the combined timezone offset 60 * tz.tz_minuteswest + (wall_cmos_clock ? adjkerntz : 0).) A utc_offset() of -36.5 years is preposterous but easy to reach without overflow (utc_offset() has blind overflow at approx. +-69 years). For 32-bit time_t, both of the above abominable ULL checks are vacuously false, but panics still occurs for all widths of time_t with non- preposterous values near the Epoch: tv_sec = 0 (Epoch in UTC) utc_offset() = 10 * 60 (garbage or panic in RTC; should be 1969) Wthout INVARIANTS, this writes the RTC with the garbage time 70/01/01 00:5c:05, where at least the 5c is from a buffer overrun. With INVARIANTS, garbage produced by clock_tv_to_ct() is detected there. tv_sec = INT_MAX (2038 in UTC) utc_offset() = -1 (garbage or panic in RTC; should be EINVAL) This takes 32-bit time_t. Then tv_sec - utc_offset() blindly overflows to INT_MIN on supported arches. The resulting garbage or panic in the RTC is not much worse than for any other negative value of tv_sec - utc_offset(). The settime() level doesn't know about utc_offset() (except for the historical mistake of setting the timezone using settimeofday()), and this is correct. Only the RTC level uses utc_offset() (or the timezone). The correct error handling is to not KASSERT() anything, but avoid overflow and just don't continue after overflow or set the RTC to a preposterous or unportable value. Bruce