On Tue, Feb 02, 2021 at 05:32:32PM -0600, Madhavan T. Venkataraman wrote:

On 2/2/21 4:05 AM, Mark Rutland wrote:

quoted

I think that practically speaking it's necessary to track all potential
paths through functions that may alter the shadow stack or the shadow
stack pointer to ensure that the manipulation is well-balanced and that
the shadow stack pointer isn't corrupted.

Practically speaking, this requires decoding a significant number of
instructions, and tracing through all potential paths a function may
take.

I thought about it some more since you don't like the shadow stack
that much.

Just to be clear, what I was trying to get across was:

* Whatever we do, we want to verify at compile time that the kernel
  binary matches our expecations, and practically speaking this almost
  certainly means using objtool.

* The analysis that objtool will have to do is not made significantly
  simpler through the use of a shadow stack, as it still needs to track
  all paths though a function, etc.

I'm not averse to using a shadow stack (and clang's SCS is a useful
security feature), I just don't think that it helps much with
compile-time verification, and I don't see a strong reason to mandate it
for livepatching.

[...]

The goal is - even if a function modifies fp and/or does not restore sp to its
correct value at the end, the prolog and epilog should manage it so that everything
works. To do this, the current frame pointer address is stored in fp as well as cur_fp.
Even if fp is modified, cur_fp will still point to the correct frame address.

Prolog
=======

The original prolog is:

- Push fp and return address on the stack
- fp = sp


The new prolog is:

- Save cur_fp on the stack
- Push fp, return address on the stack
- fp = sp
- cur_fp = fp

Epilog
======

The original epilog is:

- Pop fp and return address

The new epilog is:

- sp = cur_fp
- Pop fp and return address
- Restore cur_fp from the stack


I think this is pretty simple.

I'm afraid I don't understand the problem you're trying to solve here.
The epilog you propose is also unsound in the face of asynchronous
exceptions, so I suspect you haven't thought as hard about this as you
need to.

Even if the compiler uses a different prologue/epilogue sequence, we
still need to verify that the rest of the function does nothing to
undermine that.

I think this is merely different rather than simpler, and regardless
this would be an invasive change to compilers.

Unwinder
========

The unwinder will start the stack walk from cur_fp instead of fp. At each frame,
it will use the saved cur_fp instead of the saved fp. 

Also, at each step, it can know if fp was actually changed by the function in
the frame. The unwinder can optionally issue warnings.

So this is just about aditional robustness?

I'm happy to use a shadow stack for /additional/ robustness, I just
don't think a shadow stack alone solves all the other issues that we
need compile time verification for, nor does it solve cases where we
might want metadata generated at compile time.

Compiler issue
===============

This solution is geared towards the kernel only. It assumes that the stack
has a fixed size and alignment so the bottom of the stack can be reached
from the current sp.

So, the compiler has to support two prologs and epilogs, one pair for apps
and one pair for the kernel.

Since this is just a tiny bit of code, I don't think this is a problem.

I suspect it's more compilated than that. Configuration differences like
this can easily double the necessary testing, and are liable to becomer
a maintenance burden, so I don't expect compiler folk are likely to want
to support this unless necessary.

At the moment, I don't think that this solves a real problem, and I
don't think that we need to change this.

In future, we might want to agree some constraints with toolchain folk
if we have specific concerns or pain points, but I don't think we've
actually enumerated those and why we can't solve them through over
means.

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel