Re: svn commit: r330602 - head/sys/compat/cloudabi

[Bruce Evans]

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
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r330602 - head/sys/compat/cloudabi

[Warner Losh]

On Wed, Mar 7, 2018 at 8:44 AM, Ed Maste  wrote:

> On 7 March 2018 at 09:47, Eitan Adler  wrote:
> > Author: eadler
> > Date: Wed Mar  7 14:47:43 2018
> > New Revision: 330602
> > URL: https://svnweb.freebsd.org/changeset/base/330602
> >
> > 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?
>

All the standard says are that all named are initialized to 0 (which has
the usual meaning for the integer 0 for pointers) unless otherwise stated:

6.7.8 para 9:

"Except where explicitly stated otherwise, for the purposes of this
subclause unnamed members of objects of structure and union type do not
participate in initialization. Unnamed members of structure objects have
indeterminate value even after initialization."

6.7.8 para 21:

"If there are fewer initializers in a brace-enclosed list than there are
elements or members of an aggregate, or fewer characters in a string
literal used to initialize an array of known size than there are elements
in the array, the remainder of the aggregate shall be initialized
implicitly the same as objects that have static storage duration."

The aggregate here, I believe refers to the elements, not the padded
structure, and the padding is definitely an unnamed part of the structure,
which makes it unspecified.

However 6.7.8 is quite long and detailed, and I'm not 100% sure I have
totally grokked all its subtle implications.

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.
>

I believe that's correct as well.

Warner
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r330602 - head/sys/compat/cloudabi

[Benjamin Kaduk]

On Wed, Mar 7, 2018 at 9:44 AM, Ed Maste  wrote:

> On 7 March 2018 at 09:47, Eitan Adler  wrote:
> > Author: eadler
> > Date: Wed Mar  7 14:47:43 2018
> > New Revision: 330602
> > URL: https://svnweb.freebsd.org/changeset/base/330602
> >
> > 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.
>
>
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.

The "issue" being "fixed" here is that the formal C grammar does not
admit a totally empty initializer, so at least one element of the
initializer
needs to be given in order to satisfy the grammar.  This is pretty silly,
and the extension to allow a totally empty initializer a quite natural one
to make.

-Ben
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r330602 - head/sys/compat/cloudabi

[Ed Maste]

On 7 March 2018 at 09:47, Eitan Adler  wrote:
> Author: eadler
> Date: Wed Mar  7 14:47:43 2018
> New Revision: 330602
> URL: https://svnweb.freebsd.org/changeset/base/330602
>
> 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.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"