On 5/3/2021 8:29 AM, Andy Lutomirski wrote:
On May 3, 2021, at 8:14 AM, Yu, Yu-cheng <yu-cheng.yu@xxxxxxxxx> wrote:
On 5/2/2021 4:23 PM, Andy Lutomirski wrote:
On Fri, Apr 30, 2021 at 10:47 AM Andy Lutomirski <luto@xxxxxxxxxx> wrote:
On Fri, Apr 30, 2021 at 10:00 AM Yu, Yu-cheng <yu-cheng.yu@xxxxxxxxx> wrote:
On 4/28/2021 4:03 PM, Andy Lutomirski wrote:
On Tue, Apr 27, 2021 at 1:44 PM Yu-cheng Yu <yu-cheng.yu@xxxxxxxxx> wrote:
When shadow stack is enabled, a task's shadow stack states must be saved
along with the signal context and later restored in sigreturn. However,
currently there is no systematic facility for extending a signal context.
There is some space left in the ucontext, but changing ucontext is likely
to create compatibility issues and there is not enough space for further
extensions.
Introduce a signal context extension struct 'sc_ext', which is used to save
shadow stack restore token address. The extension is located above the fpu
states, plus alignment. The struct can be extended (such as the ibt's
wait_endbr status to be introduced later), and sc_ext.total_size field
keeps track of total size.
I still don't like this.
Here's how the signal layout works, for better or for worse:
[...]
That's where we are right now upstream. The kernel has a parser for
the FPU state that is bugs piled upon bugs and is going to have to be
rewritten sometime soon. On top of all this, we have two upcoming
features, both of which require different kinds of extensions:
1. AVX-512. (Yeah, you thought this story was over a few years ago,
but no. And AMX makes it worse.) To make a long story short, we
promised user code many years ago that a signal frame fit in 2048
bytes with some room to spare. With AVX-512 this is false. With AMX
it's so wrong it's not even funny. The only way out of the mess
anyone has come up with involves making the length of the FPU state
vary depending on which features are INIT, i.e. making it more compact
than "compact" mode is. This has a side effect: it's no longer
possible to modify the state in place, because enabling a feature with
no space allocated will make the structure bigger, and the stack won't
have room. Fortunately, one can relocate the entire FPU state, update
the pointer in mcontext, and the kernel will happily follow the
pointer. So new code on a new kernel using a super-compact state
could expand the state by allocating new memory (on the heap? very
awkwardly on the stack?) and changing the pointer. For all we know,
some code already fiddles with the pointer. This is great, except
that your patch sticks more data at the end of the FPU block that no
one is expecting, and your sigreturn code follows that pointer, and
will read off into lala land.
Then, what about we don't do that at all. Is it possible from now on we
don't stick more data at the end, and take the relocating-fpu approach?
2. CET. CET wants us to find a few more bytes somewhere, and those
bytes logically belong in ucontext, and here we are.
Fortunately, we can spare CET the need of ucontext extension. When the
kernel handles sigreturn, the user-mode shadow stack pointer is right at
the restore token. There is no need to put that in ucontext.
That seems entirely reasonable. This might also avoid needing to
teach CRIU about CET at all.
Wait, what's the actual shadow stack token format? And is the token
on the new stack or the old stack when sigaltstack is in use? For
that matter, is there any support for an alternate shadow stack for
signals?
The restore token is a pointer pointing directly above itself and bit[0] indicates 64-bit mode.
Because the shadow stack stores only return addresses, there is no alternate shadow stack. However, the application can allocate and switch to a new shadow stack.
I think we should make the ABI support an alternate shadow stack even if we don’t implement it initially. After all, some day someone might want to register a handler for shadow stack overflow.
Agree. We can probably add something in parallel of sigaltstack(), and
let the user choose separately alternate normal/shadow stacks.
Thanks,
Yu-cheng
