From owner-svn-src-head@freebsd.org Wed Oct 23 13:37:52 2019 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id B3872152B75; Wed, 23 Oct 2019 13:37:52 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 46yrzM4dmjz47hF; Wed, 23 Oct 2019 13:37:51 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.15.2/8.15.2) with ESMTPS id x9NDbhUO021432 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NO); Wed, 23 Oct 2019 16:37:46 +0300 (EEST) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua x9NDbhUO021432 Received: (from kostik@localhost) by tom.home (8.15.2/8.15.2/Submit) id x9NDbfWl021431; Wed, 23 Oct 2019 16:37:41 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Wed, 23 Oct 2019 16:37:41 +0300 From: Konstantin Belousov To: Bruce Evans Cc: Alan Somers , Andrew Turner , src-committers , svn-src-all , svn-src-head Subject: Re: svn commit: r353640 - head/sys/kern Message-ID: <20191023133741.GT73312@kib.kiev.ua> References: <201910161321.x9GDL2ee021543@repo.freebsd.org> <20191023210546.L892@besplex.bde.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20191023210546.L892@besplex.bde.org> User-Agent: Mutt/1.12.2 (2019-09-21) X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FORGED_GMAIL_RCVD,FREEMAIL_FROM, NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on tom.home X-Rspamd-Queue-Id: 46yrzM4dmjz47hF X-Spamd-Bar: - Authentication-Results: mx1.freebsd.org; dkim=none; dmarc=fail reason="No valid SPF, No valid DKIM" header.from=gmail.com (policy=none); spf=softfail (mx1.freebsd.org: 2001:470:d5e7:1::1 is neither permitted nor denied by domain of kostikbel@gmail.com) smtp.mailfrom=kostikbel@gmail.com X-Spamd-Result: default: False [-2.00 / 15.00]; ARC_NA(0.00)[]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; RCVD_TLS_ALL(0.00)[]; FROM_HAS_DN(0.00)[]; FREEMAIL_FROM(0.00)[gmail.com]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; MIME_GOOD(-0.10)[text/plain]; HAS_XAW(0.00)[]; R_SPF_SOFTFAIL(0.00)[~all]; RCPT_COUNT_FIVE(0.00)[6]; IP_SCORE_FREEMAIL(0.00)[]; TO_MATCH_ENVRCPT_SOME(0.00)[]; TO_DN_ALL(0.00)[]; IP_SCORE(0.00)[ip: (-2.78), ipnet: 2001:470::/32(-4.60), asn: 6939(-3.45), country: US(-0.05)]; FREEMAIL_TO(0.00)[optusnet.com.au]; FROM_EQ_ENVFROM(0.00)[]; R_DKIM_NA(0.00)[]; MIME_TRACE(0.00)[0:+]; ASN(0.00)[asn:6939, ipnet:2001:470::/32, country:US]; RCVD_COUNT_TWO(0.00)[2]; FREEMAIL_ENVFROM(0.00)[gmail.com]; DMARC_POLICY_SOFTFAIL(0.10)[gmail.com : No valid SPF, No valid DKIM,none] X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 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, 23 Oct 2019 13:37:52 -0000 On Wed, Oct 23, 2019 at 11:18:06PM +1100, Bruce Evans wrote: > On Tue, 22 Oct 2019, Alan Somers wrote: > > > On Wed, Oct 16, 2019 at 7:21 AM Andrew Turner wrote: > > > >> Author: andrew > >> Date: Wed Oct 16 13:21:01 2019 > >> New Revision: 353640 > >> URL: https://svnweb.freebsd.org/changeset/base/353640 > >> > >> Log: > >> Stop leaking information from the kernel through timespec > >> > >> The timespec struct holds a seconds value in a time_t and a nanoseconds > >> value in a long. On most architectures these are the same size, however > >> on 32-bit architectures other than i386 time_t is 8 bytes and long is > >> 4 bytes. > >> > >> Most ABIs will then pad a struct holding an 8 byte and 4 byte value to > >> 16 bytes with 4 bytes of padding. When copying one of these structs the > >> compiler is free to copy the padding if it wishes. > > I doubt that most ABIs pad the struct in that case. Pure 32-bit arches > don't have any 64-bit memory or register operations, so they use 32-bit > longs and don't pessimize structs with 64-bit types in them using padding. > > If there are any such ABIs, then there are thousands if not millions more > places to fix. Every place where a timespec is written must be checked. 32bit ARM requires 8-byte alignment for 64bit integers AFAIR. Also I believe that this is the only such 32bit architecture supported by FreeBSD. > > E.g., gettimeofday() reduces to microtime(). The implementation of > microtime() happens to fill the timeval fields separately. That always > leaks kernel context if struct timeval has padding and nothing else > zeros the padding. And for gettimeofday() it is especially clear that > nothing else zeros the padding. sys_gettimeofday() allocates a timeval > on the kernel stack; microtime() initializes its fields separately > leaving any padding untouched; copyout() then copies out kernel context. > > This commit attempts to fix the different problem that if lower levels > like microtime() were pessimized to initialize the padding, then callers > would have to be careful to preserve this by not using struct > assignments for copying structs. This is not easy to do. bcopy() and > copyout() should preserve memory, but there are no guarantees if the > struct is accessed in other ways. The fix in this commit can easily > not work on arches that have the problem: > > bzero(&tv); /* padding now all 0 */ > tv.tv_sec = another_tv_sec; > /* > * Padding now indeterminate (C99 6.2.6.1p6). It is reasonable > * for the implementation to use the padding on ABIs that have it, > * 64-bit stores of registers with uncleared kernel context gives > * gives the context leak. > */ > tv.tv_nsec = another_tv_nsec; > > I don't see anything in C99 that requires padding to remain unchanged > even with no stores in the program. > > This problem affects more than time_t's. It affects all padding. > > I next checked the simplest itimer call. sys_getitimer() doesn't > handle the problem, of course: it allocates a struct itimerval > aitimerval the kernel stack; kern_getitimer() allocates a struct timeval > ctv on the kernel stack; ctv is initialized 1 field at a time in most > cases so its padding remains uninitialized; aitv is inititialized using > a struct assignment in most cases so it has the problem that this > commit attempts to fix. > > This is just another bug in 64-bit time_t's, especially on 32-bit arches. > Instead of 32-bit time_t's which work until 2038 or 2106, we have subtle > security holes now. > > Bruce