From owner-svn-src-all@freebsd.org Mon May 6 21:25:21 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 762FB15969AE for ; Mon, 6 May 2019 21:25:21 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-qt1-x830.google.com (mail-qt1-x830.google.com [IPv6:2607:f8b0:4864:20::830]) (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 165288B7D5 for ; Mon, 6 May 2019 21:25:21 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-qt1-x830.google.com with SMTP id d13so16648790qth.5 for ; Mon, 06 May 2019 14:25:21 -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=/loTN5dadOPclcl0IpkLKCXqos51rM/PMqKBOulAdoo=; b=XaXnznmNtegNUvzKMKgUCtogKA7uMvzTGuVjTPHzS1ZATEioevDbYZpPghxr98Vxuq 6p2jCaFPGksgApwFPbD80q+jxWF25kmlvnP4rD5aP7cjVO9Hg07VwLnM2pkE88wVS8rH xuso64AycjCT92yAravKtdWZ32scmFpk5KBlV+V+bD2cRq6OfFAGQ2lpR1UL7niQLEc/ X10xpl0gY/UtpuJXMmseZqM9sMfMh3I2h+FaZ72HGOizCNNhj+sWBbT3f5MyKuFtsMbP Q4y5/by7wyJ/8zQPhfd230X5N3RUA7sjCiw/+ABRIfRBIIbwXVv/Eh5IkFI41rlkaOBQ n90A== 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=/loTN5dadOPclcl0IpkLKCXqos51rM/PMqKBOulAdoo=; b=qKZY4kVj3aJHh1vWDZl9K3za6QsP9WI31zXwJc+EQoNLDrsK+c26THZMFKNMpgLS1B BYxiImbuBOzqp8P5r8Ma7T4nOYTjVcWnaoAIj92o83NQ6mnItuWOUzskwdzeeFoOz9Q+ d5FOdLY2LPzqNFq+bKHxmtyfh0NjtcOa3oeCpYFXce2bZnjLNh79lqNm4IaRl/foq7M9 zoiqfVBZzSLbRsyZOwHSTDrkkgXOVuoOfxlD3tejm/pOfaiknMTRHK5X21x048jXiBvc XN1L07AgDMWKOJ7XH8c9pEZ4rua4uPKdrfP2xecP/lvzEhbsw4bjR9UZvfx0tHONnTc5 yPEQ== X-Gm-Message-State: APjAAAXUVGS4BJeCtroryVCNqmlQkJ8Bxpma4xo3H1FFuI4kEeDskxoi AgqlfOIJQu00lMQVRFwOdHeyiT6J7JlvsytN0veFzA== X-Google-Smtp-Source: APXvYqw+lZMimrYX8VUBluD9gkRgq7IZYNGQ+IJw4BXPckZ6SeskpUSOY9A2iFSoX5rm7RvtMZXRIVl+cn7f6w5L7h8= X-Received: by 2002:a0c:ee25:: with SMTP id l5mr22177662qvs.153.1557177920398; Mon, 06 May 2019 14:25:20 -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> <20190506205743.GA65083@raichu> <85c51618-01ca-2005-388e-52762358320b@FreeBSD.org> In-Reply-To: <85c51618-01ca-2005-388e-52762358320b@FreeBSD.org> From: Warner Losh Date: Mon, 6 May 2019 15:25:09 -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: 165288B7D5 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_LONG(-1.00)[-1.000,0]; REPLY(-4.00)[]; NEURAL_HAM_SHORT(-0.99)[-0.988,0] 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 21:25:21 -0000 On Mon, May 6, 2019 at 3:11 PM John Baldwin wrote: > On 5/6/19 1:57 PM, Mark Johnston wrote: > > On Mon, May 06, 2019 at 01:40:19PM -0700, 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. > > > > The drivers that set NO_ADJ still account for the offset in their > > individual settime methods. I don't see how it can be correct for any > > driver to ignore adjkerntz? > > > >> 2) utc_offset can be negative for machines using local time in timezones > >> "before" UTC. > > > > Hmm, I believe the patch still handles this case? > > Yes, it's just redundant as the earlier 'tv_sec < 0' check means we'll > never see a value < utc_offset. One thought I had was similar to what > Warner suggested of just using the max value of '24 * 60 * 60', though > I suppose the maximal utc_offset is more like '12 * 60 * 60'? utc_offset > is probably good enough as is. > The maximum UTC offset is closer to 15 hours due to that crazy international date line and different groups of islands wanting to be on one side or the other of the international date line. that's why I suggested 24 hours (if you read carefully, I typed * 2 for bogus reasons). Warner