Skip to content

Commit 81a56f6

Browse files
committed
gcc-plugins: structleak: Generalize to all variable types
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> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
1 parent 49a5785 commit 81a56f6

File tree

3 files changed

+74
-22
lines changed

3 files changed

+74
-22
lines changed

scripts/Makefile.gcc-plugins

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ gcc-plugin-$(CONFIG_GCC_PLUGIN_SANCOV) += sancov_plugin.so
1515
gcc-plugin-$(CONFIG_GCC_PLUGIN_STRUCTLEAK) += structleak_plugin.so
1616
gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK_VERBOSE) \
1717
+= -fplugin-arg-structleak_plugin-verbose
18+
gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF) \
19+
+= -fplugin-arg-structleak_plugin-byref
1820
gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL) \
1921
+= -fplugin-arg-structleak_plugin-byref-all
2022
gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK) \

scripts/gcc-plugins/Kconfig

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -67,27 +67,63 @@ config GCC_PLUGIN_LATENT_ENTROPY
6767
* https://pax.grsecurity.net/
6868

6969
config GCC_PLUGIN_STRUCTLEAK
70-
bool "Force initialization of variables containing userspace addresses"
70+
bool "Zero initialize stack variables"
7171
# Currently STRUCTLEAK inserts initialization out of live scope of
7272
# variables from KASAN point of view. This leads to KASAN false
7373
# positive reports. Prohibit this combination for now.
7474
depends on !KASAN_EXTRA
7575
help
76-
This plugin zero-initializes any structures containing a
77-
__user attribute. This can prevent some classes of information
78-
exposures.
79-
80-
This plugin was ported from grsecurity/PaX. More information at:
76+
While the kernel is built with warnings enabled for any missed
77+
stack variable initializations, this warning is silenced for
78+
anything passed by reference to another function, under the
79+
occasionally misguided assumption that the function will do
80+
the initialization. As this regularly leads to exploitable
81+
flaws, this plugin is available to identify and zero-initialize
82+
such variables, depending on the chosen level of coverage.
83+
84+
This plugin was originally ported from grsecurity/PaX. More
85+
information at:
8186
* https://grsecurity.net/
8287
* https://pax.grsecurity.net/
8388

84-
config GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
85-
bool "Force initialize all struct type variables passed by reference"
89+
choice
90+
prompt "Coverage"
8691
depends on GCC_PLUGIN_STRUCTLEAK
87-
depends on !COMPILE_TEST
92+
default GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
8893
help
89-
Zero initialize any struct type local variable that may be passed by
90-
reference without having been initialized.
94+
This chooses the level of coverage over classes of potentially
95+
uninitialized variables. The selected class will be
96+
zero-initialized before use.
97+
98+
config GCC_PLUGIN_STRUCTLEAK_USER
99+
bool "structs marked for userspace"
100+
help
101+
Zero-initialize any structures on the stack containing
102+
a __user attribute. This can prevent some classes of
103+
uninitialized stack variable exploits and information
104+
exposures, like CVE-2013-2141:
105+
https://git.kernel.org/linus/b9e146d8eb3b9eca
106+
107+
config GCC_PLUGIN_STRUCTLEAK_BYREF
108+
bool "structs passed by reference"
109+
help
110+
Zero-initialize any structures on the stack that may
111+
be passed by reference and had not already been
112+
explicitly initialized. This can prevent most classes
113+
of uninitialized stack variable exploits and information
114+
exposures, like CVE-2017-1000410:
115+
https://git.kernel.org/linus/06e7e776ca4d3654
116+
117+
config GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
118+
bool "anything passed by reference"
119+
help
120+
Zero-initialize any stack variables that may be passed
121+
by reference and had not already been explicitly
122+
initialized. This is intended to eliminate all classes
123+
of uninitialized stack variable exploits and information
124+
exposures.
125+
126+
endchoice
91127

92128
config GCC_PLUGIN_STRUCTLEAK_VERBOSE
93129
bool "Report forcefully initialized variables"

scripts/gcc-plugins/structleak_plugin.c

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
* Options:
1717
* -fplugin-arg-structleak_plugin-disable
1818
* -fplugin-arg-structleak_plugin-verbose
19+
* -fplugin-arg-structleak_plugin-byref
1920
* -fplugin-arg-structleak_plugin-byref-all
2021
*
2122
* Usage:
@@ -26,7 +27,6 @@
2627
* $ gcc -fplugin=./structleak_plugin.so test.c -O2
2728
*
2829
* TODO: eliminate redundant initializers
29-
* increase type coverage
3030
*/
3131

3232
#include "gcc-common.h"
@@ -37,13 +37,18 @@
3737
__visible int plugin_is_GPL_compatible;
3838

3939
static struct plugin_info structleak_plugin_info = {
40-
.version = "201607271510vanilla",
40+
.version = "20190125vanilla",
4141
.help = "disable\tdo not activate plugin\n"
42-
"verbose\tprint all initialized variables\n",
42+
"byref\tinit structs passed by reference\n"
43+
"byref-all\tinit anything passed by reference\n"
44+
"verbose\tprint all initialized variables\n",
4345
};
4446

47+
#define BYREF_STRUCT 1
48+
#define BYREF_ALL 2
49+
4550
static bool verbose;
46-
static bool byref_all;
51+
static int byref;
4752

4853
static tree handle_user_attribute(tree *node, tree name, tree args, int flags, bool *no_add_attrs)
4954
{
@@ -118,6 +123,7 @@ static void initialize(tree var)
118123
gimple_stmt_iterator gsi;
119124
tree initializer;
120125
gimple init_stmt;
126+
tree type;
121127

122128
/* this is the original entry bb before the forced split */
123129
bb = single_succ(ENTRY_BLOCK_PTR_FOR_FN(cfun));
@@ -148,11 +154,15 @@ static void initialize(tree var)
148154
if (verbose)
149155
inform(DECL_SOURCE_LOCATION(var),
150156
"%s variable will be forcibly initialized",
151-
(byref_all && TREE_ADDRESSABLE(var)) ? "byref"
152-
: "userspace");
157+
(byref && TREE_ADDRESSABLE(var)) ? "byref"
158+
: "userspace");
153159

154160
/* build the initializer expression */
155-
initializer = build_constructor(TREE_TYPE(var), NULL);
161+
type = TREE_TYPE(var);
162+
if (AGGREGATE_TYPE_P(type))
163+
initializer = build_constructor(type, NULL);
164+
else
165+
initializer = fold_convert(type, integer_zero_node);
156166

157167
/* build the initializer stmt */
158168
init_stmt = gimple_build_assign(var, initializer);
@@ -184,13 +194,13 @@ static unsigned int structleak_execute(void)
184194
if (!auto_var_in_fn_p(var, current_function_decl))
185195
continue;
186196

187-
/* only care about structure types */
188-
if (TREE_CODE(type) != RECORD_TYPE && TREE_CODE(type) != UNION_TYPE)
197+
/* only care about structure types unless byref-all */
198+
if (byref != BYREF_ALL && TREE_CODE(type) != RECORD_TYPE && TREE_CODE(type) != UNION_TYPE)
189199
continue;
190200

191201
/* if the type is of interest, examine the variable */
192202
if (TYPE_USERSPACE(type) ||
193-
(byref_all && TREE_ADDRESSABLE(var)))
203+
(byref && TREE_ADDRESSABLE(var)))
194204
initialize(var);
195205
}
196206

@@ -232,8 +242,12 @@ __visible int plugin_init(struct plugin_name_args *plugin_info, struct plugin_gc
232242
verbose = true;
233243
continue;
234244
}
245+
if (!strcmp(argv[i].key, "byref")) {
246+
byref = BYREF_STRUCT;
247+
continue;
248+
}
235249
if (!strcmp(argv[i].key, "byref-all")) {
236-
byref_all = true;
250+
byref = BYREF_ALL;
237251
continue;
238252
}
239253
error(G_("unknown option '-fplugin-arg-%s-%s'"), plugin_name, argv[i].key);

0 commit comments

Comments
 (0)