[1/2] gcc-plugins: structleak: Generalize to all variable types
diff mbox series

Message ID 20190212180441.15340-2-keescook@chromium.org
State New
Headers show
Series
  • gcc-plugins: structleak: Generalize to all variable types
Related show

Commit Message

Kees Cook Feb. 12, 2019, 6:04 p.m. UTC
This adjusts structleak to also work with non-struct types when they
are passed by reference, since those variables may leak just like
anything else. This is exposed via an improved set of Kconfig options.
(This does mean structleak is slightly misnamed now.)

Building with CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL should give the
kernel complete initialization coverage of all stack variables passed
by reference, including padding (see lib/test_stackinit.c).

Using CONFIG_GCC_PLUGIN_STRUCTLEAK_VERBOSE to count added initializations
under defconfig:

	..._BYREF:      5945 added initializations
	..._BYREF_ALL: 16606 added initializations

There is virtually no change to text+data size (both have less than 0.05%
growth):

   text    data     bss     dec     hex filename
19502103        5051456 1917000 26470559        193e89f vmlinux.stock
19513412        5051456 1908808 26473676        193f4cc vmlinux.byref
19516974        5047360 1900616 26464950        193d2b6 vmlinux.byref_all

The measured performance difference is in the noise for hackbench and
kernel build benchmarks:

Stock:

	5x hackbench -g 20 -l 1000
	Mean:   10.649s
	Std Dev: 0.339

	5x kernel build (4-way parallel)
	Mean:  261.98s
	Std Dev: 1.53

CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF:

	5x hackbench -g 20 -l 1000
	Mean:   10.540s
	Std Dev: 0.233

	5x kernel build (4-way parallel)
	Mean:  260.52s
	Std Dev: 1.31

CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL:

	5x hackbench -g 20 -l 1000
	Mean:   10.320
	Std Dev: 0.413

	5x kernel build (4-way parallel)
	Mean:  260.10
	Std Dev: 0.86

This does not yet solve missing padding initialization for structures
on the stack that are never passed by reference (which should be a tiny
minority). Hopefully this will be more easily addressed by upstream
compiler fixes after clarifying the C11 padding initialization
specification.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 scripts/Makefile.gcc-plugins            |  2 +
 scripts/gcc-plugins/Kconfig             | 58 ++++++++++++++++++++-----
 scripts/gcc-plugins/structleak_plugin.c | 36 ++++++++++-----
 3 files changed, 74 insertions(+), 22 deletions(-)

Comments

Arnd Bergmann Feb. 28, 2019, 8:27 p.m. UTC | #1
On Tue, Feb 12, 2019 at 7:08 PM Kees Cook <keescook@chromium.org> wrote:
>
> This adjusts structleak to also work with non-struct types when they
> are passed by reference, since those variables may leak just like
> anything else. This is exposed via an improved set of Kconfig options.
> (This does mean structleak is slightly misnamed now.)
>
> Building with CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL should give the
> kernel complete initialization coverage of all stack variables passed
> by reference, including padding (see lib/test_stackinit.c).
>
> Using CONFIG_GCC_PLUGIN_STRUCTLEAK_VERBOSE to count added initializations
> under defconfig:
>
>         ..._BYREF:      5945 added initializations
>         ..._BYREF_ALL: 16606 added initializations
>
> There is virtually no change to text+data size (both have less than 0.05%
> growth):

I just resumed my randconfig build testing after a longer break, and found
a regression for stack usage that I bisected to your change. It shows up in a
variety of files depending on the configuration, so far the worst one is the
configuration at https://pastebin.com/UK54qbKa that leads to

../drivers/media/dvb-frontends/stv090x.c: In function 'stv090x_start_search':
../drivers/media/dvb-frontends/stv090x.c:1595:1: error: the frame size
of 5320 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
../drivers/media/dvb-frontends/stv090x.c: In function 'stv090x_optimize_track':
../drivers/media/dvb-frontends/stv090x.c:3090:1: error: the frame size
of 5872 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
../drivers/media/dvb-frontends/stv090x.c: In function 'stv090x_algo':
../drivers/media/dvb-frontends/stv090x.c:3431:1: error: the frame size
of 5144 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
 }

At least for this specific file, I also see a significant (though not alarming)
increase in code size:

   text       data        bss        dec        hex    filename
179196       4632        256     184084      2cf14
obj-x86/drivers/media/dvb-frontends/stv090x-before.o
 216740       4632        256     221628      361bc
obj-x86/drivers/media/dvb-frontends/stv090x-after.o

Part of the problem here is definitely interaction with the asan-stack
sanitizer. Changing asan-stack=1 to asan-stack=0, it looks a lot
better:

../drivers/media/dvb-frontends/stv090x.c: In function 'stv090x_start_search':
../drivers/media/dvb-frontends/stv090x.c:1595:1: warning: the frame
size of 120 bytes is larger than 20 bytes [-Wframe-larger-than=]
../drivers/media/dvb-frontends/stv090x.c: In function 'stv090x_optimize_track':
../drivers/media/dvb-frontends/stv090x.c:3090:1: warning: the frame
size of 168 bytes is larger than 20 bytes [-Wframe-larger-than=]
../drivers/media/dvb-frontends/stv090x.c: In function 'stv090x_algo':
../drivers/media/dvb-frontends/stv090x.c:3431:1: warning: the frame
size of 192 bytes is larger than 20 bytes [-Wframe-larger-than=]

   text       data        bss        dec        hex    filename
184061       4632        256     188949      2e215
obj-x86/drivers/media/dvb-frontends/stv090x.o

I get similar results with asan-stack=1 but without your plugin, only
the combination of the two has the explosive stack size growth.

I can help analyze this further, but maybe you can have a look first,
there might be something obvious when you read the input to the
plugin.

      Arnd
Ard Biesheuvel March 2, 2019, 9:04 a.m. UTC | #2
On Thu, 28 Feb 2019 at 21:27, Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, Feb 12, 2019 at 7:08 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > This adjusts structleak to also work with non-struct types when they
> > are passed by reference, since those variables may leak just like
> > anything else. This is exposed via an improved set of Kconfig options.
> > (This does mean structleak is slightly misnamed now.)
> >
> > Building with CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL should give the
> > kernel complete initialization coverage of all stack variables passed
> > by reference, including padding (see lib/test_stackinit.c).
> >
> > Using CONFIG_GCC_PLUGIN_STRUCTLEAK_VERBOSE to count added initializations
> > under defconfig:
> >
> >         ..._BYREF:      5945 added initializations
> >         ..._BYREF_ALL: 16606 added initializations
> >
> > There is virtually no change to text+data size (both have less than 0.05%
> > growth):
>
> I just resumed my randconfig build testing after a longer break, and found
> a regression for stack usage that I bisected to your change. It shows up in a
> variety of files depending on the configuration, so far the worst one is the
> configuration at https://pastebin.com/UK54qbKa that leads to
>
> ../drivers/media/dvb-frontends/stv090x.c: In function 'stv090x_start_search':
> ../drivers/media/dvb-frontends/stv090x.c:1595:1: error: the frame size
> of 5320 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
> ../drivers/media/dvb-frontends/stv090x.c: In function 'stv090x_optimize_track':
> ../drivers/media/dvb-frontends/stv090x.c:3090:1: error: the frame size
> of 5872 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
> ../drivers/media/dvb-frontends/stv090x.c: In function 'stv090x_algo':
> ../drivers/media/dvb-frontends/stv090x.c:3431:1: error: the frame size
> of 5144 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
>  }
>
> At least for this specific file, I also see a significant (though not alarming)
> increase in code size:
>
>    text       data        bss        dec        hex    filename
> 179196       4632        256     184084      2cf14
> obj-x86/drivers/media/dvb-frontends/stv090x-before.o
>  216740       4632        256     221628      361bc
> obj-x86/drivers/media/dvb-frontends/stv090x-after.o
>
> Part of the problem here is definitely interaction with the asan-stack
> sanitizer. Changing asan-stack=1 to asan-stack=0, it looks a lot
> better:
>
> ../drivers/media/dvb-frontends/stv090x.c: In function 'stv090x_start_search':
> ../drivers/media/dvb-frontends/stv090x.c:1595:1: warning: the frame
> size of 120 bytes is larger than 20 bytes [-Wframe-larger-than=]
> ../drivers/media/dvb-frontends/stv090x.c: In function 'stv090x_optimize_track':
> ../drivers/media/dvb-frontends/stv090x.c:3090:1: warning: the frame
> size of 168 bytes is larger than 20 bytes [-Wframe-larger-than=]
> ../drivers/media/dvb-frontends/stv090x.c: In function 'stv090x_algo':
> ../drivers/media/dvb-frontends/stv090x.c:3431:1: warning: the frame
> size of 192 bytes is larger than 20 bytes [-Wframe-larger-than=]
>
>    text       data        bss        dec        hex    filename
> 184061       4632        256     188949      2e215
> obj-x86/drivers/media/dvb-frontends/stv090x.o
>
> I get similar results with asan-stack=1 but without your plugin, only
> the combination of the two has the explosive stack size growth.
>
> I can help analyze this further, but maybe you can have a look first,
> there might be something obvious when you read the input to the
> plugin.
>

Is this before or after use-after-scope was disabled entirely?
Kees Cook March 2, 2019, 3:43 p.m. UTC | #3
On Sat, Mar 2, 2019 at 1:05 AM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> On Thu, 28 Feb 2019 at 21:27, Arnd Bergmann <arnd@arndb.de> wrote:
> > I get similar results with asan-stack=1 but without your plugin, only
> > the combination of the two has the explosive stack size growth.

I can look more closely, but I'm not sure it's entirely worth it:
these two may not make sense to build at the same time. (e.g. the
use-after-scope config was disallowed to work with this plugin.)

> > I can help analyze this further, but maybe you can have a look first,
> > there might be something obvious when you read the input to the
> > plugin.
> >
>
> Is this before or after use-after-scope was disabled entirely?

I was wondering the same thing, but I assumed it didn't matter: it
wasn't possible to use both before it was entirely disabled.
Arnd Bergmann March 2, 2019, 10:15 p.m. UTC | #4
On Sat, Mar 2, 2019 at 4:43 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Sat, Mar 2, 2019 at 1:05 AM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >
> > On Thu, 28 Feb 2019 at 21:27, Arnd Bergmann <arnd@arndb.de> wrote:
> > > I get similar results with asan-stack=1 but without your plugin, only
> > > the combination of the two has the explosive stack size growth.
>
> I can look more closely, but I'm not sure it's entirely worth it:
> these two may not make sense to build at the same time. (e.g. the
> use-after-scope config was disallowed to work with this plugin.)

Well, I still want to make sure all 'randconfig' builds complete without
warnings, and without having to turn off the otherwise useful
stack overflow warnings.

One thing I noticed is that your patch removes the 'depends on
!COMPILE_TEST' check for GCC_PLUGIN_STRUCTLEAK_BYREF_ALL,
so if we add that back in, it would at least take care of the
allmodconfig and randconfig cases.

> > > I can help analyze this further, but maybe you can have a look first,
> > > there might be something obvious when you read the input to the
> > > plugin.
> > >
> >
> > Is this before or after use-after-scope was disabled entirely?
>
> I was wondering the same thing, but I assumed it didn't matter: it
> wasn't possible to use both before it was entirely disabled.

Right. I already had the use-after-scope stuff disabled for
build testing, using the same 'depends on !COMPILE_TEST'
check, so one more reason it did not make a difference.

      Arnd
Kees Cook March 4, 2019, 5:32 p.m. UTC | #5
On Sat, Mar 2, 2019 at 2:16 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Sat, Mar 2, 2019 at 4:43 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > On Sat, Mar 2, 2019 at 1:05 AM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > >
> > > On Thu, 28 Feb 2019 at 21:27, Arnd Bergmann <arnd@arndb.de> wrote:
> > > > I get similar results with asan-stack=1 but without your plugin, only
> > > > the combination of the two has the explosive stack size growth.
> >
> > I can look more closely, but I'm not sure it's entirely worth it:
> > these two may not make sense to build at the same time. (e.g. the
> > use-after-scope config was disallowed to work with this plugin.)
>
> Well, I still want to make sure all 'randconfig' builds complete without
> warnings, and without having to turn off the otherwise useful
> stack overflow warnings.
>
> One thing I noticed is that your patch removes the 'depends on
> !COMPILE_TEST' check for GCC_PLUGIN_STRUCTLEAK_BYREF_ALL,
> so if we add that back in, it would at least take care of the
> allmodconfig and randconfig cases.

Okay, I'll add this back in the next few days.
Alexander Popov March 11, 2019, 11:05 p.m. UTC | #6
Hello Kees, hello everyone,

On 12.02.2019 21:04, Kees Cook wrote:
> Building with CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL should give the
> kernel complete initialization coverage of all stack variables passed
> by reference, including padding (see lib/test_stackinit.c).

I would like to note that new STRUCTLEAK_BYREF_ALL initializes *less* stack
variables than STACKINIT, that was introduced earlier:
https://www.openwall.com/lists/kernel-hardening/2019/01/23/3

Citing the patches:
- the STACKINIT plugin "attempts to perform unconditional initialization of all
stack variables";
- the STRUCTLEAK_BYREF_ALL feature "gives the kernel complete initialization
coverage of all stack variables passed by reference".

I.e. stack variables not passed by reference are not initialized by
STRUCTLEAK_BYREF_ALL.

Kees, what do you think about adding such cases to your lib/test_stackinit.c?
This simple example demonstrates the idea:


diff --git a/lib/test_stackinit.c b/lib/test_stackinit.c
index 13115b6..f9ef313 100644
--- a/lib/test_stackinit.c
+++ b/lib/test_stackinit.c
@@ -320,9 +320,18 @@ static noinline __init int leaf_switch_2_none(unsigned long sp, bool fill,
 DEFINE_TEST_DRIVER(switch_1_none, uint64_t, SCALAR);
 DEFINE_TEST_DRIVER(switch_2_none, uint64_t, SCALAR);

+struct x {
+       int x1;
+       int x2;
+       int x3;
+};
+
 static int __init test_stackinit_init(void)
 {
        unsigned int failures = 0;
+       struct x _x;
+
+       printk("uninitialized struct fields sum: %d\n", _x.x1 + _x.x2 + _x.x3);

 #define test_scalars(init)     do {                            \
                failures += test_u8_ ## init ();                \


Kernel output:
  root@vm:~# insmod /lib/modules/5.0.0+/kernel/lib/test_stackinit.ko
  [   40.534622] uninitialized struct fields sum: -727800841

Best regards,
Alexander
Kees Cook March 13, 2019, 7:01 p.m. UTC | #7
On Mon, Mar 11, 2019 at 4:05 PM Alexander Popov <alex.popov@linux.com> wrote:
> Hello Kees, hello everyone,
>
> On 12.02.2019 21:04, Kees Cook wrote:
> > Building with CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL should give the
> > kernel complete initialization coverage of all stack variables passed
> > by reference, including padding (see lib/test_stackinit.c).
>
> I would like to note that new STRUCTLEAK_BYREF_ALL initializes *less* stack
> variables than STACKINIT, that was introduced earlier:
> https://www.openwall.com/lists/kernel-hardening/2019/01/23/3
>
> Citing the patches:
> - the STACKINIT plugin "attempts to perform unconditional initialization of all
> stack variables";
> - the STRUCTLEAK_BYREF_ALL feature "gives the kernel complete initialization
> coverage of all stack variables passed by reference".

That's true, yes. STACKINIT was a port of Florian's patch to gcc which
looked only for missing initialization. However, this collides with
warnings about missing initialization. :)

So, given the case that the kernel builds with -Wuninitialized and
-Wmaybe-uninitialized, the preferred method of dealing with
non-by-reference missed initializations is to fix the code itself.
(i.e. kernel builds are expected to build without warnings.) It's only
the byref cases that there is no warning (and most authors refuse to
initialize such cases). Have there been security flaws where gcc
failed to warn a missed initialization that wasn't a byref case?

> I.e. stack variables not passed by reference are not initialized by
> STRUCTLEAK_BYREF_ALL.

Correct.

> Kees, what do you think about adding such cases to your lib/test_stackinit.c?
> This simple example demonstrates the idea:
>
>
> diff --git a/lib/test_stackinit.c b/lib/test_stackinit.c
> index 13115b6..f9ef313 100644
> --- a/lib/test_stackinit.c
> +++ b/lib/test_stackinit.c
> @@ -320,9 +320,18 @@ static noinline __init int leaf_switch_2_none(unsigned long sp, bool fill,
>  DEFINE_TEST_DRIVER(switch_1_none, uint64_t, SCALAR);
>  DEFINE_TEST_DRIVER(switch_2_none, uint64_t, SCALAR);
>
> +struct x {
> +       int x1;
> +       int x2;
> +       int x3;
> +};
> +
>  static int __init test_stackinit_init(void)
>  {
>         unsigned int failures = 0;
> +       struct x _x;
> +
> +       printk("uninitialized struct fields sum: %d\n", _x.x1 + _x.x2 + _x.x3);

This would trip the build warnings:

In file included from ./include/linux/kernel.h:15:0,
                 from lib/test_stackinit.c:9:
lib/test_stackinit.c: In function ‘test_stackinit_init’:
./include/linux/printk.h:309:2: warning: ‘__x.x1’ is used
uninitialized in this function [-Wuninitialized]
  printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
  ^~~~~~

but those could be silenced for this object specifically if we really
wanted to add it. I think it'd be fine to add this to the test, but
it's a known state, though, so I hadn't bothered with it.

Patch
diff mbox series

diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
index 35042d96cf5d..5f7df50cfe7a 100644
--- a/scripts/Makefile.gcc-plugins
+++ b/scripts/Makefile.gcc-plugins
@@ -15,6 +15,8 @@  gcc-plugin-$(CONFIG_GCC_PLUGIN_SANCOV)		+= sancov_plugin.so
 gcc-plugin-$(CONFIG_GCC_PLUGIN_STRUCTLEAK)	+= structleak_plugin.so
 gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK_VERBOSE)	\
 		+= -fplugin-arg-structleak_plugin-verbose
+gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF)		\
+		+= -fplugin-arg-structleak_plugin-byref
 gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL)	\
 		+= -fplugin-arg-structleak_plugin-byref-all
 gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK)		\
diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
index d45f7f36b859..d0cc92e48f6f 100644
--- a/scripts/gcc-plugins/Kconfig
+++ b/scripts/gcc-plugins/Kconfig
@@ -67,27 +67,63 @@  config GCC_PLUGIN_LATENT_ENTROPY
 	   * https://pax.grsecurity.net/
 
 config GCC_PLUGIN_STRUCTLEAK
-	bool "Force initialization of variables containing userspace addresses"
+	bool "Zero initialize stack variables"
 	# Currently STRUCTLEAK inserts initialization out of live scope of
 	# variables from KASAN point of view. This leads to KASAN false
 	# positive reports. Prohibit this combination for now.
 	depends on !KASAN_EXTRA
 	help
-	  This plugin zero-initializes any structures containing a
-	  __user attribute. This can prevent some classes of information
-	  exposures.
-
-	  This plugin was ported from grsecurity/PaX. More information at:
+	  While the kernel is built with warnings enabled for any missed
+	  stack variable initializations, this warning is silenced for
+	  anything passed by reference to another function, under the
+	  occasionally misguided assumption that the function will do
+	  the initialization. As this regularly leads to exploitable
+	  flaws, this plugin is available to identify and zero-initialize
+	  such variables, depending on the chosen level of coverage.
+
+	  This plugin was originally ported from grsecurity/PaX. More
+	  information at:
 	   * https://grsecurity.net/
 	   * https://pax.grsecurity.net/
 
-config GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
-	bool "Force initialize all struct type variables passed by reference"
+choice
+	prompt "Coverage"
 	depends on GCC_PLUGIN_STRUCTLEAK
-	depends on !COMPILE_TEST
+	default GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
 	help
-	  Zero initialize any struct type local variable that may be passed by
-	  reference without having been initialized.
+	  This chooses the level of coverage over classes of potentially
+	  uninitialized variables. The selected class will be
+	  zero-initialized before use.
+
+	config GCC_PLUGIN_STRUCTLEAK_USER
+		bool "structs marked for userspace"
+		help
+		  Zero-initialize any structures on the stack containing
+		  a __user attribute. This can prevent some classes of
+		  uninitialized stack variable exploits and information
+		  exposures, like CVE-2013-2141:
+		  https://git.kernel.org/linus/b9e146d8eb3b9eca
+
+	config GCC_PLUGIN_STRUCTLEAK_BYREF
+		bool "structs passed by reference"
+		help
+		  Zero-initialize any structures on the stack that may
+		  be passed by reference and had not already been
+		  explicitly initialized. This can prevent most classes
+		  of uninitialized stack variable exploits and information
+		  exposures, like CVE-2017-1000410:
+		  https://git.kernel.org/linus/06e7e776ca4d3654
+
+	config GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
+		bool "anything passed by reference"
+		help
+		  Zero-initialize any stack variables that may be passed
+		  by reference and had not already been explicitly
+		  initialized. This is intended to eliminate all classes
+		  of uninitialized stack variable exploits and information
+		  exposures.
+
+endchoice
 
 config GCC_PLUGIN_STRUCTLEAK_VERBOSE
 	bool "Report forcefully initialized variables"
diff --git a/scripts/gcc-plugins/structleak_plugin.c b/scripts/gcc-plugins/structleak_plugin.c
index 10292f791e99..e89be8f5c859 100644
--- a/scripts/gcc-plugins/structleak_plugin.c
+++ b/scripts/gcc-plugins/structleak_plugin.c
@@ -16,6 +16,7 @@ 
  * Options:
  * -fplugin-arg-structleak_plugin-disable
  * -fplugin-arg-structleak_plugin-verbose
+ * -fplugin-arg-structleak_plugin-byref
  * -fplugin-arg-structleak_plugin-byref-all
  *
  * Usage:
@@ -26,7 +27,6 @@ 
  * $ gcc -fplugin=./structleak_plugin.so test.c -O2
  *
  * TODO: eliminate redundant initializers
- *       increase type coverage
  */
 
 #include "gcc-common.h"
@@ -37,13 +37,18 @@ 
 __visible int plugin_is_GPL_compatible;
 
 static struct plugin_info structleak_plugin_info = {
-	.version	= "201607271510vanilla",
+	.version	= "20190125vanilla",
 	.help		= "disable\tdo not activate plugin\n"
-			   "verbose\tprint all initialized variables\n",
+			  "byref\tinit structs passed by reference\n"
+			  "byref-all\tinit anything passed by reference\n"
+			  "verbose\tprint all initialized variables\n",
 };
 
+#define BYREF_STRUCT	1
+#define BYREF_ALL	2
+
 static bool verbose;
-static bool byref_all;
+static int byref;
 
 static tree handle_user_attribute(tree *node, tree name, tree args, int flags, bool *no_add_attrs)
 {
@@ -118,6 +123,7 @@  static void initialize(tree var)
 	gimple_stmt_iterator gsi;
 	tree initializer;
 	gimple init_stmt;
+	tree type;
 
 	/* this is the original entry bb before the forced split */
 	bb = single_succ(ENTRY_BLOCK_PTR_FOR_FN(cfun));
@@ -148,11 +154,15 @@  static void initialize(tree var)
 	if (verbose)
 		inform(DECL_SOURCE_LOCATION(var),
 			"%s variable will be forcibly initialized",
-			(byref_all && TREE_ADDRESSABLE(var)) ? "byref"
-							     : "userspace");
+			(byref && TREE_ADDRESSABLE(var)) ? "byref"
+							 : "userspace");
 
 	/* build the initializer expression */
-	initializer = build_constructor(TREE_TYPE(var), NULL);
+	type = TREE_TYPE(var);
+	if (AGGREGATE_TYPE_P(type))
+		initializer = build_constructor(type, NULL);
+	else
+		initializer = fold_convert(type, integer_zero_node);
 
 	/* build the initializer stmt */
 	init_stmt = gimple_build_assign(var, initializer);
@@ -184,13 +194,13 @@  static unsigned int structleak_execute(void)
 		if (!auto_var_in_fn_p(var, current_function_decl))
 			continue;
 
-		/* only care about structure types */
-		if (TREE_CODE(type) != RECORD_TYPE && TREE_CODE(type) != UNION_TYPE)
+		/* only care about structure types unless byref-all */
+		if (byref != BYREF_ALL && TREE_CODE(type) != RECORD_TYPE && TREE_CODE(type) != UNION_TYPE)
 			continue;
 
 		/* if the type is of interest, examine the variable */
 		if (TYPE_USERSPACE(type) ||
-		    (byref_all && TREE_ADDRESSABLE(var)))
+		    (byref && TREE_ADDRESSABLE(var)))
 			initialize(var);
 	}
 
@@ -232,8 +242,12 @@  __visible int plugin_init(struct plugin_name_args *plugin_info, struct plugin_gc
 			verbose = true;
 			continue;
 		}
+		if (!strcmp(argv[i].key, "byref")) {
+			byref = BYREF_STRUCT;
+			continue;
+		}
 		if (!strcmp(argv[i].key, "byref-all")) {
-			byref_all = true;
+			byref = BYREF_ALL;
 			continue;
 		}
 		error(G_("unknown option '-fplugin-arg-%s-%s'"), plugin_name, argv[i].key);