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

[Jeff Law <law@redhat.com>]

On 07/11/2017 06:20 PM, Wilco Dijkstra wrote:
> Jeff Law wrote:
>> aarch64 is the first target that does not have any implicit probes in
>> the caller.  Thus at prologue entry it must make conservative
>> assumptions about the offset of the most recent probed address relative
>> to the stack pointer.
> 
> No - like I mentioned before that's not correct nor acceptable as it would imply
> that ~70% of functions need a probe at entry. I did a quick run across SPEC and
> found the outgoing argument size is > 1024 in just 9 functions out of 147000!
> Those 9 were odd special cases due to auto generated code to interface between
> C and Fortran. This is extremely unlikely to occur anywhere else. So even assuming
> an unchecked caller, large outgoing arguments are simply not a realistic threat.
Whether or not such frames exist in SPEC isn't the question.   THere's
nothing in the ABI or ISA that allows us to avoid those probes without
compromising security.

Mixed code scenarios are going to be a fact of life, probably forever
unless we can convince every ISV providing software that works on top of
RHEL/*BSD/whatever  to turn on probing (based on my experiences, that
has exactly a zero chance of occurring).

In a mixed code scenario a caller may have a large alloca or large
outgoing argument area that pushes the stack pointer into the guard page
without actually accessing the guard page.  That requires a callee which
is compiled with stack checking to make worst case assumptions at
function entry to protect itself as much as possible from these attacks.

THe aarch64 maintainers can certain nix what I've done or modify it in
ways that suit them.  That is their choice as port maintainers.
However, Red Hat will have to evaluate the cost of reducing security for
our customer base against the performance improvement of such changes.
As I've said before, I do not know where that decision would fall.


> 
> Therefore even when using a tiny 4K probe size we can safely adjust SP by 3KB
> before needing an explicit probe - now only 0.6% of functions need a probe.
> If we choose a proper minimum probe distance, say 64KB, explicit probes are
> basically non-existent (just 35 functions, or ~0.02% of all functions are > 64KB).
> Clearly inserting probes can be the default as the impact on code quality is negligible.
Again, there's nothing that says 3k is safe.   You're picking an
arbitrary point that is safe in a codebase you've looked at.  But I'm
looking at this from a "what guarantees do I have from an ABI or ISA
standpoint".  The former may be more performant, but it's inherently
less secure than the latter.

> 
> With respect to implementation it is relatively easy to decide in aarch64_layout_frame
> which frames need probes and where. I don't think keeping a running offset of the last
> probe/store is useful, it'll just lead to inefficiencies and bugs. The patch doesn't deal
> with the delayed stores due to shrinkwrapping for example. Inserting probes before
> the prolog would be easier, eg.
> 
> sub tmp, sp, 65536
> str xzr, [tmp, 1024]  // allow up to 1KB of outgoing arguments in callee
> sub tmp, sp, 131072
> str xzr, [tmp, 1024]
> ... normal prolog for frame size 128-192KBIf you think you can do better without compromising security, be my
guest.  Realistically, I'm pretty close to the limit of how much more
time I can spend on the aarch64 target dependent issues.

So I think the question I need answered from the aarch64 maintainers is

Are they going to nak this code outright
Are they going to keep basic structure but suggest tweaks

I need to know the general direction so that I know whether or not to
continue to carry the aarch64 changes in the patchkit.  If y'all are
going to nak, then I'll drop them from upstream except for the changes
to indirect STACK_CHECK_PROTECT.


jeff