On Thu, Feb 25, 2021 at 12:15:01PM -0600, Qing Zhao wrote:
> > On Feb 24, 2021, at 10:41 PM, Kees Cook <keesc...@chromium.org> wrote:
> > [...]
> > test_stackinit: trailing_hole_none ok
> > test_stackinit: packed_none ok
> > test_stackinit: user ok
> > test_stackinit: failures: 8
>
> Does the above testing include “pattern initialization” in addition to “zero
> initialization”?
This was from the zero-init case. I've just tested pattern-init now and
it actually crashes GCC. I minimized the test case to this:
$ cat main.i
a() { char b[1]; }
$ gcc -ftrivial-auto-var-init=pattern -c /dev/null main.i
main.i:1:1: warning: return type defaults to ‘int’ [-Wimplicit-int]
1 | a() { char b[1]; }
| ^
main.i: In function ‘a’:
main.i:1:12: internal compiler error: in gimplify_init_ctor_eval, at
gimplify.c:4873
1 | a() { char b[1]; }
| ^
0x69740d gimplify_init_ctor_eval
../../../gcc/gcc/gimplify.c:4873
0xb5ac8f gimplify_init_constructor
../../../gcc/gcc/gimplify.c:5320
0xb6b68a gimplify_modify_expr
../../../gcc/gcc/gimplify.c:5952
0xb533ba gimplify_expr(tree_node**, gimple**, gimple**, bool (*)(tree_node*),
int)
../../../gcc/gcc/gimplify.c:14262
0xb56b26 gimplify_stmt(tree_node**, gimple**)
../../../gcc/gcc/gimplify.c:7056
0xb68e6e gimplify_and_add(tree_node*, gimple**)
../../../gcc/gcc/gimplify.c:489
0xb68e6e gimple_add_init_for_auto_var
../../../gcc/gcc/gimplify.c:1892
0xb68e6e gimplify_decl_expr
../../../gcc/gcc/gimplify.c:2010
0xb53bd6 gimplify_expr(tree_node**, gimple**, gimple**, bool (*)(tree_node*),
int)
../../../gcc/gcc/gimplify.c:14459
0xb56b26 gimplify_stmt(tree_node**, gimple**)
../../../gcc/gcc/gimplify.c:7056
0xb5727d gimplify_bind_expr
../../../gcc/gcc/gimplify.c:1421
0xb536f0 gimplify_expr(tree_node**, gimple**, gimple**, bool (*)(tree_node*),
int)
../../../gcc/gcc/gimplify.c:14463
0xb6ccc9 gimplify_stmt(tree_node**, gimple**)
../../../gcc/gcc/gimplify.c:7056
0xb6ccc9 gimplify_body(tree_node*, bool)
../../../gcc/gcc/gimplify.c:15498
0xb6d0ed gimplify_function_tree(tree_node*)
../../../gcc/gcc/gimplify.c:15652
0x9ae8d7 cgraph_node::analyze()
../../../gcc/gcc/cgraphunit.c:670
0x9b13a7 analyze_functions
../../../gcc/gcc/cgraphunit.c:1233
0x9b1f9d symbol_table::finalize_compilation_unit()
../../../gcc/gcc/cgraphunit.c:2511
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.
> > struct test_..._hole instance = { .two = ..., };
> >
> > or
> >
> > struct test_..._hole instance = { .one = ...,
> > .two = ...,
> > .three = ...,
> > .four = ...,
> >
> > };
>
> So, when the structure variables are not statically initialized, all the
> paddings are initialized correctly by the compiler?
For the zero case, yes. (Usually such happen via copies into stack from from
.rodata
sections, or accidentally from already-initialized copies.)
> In the current implementation, when the auto variable is explicitly
> initialized, compiler will do nothing.
> Looks like for structure variables we need some special handling to
> initialize the paddings.
> Need to study this a little bit and see how to fix it.
Deterministic padding init is a big part of this defense since that's
where the bulk of the really hard-to-find problems have existed (i.e. in
padding holes in structures that got copied around).
> > The last case is for switch variables outside of case statements, like
> > "var" here:
> >
> > switch (path) {
> > unsigned long var;
> >
> > case ..:
> > ...
> > case ..:
> > ...
> > ...
> > }
> >
>
> Will study this case more.
For the kernel, this is a low priority, since I purged the code of all
variable declarations outside of an execution path[1]. For reference,
here's the Clang bug for the same problem[2] (though latest Clang git
appears to no longer exhibit this issue, so they may have fixed it).
-Kees
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/?qt=grep&q=Distribute+switch+variables+for+initialization
[2] https://bugs.llvm.org/show_bug.cgi?id=44916
--
Kees Cook
