From owner-svn-src-all@freebsd.org Mon May 6 20:47:25 2019 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id C047D1595732 for ; Mon, 6 May 2019 20:47:25 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-qt1-x82a.google.com (mail-qt1-x82a.google.com [IPv6:2607:f8b0:4864:20::82a]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 5C2568947F for ; Mon, 6 May 2019 20:47:25 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-qt1-x82a.google.com with SMTP id d20so6459559qto.2 for ; Mon, 06 May 2019 13:47:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=ybWFaXiyZKCXfCw7qKdzK1f2ADxUQzkIkyNOwnBwLis=; b=pWJoHFxiHblhWbSpzlzb7F5kGeqVSy9ewFWQ/PpcNxDok8w99/W/giqpDZXmeaDMiw F9g+MeEdVpRx4wDJjUGC4zT9NxcFvtDiZ4c+r3yUCQWxpETCnKzWrhgmmbeNE1EgrY3j j6QtRQOJSCsQotS+Mjadnx3nOXSrzpj+5mN6goOlzOf5e3tv7K1QK5dvYus1Zei4NUz7 lsfLKtCOdxXvyX+XhvGeChhjbnkOkG7k5jL2UnLlfgRZ0FvqL7sBvpO0M09DZswSC/dE nSJYEF2R/VnGzm6G2P5XrzUoFo2SxfQZfoSY+Lm+SEP0Ow/b/s++cD7Q2kGzXrANDNqz hNsA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=ybWFaXiyZKCXfCw7qKdzK1f2ADxUQzkIkyNOwnBwLis=; b=DAw7rLo0/tiVxmrTBWIXTvCfIELYSiILkmGUAvwrnqC+DRL1kKF9LKUh/SqwRzKFaz Alqhj2OBXmhLeZUgjOcogNVUf4F9k0qZj+kHEaFATaGZQCDlT28qEUhS6XidNByA1TmM QCJNLky2ARRWKP02V8f9Q1RPSZkcbUYxGuVJgX/1af/2I/bTM0irYgPxHUHx61yW9C3N wJGpGKLr4NWMUM2ecoB6or3KOTe6CxtSMkyEsWkG9HLQ36oIqj+22beZwLt2sli3E7nx gyhw5FmLipBATlrLAFdVUGQX16g5ZR3xg5y1UpPYjv8N9X7q8yZw25aScnmtXvBld/td 6j2A== X-Gm-Message-State: APjAAAW8UoCqsvoSomAd0HoHJKxvByLeExTeA29LxgXac4ffU38BISqj /pignUeTBS3dZyz4FvHZZHF2sYBiWJ+T5AcNjWs4tg== X-Google-Smtp-Source: APXvYqwkALJxj8FEn8HZuDThmywlrWmRSsozTtxYPNjlcRAy6tkm6Xc2RMaxdn0NdmVgzsOQPW5s2pfl27QGtwc6Ulc= X-Received: by 2002:ac8:28d0:: with SMTP id j16mr23757746qtj.15.1557175644676; Mon, 06 May 2019 13:47:24 -0700 (PDT) MIME-Version: 1.0 References: <201905032126.x43LQilu092655@repo.freebsd.org> <335d828e-ac61-bc59-bac3-f80f27b951c7@FreeBSD.org> <20190506184502.GA35464@raichu> <52484f6b-fdae-565b-6c03-37a63d56ad30@FreeBSD.org> In-Reply-To: <52484f6b-fdae-565b-6c03-37a63d56ad30@FreeBSD.org> From: Warner Losh Date: Mon, 6 May 2019 14:47:13 -0600 Message-ID: Subject: Re: svn commit: r347063 - head/sys/kern To: John Baldwin Cc: Mark Johnston , src-committers , svn-src-all , svn-src-head X-Rspamd-Queue-Id: 5C2568947F X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-6.99 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; NEURAL_HAM_SHORT(-0.99)[-0.988,0]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; REPLY(-4.00)[] Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 May 2019 20:47:26 -0000 On Mon, May 6, 2019 at 2:40 PM John Baldwin wrote: > On 5/6/19 11:45 AM, Mark Johnston wrote: > > On Mon, May 06, 2019 at 11:07:18AM -0700, John Baldwin wrote: > >> On 5/3/19 2:26 PM, Mark Johnston wrote: > >>> Author: markj > >>> Date: Fri May 3 21:26:44 2019 > >>> New Revision: 347063 > >>> URL: https://svnweb.freebsd.org/changeset/base/347063 > >>> > >>> Log: > >>> Disallow excessively small times of day in clock_settime(2). > >>> > >>> Reported by: syzkaller > >>> Reviewed by: cem, kib > >>> MFC after: 1 week > >>> Sponsored by: The FreeBSD Foundation > >>> Differential Revision: https://reviews.freebsd.org/D20151 > >>> > >>> Modified: > >>> head/sys/kern/kern_time.c > >>> > >>> Modified: head/sys/kern/kern_time.c > >>> > ============================================================================== > >>> --- head/sys/kern/kern_time.c Fri May 3 21:13:09 2019 > (r347062) > >>> +++ head/sys/kern/kern_time.c Fri May 3 21:26:44 2019 > (r347063) > >>> @@ -412,7 +412,9 @@ 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 > 8000ULL * 365 * 24 * 60 > * 60) > >>> + if (!allow_insane_settime && > >>> + (ats->tv_sec > 8000ULL * 365 * 24 * 60 * 60 || > >>> + ats->tv_sec < utc_offset())) > >>> return (EINVAL); > >>> /* XXX Don't convert nsec->usec and back */ > >>> TIMESPEC_TO_TIMEVAL(&atv, ats); > >> > >> Pardon my ignorance, but I can't see why you are checking against > utc_offset() > >> vs some small constant? None of the discussion in the review mentioned > the > >> reason for using this particular value, and I didn't see any comparisons > >> against utc_offset or kernadjtz in kern_clock_setttime() or settime() > that > >> would have underflowed or panicked. Can you give a bit more detail on > why > >> utc_offset() is the lower bound? Thanks. > > > > I chose it because we subtract utc_offset() from the time passed in to > > clock_settime(); see settime_task_func(). That subtraction caused the > > underflow that later caused the observed panics. > > Ok, thanks. A few things I didn't see anyone else note in the review then: > > 1) This subtraction is actually not done for all rtc drivers, so it seems > like we might block small times for RTC clocks that set > CLOCKF_GETTIME_NO_ADJ. > 2) utc_offset can be negative for machines using local time in timezones > "before" UTC. > > I suppose we don't think any FreeBSD machines actually need to set the > running clock to 0 anyway so fixing it here rather than rejecting invalid > values only for RTCs that can't handle it is probably ok, but the > connection doesn't feel obvious that we are rejecting times that might > be non-representable in RTCs. > I'd be tempted to change utc_offset() to be "< 2 * 86400" personally, since that's the worst-case UTC offset and will cover all cases for people that want to run in 1970. Though honestly, we could also substitute in 946684800 (First second of 2000) as well, since who needs to run before 2000, right? Warner