From owner-svn-src-head@freebsd.org Wed Oct 23 12:18:13 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 D3A98150831; Wed, 23 Oct 2019 12:18:13 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail104.syd.optusnet.com.au (mail104.syd.optusnet.com.au [211.29.132.246]) by mx1.freebsd.org (Postfix) with ESMTP id 46yqCS127Dz43DV; Wed, 23 Oct 2019 12:18:11 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from [192.168.0.102] (c110-21-101-228.carlnfd1.nsw.optusnet.com.au [110.21.101.228]) by mail104.syd.optusnet.com.au (Postfix) with ESMTPS id 9972343E6DE; Wed, 23 Oct 2019 23:18:07 +1100 (AEDT) Date: Wed, 23 Oct 2019 23:18:06 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Alan Somers cc: Andrew Turner , src-committers , svn-src-all , svn-src-head Subject: Re: svn commit: r353640 - head/sys/kern In-Reply-To: Message-ID: <20191023210546.L892@besplex.bde.org> References: <201910161321.x9GDL2ee021543@repo.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.2 cv=P6RKvmIu c=1 sm=1 tr=0 a=PalzARQSbocsUSjMRkwAPg==:117 a=PalzARQSbocsUSjMRkwAPg==:17 a=jpOVt7BSZ2e4Z31A5e1TngXxSK0=:19 a=kj9zAlcOel0A:10 a=6I5d2MoRAAAA:8 a=4zIc5qUCshN3e9hYGH4A:9 a=CjuIK1q_8ugA:10 a=IjZwj45LgO3ly-622nXo:22 X-Rspamd-Queue-Id: 46yqCS127Dz43DV X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org; dkim=none; dmarc=none; spf=pass (mx1.freebsd.org: domain of brde@optusnet.com.au designates 211.29.132.246 as permitted sender) smtp.mailfrom=brde@optusnet.com.au X-Spamd-Result: default: False [-2.30 / 15.00]; ARC_NA(0.00)[]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; RCVD_COUNT_TWO(0.00)[2]; FROM_HAS_DN(0.00)[]; R_SPF_ALLOW(-0.20)[+ip4:211.29.132.0/23]; FREEMAIL_FROM(0.00)[optusnet.com.au]; MIME_GOOD(-0.10)[text/plain]; TO_MATCH_ENVRCPT_ALL(0.00)[]; DMARC_NA(0.00)[optusnet.com.au]; IP_SCORE_FREEMAIL(0.00)[]; RCPT_COUNT_FIVE(0.00)[5]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; IP_SCORE(0.00)[ip: (-6.93), ipnet: 211.28.0.0/14(-3.22), asn: 4804(-2.38), country: AU(0.01)]; TO_DN_ALL(0.00)[]; RCVD_NO_TLS_LAST(0.10)[]; RCVD_IN_DNSWL_LOW(-0.10)[246.132.29.211.list.dnswl.org : 127.0.5.1]; R_DKIM_NA(0.00)[]; FREEMAIL_ENVFROM(0.00)[optusnet.com.au]; ASN(0.00)[asn:4804, ipnet:211.28.0.0/14, country:AU]; MIME_TRACE(0.00)[0:+]; FROM_EQ_ENVFROM(0.00)[] 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 12:18:13 -0000 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. 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