This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH][RFA/RFC] Stack clash mitigation patch 01/08


On Thu, Jul 13, 2017 at 04:38:00PM -0600, Jeff Law wrote:
> On 07/13/2017 03:32 PM, Segher Boessenkool wrote:
> >>> -fstack-check=clash is itself not such a super name either.  It's not
> >>> checking stack, and it isn't clashing: it just does a store to every
> >>> page the stack will touch (minus the first and last or something).
> >> Yea.  I don't particularly like it either.  Suggestions?  I considered
> >> "probe" as well, but "specific" also does probing.  In the end I used
> >> the part of the marketing name of the exploits.
> > 
> > I don't think it should be inside -fstack-check at all.  Sure, the
> > mechanisms implementing it overlap a bit (more on some targets, less
> > on others), but how will a user ask for clash protection _and_ for
> > stack checking?
> The biggest problem with separating them is we would end up with a fair
> amount code that ultimately looks like
> 
> if (flag_stack_check == STATIC_BUILTIN_STACK_CHECK
>     || flag_whatever_the_new_thing_is)
>   {
>     probe the stack
>   }

But only in backends, right?  And some backends will be actually
simpler, afaics.

> We can certainly do that.  It'll touch a few more places (particularly
> in backends that have checks for Ada stack overflow, but for which I
> haven't written stack clash protection), but the changes would be simple
> and repetitive.

Sounds good :-)

> >> Certainly open to ideas on the interface aspects.
> > 
> > The interface is much harder to change than any aspect of the GCC
> > implementation.  Have to get it right at once, almost.
> Yup.  It gets backed in even faster than normal in this case I suspect.
> Red Hat will almost certainly pick up the bits and start backporting
> them to RHEL 7, RHEL 6 and RHEL 5.  So the flag will be out in the wild
> before gcc-8 hits the streets.

Are you planning to backport it for mainline GCC as well?

> >> It's independent of stack size overflow checking.  They could (in
> >> theory) even co-exist on ports that support stack size overflow
> >> checking, but I didn't test that.
> > 
> > Okay, that was my impression as well.  But that interface won't allow
> > it (unless every -fstack-check=X gets an -fstack-check=X+clash twin).
> THere may have been some mis-communication here, but it's a good place
> to spend some time thinking about interplay between options.
> 
> First, there's -fstack-limit-*.  Essentially there's an RTX which
> defines the limit for the stack pointer.  If you cross that limit a trap
> gets executed.  PPC, s390 and likely other ports have support for this.
> It should interoperate with -fstack-clash-protection and/or -fstack-check.

Yup.

> There's -fstack-check and -fstack-clash-protection.  I think with the
> direction we're going they are fundamentally incompatible because
> neither the compiler nor kernel do anything to guarantee enough stack is
> available after hitting the guard for -fstack-clash-protection.

Hrm, I have to think about that.

> And there's shrink wrapping.  This was originally raised by the aarch64
> guys and deserves some thought.  I'm particularly worried about aarch64
> if it was to shrink-wrap some particular register saves that we wanted
> to use as an implicit probe.

For normal shrink-wrapping, the volatile reg stores will always happen
in the same prologue that sets up the stack frame as well, so there
won't be any problem (except the usual stack ties you might need in the
prologue, etc.) (*)  For separate shrink-wrapping, yeah you'll need any
bb that could potentially store to the stack require the component for
the register you use as implicit probe.  This is pretty nasty.

> >> We created that alloca'd object at the wrong lexical scope which mucked
> >> up its expected persistence.  I'm sure I'd spot it trivially once I set
> >> up the test again.
> > 
> > That sounds like it might be a bug even.
> That was my thought as well.  It's c-torture/execute/20071029-1.c.

Ah cool, thanks for digging it up.


Segher


(*) Related, I know you don't want to scan generated code to see if
all probes are in place, but it seems you pretty much have to if you
want to make sure no later pass deletes the probes.  Well, something
for targets to worry about :-)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]