From owner-svn-src-head@freebsd.org Wed Mar 7 20:42:46 2018 Return-Path: Delivered-To: svn-src-head@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 B2E75F2EC6B; Wed, 7 Mar 2018 20:42:46 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail108.syd.optusnet.com.au (mail108.syd.optusnet.com.au [211.29.132.59]) by mx1.freebsd.org (Postfix) with ESMTP id 7786B6D3A2; Wed, 7 Mar 2018 20:42:45 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from [192.168.0.101] (c110-21-101-228.carlnfd1.nsw.optusnet.com.au [110.21.101.228]) by mail108.syd.optusnet.com.au (Postfix) with ESMTPS id 4364C1A3927; Thu, 8 Mar 2018 07:42:37 +1100 (AEDT) Date: Thu, 8 Mar 2018 07:42:36 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Benjamin Kaduk cc: Ed Maste , Eitan Adler , svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers Subject: Re: svn commit: r330602 - head/sys/compat/cloudabi In-Reply-To: Message-ID: <20180308070722.R1227@besplex.bde.org> References: <201803071447.w27Elh7C053393@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=FNpr/6gs c=1 sm=1 tr=0 a=PalzARQSbocsUSjMRkwAPg==:117 a=PalzARQSbocsUSjMRkwAPg==:17 a=kj9zAlcOel0A:10 a=6I5d2MoRAAAA:8 a=gcnCtRpcewpaml6cjn8A:9 a=CjuIK1q_8ugA:10 a=IjZwj45LgO3ly-622nXo:22 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.25 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, 07 Mar 2018 20:42:46 -0000 On Wed, 7 Mar 2018, Benjamin Kaduk wrote: > On Wed, Mar 7, 2018 at 9:44 AM, Ed Maste wrote: > >> On 7 March 2018 at 09:47, Eitan Adler wrote: >>> ... >>> Log: >>> sys/cloudabi: Avoid relying on GNU specific extensions >>> >>> An empty initializer list is not technically valid C grammar. >>> >>> MFC After: 1 week >>> >>> - cloudabi_fdstat_t fsb = {}; >>> + cloudabi_fdstat_t fsb = {0}; >> >> In practice it appears initializing via { 0 } also zeros any padding >> in the struct, but I do not believe it's required by the C standard. >> Perhaps a language lawyer can weigh in? >> >> Commenting on this commit just because it's highlighted by this >> change; I do not believe there's a difference between the GNU >> extension { } and { 0 } here. It is also a style bug to initialize variables in declarations. It is interesting that this style bug gives other bugs that are more serious than when the style rule was new: - locking is often needed before complicated initializations. It would be an even larger style bug to write the lock acquisition in initializers, and C doesn't have finalizers so it is impossible to obfuscate the lock release by writing it in finalizers - this problem of initializing padding. Auto initializers also used to be good for pessimizations. Use them instead of static initializers for constant values. This asks the compiler to initialize them on every entry to the function. It was a typical implementation to keep the values in an unnamed static object and copy this to the stack at runtime. Now compilers are more likely to optimize away the copying, so the pessimization doesn't work so well. Copying from a static object tends to give zero padding, but optimizated variants should only give zero padding if that is optimal. > The C spec says that if an incomplete initializer is given, then all other > fields > of the structure are initialized to zero. The state of padding is > unspecified, > whether a complete or incomplete initializer is given. This seems to apply to static objects too. So initializing dynamic objects by copying them from static objects is insecure even if you copy using memcpy() (copying using struct assignment might skip the padding so shouldn't be used). A malicious compiler could initialize the padding with security-related info. non-malicious compiler might initialize the padding with stack garbage that happens to be security-related. Bruce