Fiber: add shadow stack support #9283
Conversation
|
With this, all fiber test passed on CET machine. Otherwise, we see below failures. This doesn't incur new test failure on my local test. Wait for CI results. |
|
Thank you for the PR! I'm uncomfortable to patch the bundled asm files from boost. Do you see a way to avoid that? |
|
I would prefer to see the assembly files changed upstream in https://github.com/boostorg/context before modifying the assembly files here if possible. Can we put the shadow stack into the same allocation as the normal stack? The normal stack pointer can be shifted lower in the make_context assembly with the shadow stack starting at the stack pointer given to make_context. This would avoid another allocation, special handling in zend_fiber_stack_allocate/free, and writing the pointer onto the stack memory. The ucontext related functions have been updated to support shadow stacks, perhaps some inspiration can be taken from there? https://code.woboq.org/userspace/glibc/sysdeps/unix/sysv/linux/x86_64/ |
|
Hi @trowski,
Do you mean that we allcoate normal stack adjacent to shadow stack (SS)? For example, Current patch has much interaction between .C and .asm. One alternative is to handle all SS stuff in .asm, that is, we allocate SS in make_fcontext and record the base/size/pointer in fcontext. But I don't find a good place to
The author give me several advises weeks ago. ucontext record SS info in C struct. |
github-actions
bot
added
Status: Needs Triage
and removed
Status: Needs Feedback
labels
|
I was hoping to avoid two allocations for each fiber – but certainly is not a must, just would be nice. I'm looking forward to feedback from the author of map_shadow_stack. If the shadow stack must be allocated with a separate syscall then I wouldn't attempt to request a specific range from mmap, that sounds too fragile. I don't have a better suggestion at the moment for passing the shadow stack pointer to make_fcontext. |
Let me upstream this to boost at first. I will propose add one more arg to |
:) |
Shadow stack is part of Intel's Control-Flow Enforcement Technology.
Whenever a function is called, the return address is pushed onto both
the regular stack and the shadow stack. When that function returns, the
return addresses are popped off both stacks and compared; if they fail
to match, #CP raised.
Backport this commit from php/php-src#9283
With this commit, we create shadow stack with syscall map_shadow_stack
(no.451) for each fiber context and switch the shadow stack accordingly
during fcontext switch.
Signed-off-by: PeterYang12 <yuhan.yang@intel.com>
Signed-off-by: chen-hu-97 <hu1.chen@intel.com>
The boost counterpart has been merged in boostorg/context#207. |
|
Thank you! I wonder whether we should rename |
Will rename |
chen-hu-97
force-pushed
the
jit
branch
2 times, most recently
from
ac964fc to
f152c18
Compare
Shadow stack is part of Intel's Control-Flow Enforcement Technology (CET). Whenever a function is called, the return address is pushed onto both the regular stack and the shadow stack. When that function returns, the return addresses are popped off both stacks and compared; if they fail to match, #CP raised. With this commit, we create shadow stack for each fiber context and switch the shadow stack accordingly during fcontext switch. Signed-off-by: Chen, Hu <hu1.chen@intel.com>
|
Update this PR to rename Thanks for @cmb69's comments. Sorry for the latency. |
|
Thank you! From a cursory glance, this looks good to me, but maybe @trowski can have a closer look. |
|
This LGTM now. @ramsey Can this be merged into the 8.2 branch? |
|
8.2 is at the fifth RC right now. Can you help me understand the reason this should be merged to 8.2 and any risks associated with it? |
|
My understanding is that a machine with CET enabled will not be able to use fibers without this change. As these changes only affect machines with shadow stack support, there's not much risk to these changes. I have no idea how common CET is at this time though, so waiting until 8.3 may not have a large impact. |
|
Well, we treated #8339 as bug fix (went into PHP 8.1.8RC1), so I think it should be okay to target PHP-8.2 here. @chen-hu-97, what do you think about the target branch? |
|
I'm convinced; this should target 8.3. |
|
So shadow stacks are an enhancement to CET support, not a requirement. Great, then targeting 8.3 makes sense to me too. |
|
Thank you! |
Shadow stack is part of Intel's Control-Flow Enforcement Technology (CET).
Whenever a function is called, the return address is pushed onto both
the regular stack and the shadow stack. When that function returns, the
return addresses are popped off both stacks and compared; if they fail
to match, #CP raised.
With this commit, we create shadow stack for each fiber context and
switch the shadow stack accordingly during fcontext switch.
Signed-off-by: Chen, Hu hu1.chen@intel.com