From owner-svn-src-all@freebsd.org Mon May 6 21:14:05 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 5AE661596541 for ; Mon, 6 May 2019 21:14:05 +0000 (UTC) (envelope-from ian@freebsd.org) Received: from outbound1a.eu.mailhop.org (outbound1a.eu.mailhop.org [52.58.109.202]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id BDBD38ADB4 for ; Mon, 6 May 2019 21:14:04 +0000 (UTC) (envelope-from ian@freebsd.org) ARC-Seal: i=1; a=rsa-sha256; t=1557176272; cv=none; d=outbound.mailhop.org; s=arc-outbound20181012; b=knS7fgC/86RuCkawanYaTH4TouCUXFgNL0UzgNmZgBGhz2OS0iYGa6AZwwuHEX6D/YviqcNtn+b5U 6Fb+ofwpaQfmq8aSst6PBlEw9TgWiTSdIsw3TwGxPlz+rdB3C99g9/CY3Y3Q9vi335z608+sQrekJl V3lrgEv51fhQWhGY1h4jAwoHQsUR8ikK8R2EmWajsS3Q/MAGe0+UXOiJQ93Epyjsf4hLdvHUI4xlzF shq63KfPeX6QEsgBcKw8oFcO6e4RgHXrjmbKlEr9OcaWOTPTle3a7y3KgWDLlqAC4FU6TspdIhraPQ urrTT6va2gEbw7qvYxd/YShGYUQXxJQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=outbound.mailhop.org; s=arc-outbound20181012; h=content-transfer-encoding:mime-version:content-type:references:in-reply-to: date:cc:to:from:subject:message-id:dkim-signature:from; bh=HhAr9gGqIMeKjmWCBsi8aj5mopL5+cZxi6mBDV3dbNM=; b=gSTjIOaSpiBdrLIwWLxQkInmSXR+y0ygNKQKxrL3cQs8zqk6RPr9hrPHB/MyaBh0GKRrvQF84fyhR s+ytkH7MY60B05tezrJIRlh4h4DEE/kb7XNbjWWo+6udMy7oOvqAzcPxzn8EH6A9cAA61gEwMYy6pw qrGsdMROWwPkAisPt2gJz/7oJ3TCCU8HkmL3IvzPBFfADffzP+xDxJElMW+IkDwBTOMnAb0Hkezngh mQ30mQthNFly6PkhwlQPGCNBSEyjY+y4CZlyD7V5Yyvi9Iil508vhvxDukbTaREG95oJu2cg9vw7RM 6VOB6e6ImjsAUFcVPB5FqFk45FYXkNg== ARC-Authentication-Results: i=1; outbound2.eu.mailhop.org; spf=softfail smtp.mailfrom=freebsd.org smtp.remote-ip=67.177.211.60; dmarc=none header.from=freebsd.org; arc=none header.oldest-pass=0; DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=outbound.mailhop.org; s=dkim-high; h=content-transfer-encoding:mime-version:content-type:references:in-reply-to: date:cc:to:from:subject:message-id:from; bh=HhAr9gGqIMeKjmWCBsi8aj5mopL5+cZxi6mBDV3dbNM=; b=D6gzzgXMxyQVhMLzCIP5wLvSy87+UctS88x7LkNreeHb7Oe3KzOLtdaIN619SfVBXaz9iZvAznT6x +jX7vilITXg+2zwek6WTTZ2QpS2kW0r1LUxk0K18Lt6g2WoPw6KMox4KMuvqgyxQ5TT30dEPrAmES6 KrouzAYft+FiNNtIfVW1Woyd7iWeRr/AZcrwkE/+sFMtASnfFCOfx0dieIgdKE5SOzW8KNSgM3kpAs 2HjoVzOAIMzwHoKWQsphqoEVK2HFOGpTl0y0v4Xb+V52rsY0tWhfp/1LB9uVrMjpVoRNEg//5bIHB1 jMVctpJOuBUnPF2CFBwoZf9ajDybqtQ== X-MHO-RoutePath: aGlwcGll X-MHO-User: 98466a3b-7041-11e9-803b-31925da7267c X-Report-Abuse-To: https://support.duocircle.com/support/solutions/articles/5000540958-duocircle-standard-smtp-abuse-information X-Originating-IP: 67.177.211.60 X-Mail-Handler: DuoCircle Outbound SMTP Received: from ilsoft.org (unknown [67.177.211.60]) by outbound2.eu.mailhop.org (Halon) with ESMTPSA id 98466a3b-7041-11e9-803b-31925da7267c; Mon, 06 May 2019 20:57:46 +0000 (UTC) Received: from rev (rev [172.22.42.240]) by ilsoft.org (8.15.2/8.15.2) with ESMTP id x46KvhCi034092; Mon, 6 May 2019 14:57:43 -0600 (MDT) (envelope-from ian@freebsd.org) Message-ID: <2317f6bdf0b9045fe137b5e9080b00de0da64678.camel@freebsd.org> Subject: Re: svn commit: r347063 - head/sys/kern From: Ian Lepore To: Warner Losh , John Baldwin Cc: Mark Johnston , src-committers , svn-src-all , svn-src-head Date: Mon, 06 May 2019 14:57:43 -0600 In-Reply-To: References: <201905032126.x43LQilu092655@repo.freebsd.org> <335d828e-ac61-bc59-bac3-f80f27b951c7@FreeBSD.org> <20190506184502.GA35464@raichu> <52484f6b-fdae-565b-6c03-37a63d56ad30@FreeBSD.org> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.28.5 FreeBSD GNOME Team Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: BDBD38ADB4 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.990,0]; REPLY(-4.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000,0] 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:14:05 -0000 On Mon, 2019-05-06 at 14:47 -0600, Warner Losh wrote: > 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 I do. The first GPS epoch rollover was August 1999, and I've been known to use a gps simulator to drive the system through that event and make sure software handles it correctly. Now I'd probably be more likely to use the rollover that happened last month, but still... some people like to live in the past, at least in simulations. :) I wasn't even happy about the change that prevents setting time near the unix epoch. But, given that there is a tunable/sysctl to disable the sanity checks, it's probably not all that important; we can set debug.allow_insane_settime=1. -- Ian