public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Richard Biener <rguenther@suse.de>
Cc: Qing Zhao <qing.zhao@oracle.com>,
	richard Sandiford <richard.sandiford@arm.com>,
	gcc-patches Qing Zhao via <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH][version 3]add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
Date: Mon, 7 Jun 2021 16:45:59 -0700	[thread overview]
Message-ID: <202106071641.6CB3850@keescook> (raw)
In-Reply-To: <nycvar.YFH.7.76.2106070937340.9200@zhemvz.fhfr.qr>

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.

-- 
Kees Cook

  parent reply	other threads:[~2021-06-07 23:46 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-12 17:16 Qing Zhao
2021-05-25 19:26 ` Qing Zhao
2021-05-26 11:18 ` Richard Biener
2021-05-27 19:44   ` Qing Zhao
2021-06-07  7:48     ` Richard Biener
2021-06-07 16:13       ` Qing Zhao
2021-06-08  7:37         ` Richard Biener
2021-06-08 16:56           ` Kees Cook
2021-06-08 17:32             ` Qing Zhao
2021-06-08 17:36               ` Kees Cook
2021-06-07 23:45       ` Kees Cook [this message]
2021-06-08  8:27         ` Richard Biener
2021-05-27 21:42   ` Qing Zhao
2021-06-03 20:14   ` Qing Zhao
2021-06-07  7:50     ` Richard Biener
2021-06-03 20:18   ` Qing Zhao
2021-06-07  7:53     ` Richard Biener
2021-06-07 16:18       ` Qing Zhao
2021-06-07 23:48         ` Kees Cook
2021-06-08  7:41         ` Richard Biener
2021-06-08 15:27           ` Qing Zhao
2021-06-08 16:59           ` Kees Cook
2021-06-08 18:05             ` Qing Zhao
2021-06-11 11:04             ` Richard Biener
2021-06-11 17:14               ` Kees Cook
2021-06-10 21:11   ` Qing Zhao
2021-06-11 11:12     ` Richard Biener
2021-06-11 15:49       ` Qing Zhao
2021-06-11 16:24         ` Kees Cook
2021-06-11 17:00         ` Qing Zhao
2021-06-14 16:10         ` Qing Zhao
2021-06-15 13:21           ` Richard Biener
2021-06-15 21:49             ` Qing Zhao
2021-06-16  6:19               ` Richard Biener
2021-06-16 15:04                 ` Qing Zhao
2021-06-16 19:39                   ` Qing Zhao
2021-06-18 23:47                     ` Kees Cook
2021-06-21 15:39                       ` Qing Zhao
2021-06-21 16:18                         ` Kees Cook
2021-06-21 17:11                           ` Qing Zhao
2021-06-22  8:25                           ` Richard Sandiford
2021-06-22  8:59                             ` Richard Biener
2021-06-22 13:54                               ` Qing Zhao
2021-06-22 14:00                                 ` Richard Biener
2021-06-22 14:10                                   ` Qing Zhao
2021-06-22 14:15                                     ` Richard Biener
2021-06-22 14:33                                       ` Qing Zhao
2021-06-22 19:04                                         ` Richard Biener
2021-06-22 17:55                             ` Kees Cook
2021-06-22 18:18                               ` Richard Sandiford
2021-06-22 21:31                                 ` Qing Zhao
2021-06-23  6:05                                   ` Richard Biener
2021-06-21  7:53                   ` Richard Biener
2021-06-21 15:11                     ` Qing Zhao
2021-06-21 15:35                       ` Richard Biener
2021-06-21 16:13                         ` Qing Zhao
2021-06-22  6:24                           ` Richard Biener

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=202106071641.6CB3850@keescook \
    --to=keescook@chromium.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=qing.zhao@oracle.com \
    --cc=rguenther@suse.de \
    --cc=richard.sandiford@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).