Hi, Kees,
I am looking at the structure padding initialization issue. And also have some
questions:
> On Feb 24, 2021, at 10:41 PM, Kees Cook <keesc...@chromium.org> wrote:
>
> It looks like there is still some issues with padding and pre-case
> switch variables. Here's the test output, FWIW:
>
>
> test_stackinit: small_hole_static_all FAIL (uninit bytes: 3)
> test_stackinit: big_hole_static_all FAIL (uninit bytes: 61)
> test_stackinit: trailing_hole_static_all FAIL (uninit bytes: 7)
> test_stackinit: small_hole_dynamic_all FAIL (uninit bytes: 3)
> test_stackinit: big_hole_dynamic_all FAIL (uninit bytes: 61)
> test_stackinit: trailing_hole_dynamic_all FAIL (uninit bytes: 7)
>
> test_stackinit: switch_1_none FAIL (uninit bytes: 8)
> test_stackinit: switch_2_none FAIL (uninit bytes: 8)
> test_stackinit: failures: 8
>
>
> /* Simple structure with padding likely to be covered by compiler. */
> struct test_small_hole {
> size_t one;
> char two;
> /* 3 byte padding hole here. */
> int three;
> unsigned long four;
> };
>
> /* Try to trigger unhandled padding in a structure. */
> struct test_aligned {
> u32 internal1;
> u64 internal2;
> } __aligned(64);
>
> struct test_big_hole {
> u8 one;
> u8 two;
> u8 three;
> /* 61 byte padding hole here. */
> struct test_aligned four;
> } __aligned(64);
>
> struct test_trailing_hole {
> char *one;
> char *two;
> char *three;
> char four;
> /* "sizeof(unsigned long) - 1" byte padding hole here. */
> };
>
> They fail when they're statically initialized (either fully or
> partially),
So, when the structure is not statically initialized, the compiler
initialization is good?
For the failing cases, what’s the behavior of the LLVM -ftrivial-auto-var-init?
From the LLVM patch: (https://reviews.llvm.org/D54604
<https://reviews.llvm.org/D54604>)
====
To keep the patch simple, only some undef is removed for now, see
replaceUndef. The padding-related infoleaks are therefore not all gone yet.
This will be addressed in a follow-up, mainly because addressing padding-related
leaks should be a stand-alone option which is implied by variable
initialization.
====
Yes, in GCC’s implementation, I think that fixing all padding-related leaks
also require a
separate patch.
Qing
> for example:
>
> struct test_..._hole instance = { .two = ..., };
>
> or
>
> struct test_..._hole instance = { .one = ...,
> .two = ...,
> .three = ...,
> .four = ...,
> };
>
> The last case is for switch variables outside of case statements, like
> "var" here:
>
> switch (path) {
> unsigned long var;
>
> case ..:
> ...
> case ..:
> ...
> ...
> }
>
>
> I'm really looking forward to having this available. Thanks again!
>
> -Kees
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/lib/test_stackinit.c
>
> --
> Kees Cook
