Date: Wed, 15 Nov 2017 21:23:49 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Ed Maste <emaste@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r325825 - head/sys/kern Message-ID: <20171115171430.P1572@besplex.bde.org> In-Reply-To: <201711141818.vAEIIILV078187@repo.freebsd.org> References: <201711141818.vAEIIILV078187@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <scottieyan@gmail.com> > 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20171115171430.P1572>