On Fri, Mar 12, 2021 at 03:35:28PM -0600, Qing Zhao wrote:
> 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.
> ====
Right, padding init happened in:
https://github.com/llvm/llvm-project/commit/4f7bc0eee7e6099b1abd57dac3c83529944ab23c
And was further clarified that, IIUC, padding _must be zero_ regardless
of pattern-vs-zero in:
https://github.com/llvm/llvm-project/commit/d39fbc7e20d84364e409ce59724ce20625637062
> Yes, in GCC’s implementation, I think that fixing all padding-related leaks
> also require a
> separate patch.
That's fine -- but it'll need to be tied to -ftrivial-auto-var-init,
since otherwise the memory isn't actually fully initialized. :)
-Kees
--
Kees Cook
