On Tue, Jun 08, 2021 at 09:41:38AM +0200, Richard Biener wrote:
> On Mon, 7 Jun 2021, Qing Zhao wrote:
>
> > Hi,
> >
> > > On Jun 7, 2021, at 2:53 AM, Richard Biener <[email protected]> wrote:
> > >
> > >>
> > >> To address the above suggestion:
> > >>
> > >> My study shows: the call to __builtin_clear_padding is expanded during
> > >> gimplification phase.
> > >> And there is no __bultin_clear_padding expanding during rtx expanding
> > >> phase.
> > >> However, for -ftrivial-auto-var-init, padding initialization should be
> > >> done both in gimplification phase and rtx expanding phase.
> > >> since the __builtin_clear_padding might not be good for rtx expanding,
> > >> reusing __builtin_clear_padding might not work.
> > >>
> > >> Let me know if you have any more comments on this.
> > >
> > > Yes, I didn't suggest to literally emit calls to __builtin_clear_padding
> > > but instead to leverage the lowering code, more specifically share the
> > > code that figures _what_ is to be initialized (where the padding is)
> > > and eventually the actual code generation pieces. That might need some
> > > refactoring but the code where padding resides should be present only
> > > a single time (since it's quite complex).
> >
> > Okay, I see your point here.
> >
> > >
> > > Which is also why I suggested to split out the padding initialization
> > > bits to a separate patch (and option).
> >
> > Personally, I am okay with splitting padding initialization from this
> > current patch,
> > Kees, what’s your opinion on this? i.e, the current -ftrivial-auto-var-init
> > will NOT initialize padding, we will add another option to
> > Explicitly initialize padding.
>
> It would also be possible to have -fauto-var-init, -fauto-var-init-padding
> and have -ftrivial-auto-var-init for clang compatibility enabling both.
Sounds good to me!
> Or -fauto-var-init={zero,pattern,padding} and allow
> -fauto-var-init=pattern,padding to be specified. Note there's also
> padding between auto variables on the stack - that "trailing"
> padding isn't initialized either? (yes, GCC sorts variables to minimize
> that padding) For example for
>
> void foo()
> {
> char a[3];
> bar (a);
> }
>
> there's 12 bytes padding after 'a', shouldn't we initialize that? If not,
> why's other padding important to be initialized?
This isn't a situation that I'm aware of causing real-world problems.
The issues have all come from padding within an addressable object. I
haven't tested Clang's behavior on this (and I have no kernel tests for
this padding), but I do check for trailing padding, like:
struct test_trailing_hole {
char *one;
char *two;
char *three;
char four;
/* "sizeof(unsigned long) - 1" byte padding hole here. */
};
-Kees
--
Kees Cook
