(CC’ing gcc-patch alias).
Hi, Kees,
> On Mar 12, 2021, at 3:55 PM, Kees Cook <keesc...@chromium.org> wrote:
>
> 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
Thanks a lot for the above information, they are very useful.
I will take a look at the LLVM patch and try to implement this feature into GCC
as well.
>
>> 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. :)
Okay, will do that.
Thanks again.
Qing
>
> -Kees
>
> --
> Kees Cook
