Date: Thu, 8 Mar 2018 07:42:36 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Benjamin Kaduk <bjkfbsd@gmail.com> Cc: Ed Maste <emaste@freebsd.org>, Eitan Adler <eadler@freebsd.org>, svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers <src-committers@freebsd.org> Subject: Re: svn commit: r330602 - head/sys/compat/cloudabi Message-ID: <20180308070722.R1227@besplex.bde.org> In-Reply-To: <CAJ5_RoD%2BQVJYK9%2B5OzTJoHA2gVVTWRL4yNYNZ7SKMkn70pvevw@mail.gmail.com> References: <201803071447.w27Elh7C053393@repo.freebsd.org> <CAPyFy2DOrHNs2BEQVqJYBGxT4iiA3ms_crXcX=q0wXr=63ddug@mail.gmail.com> <CAJ5_RoD%2BQVJYK9%2B5OzTJoHA2gVVTWRL4yNYNZ7SKMkn70pvevw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 7 Mar 2018, Benjamin Kaduk wrote:
> On Wed, Mar 7, 2018 at 9:44 AM, Ed Maste <emaste@freebsd.org> wrote:
>
>> On 7 March 2018 at 09:47, Eitan Adler <eadler@freebsd.org> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180308070722.R1227>