On Mon, 7 Jun 2021, Kees Cook wrote:
> On Mon, Jun 07, 2021 at 09:48:41AM +0200, Richard Biener wrote:
> > On Thu, 27 May 2021, Qing Zhao wrote:
> > > @@ -5001,6 +5185,17 @@ gimplify_init_constructor (tree *expr_p, gimple_seq
> > > *pre_p, gimple_seq *post_p,
> > > /* If a single access to the target must be ensured and all
> > > elements
> > > are zero, then it's optimal to clear whatever their number.
> > > */
> > > cleared = true;
> > > + else if (flag_trivial_auto_var_init > AUTO_INIT_UNINITIALIZED
> > > + && !TREE_STATIC (object)
> > > + && type_has_padding (type))
> > > + /* If the user requests to initialize automatic variables with
> > > + paddings inside the type, we should initialize the paddings
> > > too.
> > > + C guarantees that brace-init with fewer initializers than
> > > members
> > > + aggregate will initialize the rest of the aggregate as-if it
> > > were
> > > + static initialization. In turn static initialization
> > > guarantees
> > > + that pad is initialized to zero bits.
> > > + So, it's better to clear the whole record under such
> > > situation. */
> > > + cleared = true;
> > >
> > > so here we have padding as well - I think this warrants to be controlled
> > > by an extra option? And we can maybe split this out to a separate
> > > patch? (the whole padding stuff)
> > >
> > > Clang does the padding initialization with this option, shall we be
> > > consistent with Clang?
> >
> > Just for the sake of consistency? No. Is there a technical reason
> > for this complication? Say we have
> >
> > struct { short s; int i; } a;
> >
> > what's the technical reason to initialize the padding? I might
> > be tempted to use -ftrivial-auto-init but I'd definitely don't
> > want to spend cycles/instructions initializing the padding in the
> > above struct.
>
> Yes, this is very important. This is one of the more common ways memory
> content leaks happen in programs (especially the kernel). e.g.:
>
> struct example {
> short s;
> int i;
> };
>
> struct example instance = { .i = foo };
>
> While "s" gets zeroed, the padding may not, and may contain prior memory
> contents. Having this be deterministically zero is important for this
> feature. If the structure gets byte-copied to a buffer (e.g. syscall,
> etc), the padding will go along for the ride.
OK, so IMHO this is really a separate feature then - note that even
allocated memory suffers from this issue if the allocator does not
zero allocated blocks in full. It then applies to even fully correctly
initialized objects and would eventually be better suited for a
language extension.
That said, the user documentation should elaborate on use cases.
pre-zeroing of fields makes bugs due to uninitialized accesses
more "reliable", zeroing of everything avoids information leaks.
All only for auto-vars (I suppose securing of allocated storage
should then happen in the allocator itself and thus likely a bit
less optimized).
Btw, see my other suggestion about simply making sure to pre-initialize
the whole frame at its allocation point.
Richard.
