From owner-svn-src-all@freebsd.org Sun Feb 26 20:56:52 2017 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 49B43CEC4AA; Sun, 26 Feb 2017 20:56:52 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail107.syd.optusnet.com.au (mail107.syd.optusnet.com.au [211.29.132.53]) by mx1.freebsd.org (Postfix) with ESMTP id D469D11E; Sun, 26 Feb 2017 20:56:51 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from besplex.bde.org (c122-106-153-191.carlnfd1.nsw.optusnet.com.au [122.106.153.191]) by mail107.syd.optusnet.com.au (Postfix) with ESMTPS id E1C6AD41C5F; Mon, 27 Feb 2017 07:56:42 +1100 (AEDT) Date: Mon, 27 Feb 2017 07:56:42 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Dmitry Chagin cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r314293 - head/sys/compat/linux In-Reply-To: <201702260940.v1Q9egOq029307@repo.freebsd.org> Message-ID: <20170227070338.X992@besplex.bde.org> References: <201702260940.v1Q9egOq029307@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=VbSHBBh9 c=1 sm=1 tr=0 a=Tj3pCpwHnMupdyZSltBt7Q==:117 a=Tj3pCpwHnMupdyZSltBt7Q==:17 a=kj9zAlcOel0A:10 a=xh91hpXy2ZdKNyyoVj0A:9 a=CjuIK1q_8ugA:10 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.23 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: Sun, 26 Feb 2017 20:56:52 -0000 On Sun, 26 Feb 2017, Dmitry Chagin wrote: > Log: > Return EOVERFLOW error in case then the size of tv_sec field of struct timespec > in COMPAT_LINUX32 Linuxulator's not equal to the size of native tv_sec. This has several bugs, and it is silly to check the range of times (where overflow can't happen occurs for normal times) while blindly truncating security-related things like uids (where overflow happens for normal uids). > Modified: head/sys/compat/linux/linux_time.c > ============================================================================== > --- head/sys/compat/linux/linux_time.c Sun Feb 26 09:37:25 2017 (r314292) > +++ head/sys/compat/linux/linux_time.c Sun Feb 26 09:40:42 2017 (r314293) > ... > -void > +int > native_to_linux_timespec(struct l_timespec *ltp, struct timespec *ntp) > { > > LIN_SDT_PROBE2(time, native_to_linux_timespec, entry, ltp, ntp); > - > +#ifdef COMPAT_LINUX32 > + if (ntp->tv_sec > INT_MAX && This doesn't use the parametrization that l_time_t is l_long = int32_t when COMPAT_LINUX32, but uses an ugly ifdef, then assumes than int == int32_t. > + sizeof(ltp->tv_sec) != sizeof(ntp->tv_sec)) > + return (EOVERFLOW); > +#endif Negative values are not checked for, so overflow can still occur. Sometimes negative values overlow to a positive value(e.g., bit pattern 0x8000000000000001 -> 1 in 2's complement. Other times they overflow to a negative value (e.g., 0x8000000080000001 -> 0x80000001 in 2's complement). Overflowing values are very unlikely, and the check doesn't prevent them. E.g., even COMPAT_32 APIs might set a time to 0x7fffffff. The time overflows to INT32_MIN 1 second later on 32-bit kernels. The kernel shouldn't allow setting such times, but does. It is left to the user to avoid this foot- shooting. Since setting critical times is privileged, the user usually doesn't do this. On 64-bit kernels, setting such times works and no overflow occurs for native times, but unsuspecting 32-bit applications are broken, so again it is left to privileged users to avoid this foot- shooting (now for 32-bit feet). Similarly for negative times. They are even less useful than times after 32-bit time_t overflows, but in some cases they are not errors so must not be rejected or blindly truncated by conversion functions. Overflow of tv_nsec is not checked for. It really can't happen, since although native time_t is required to be long by historical mistakes, and long might tbe 64 bits or larger, on values between 0 and 10**9-1 are valid for it, and these are representable by 32-bit long. This depends on the conversion function only being from native times which are presumably already active so have been checked for validity. uids overflow routinely. E.g., the error uid is (uid_t)-1 = 0xffffffff and blind truncation of this in linux_uid16.c overflows to 0xffff which is the 16-bit error uid (uid16_t)-1. But 0xffff = 65535 is a valid native uid. The valid native uid 0x10000 overflows to 0 (root). The valid native uid 0xfffffffe (nobody for nfs) overflows to 0xfffe (nobody). Bruce