Changes continue_execution to not clobber stack that may be in use. #217
Conversation
|
Do you have a sample to reproduce this? The BTW, are you trying to use OpenSSL inside enclave? FYI, we have an SGXSSL for that use. |
|
We're using BoringSSL, Google's vetted SSL library. It weirdly uses stack beyond RSP. I'll confer with my team about what exactly is the problem and update you tomorrow. |
|
This change is to support programs that are compiled with stack red zones. I can see about making a repro test if y'all need. |
Signed-off-by: Dionna Glaze <[email protected]>
deeglaze
force-pushed
the
fix_continue_execution
branch
from
9319ac1 to
ac3caf6
Compare
|
I pushed an update to this PR that explains the red zone situation a bit more. I think at the very least internal_handle_exception should be annotated with |
|
The changes may cause problem for 32 bit mode as 32 bit doesn't have red zone. Could you help verify if this commit works for you? lzha101@f361038 |
|
That commit causes a crash for us. Externalizing a repro would probably be more work than just roundtripping with me at this time, unfortunately. |
|
It's weird. Of cause, could you provide a test repo? |
| # Offset the stack to avoid the red zone, which interrupts are allowed to | ||
| # write into. Using the __attribute__((interrupt)) on internal_handle_exception | ||
| # is hostile to older compilers. | ||
| mov %xcx, -128(%xax) # save xip to the new stack |
There was a problem hiding this comment.
The patch doesn't really avoid the red zone, right? From this line, it saves the xip at the bottom of the red zone but it is still inside the red zone.
|
Okay, I just double and triple checked y'all's suggested patch and it indeed works for us. Sorry for the mixup. |
|
I create another PR #231 for the red zone fix. Could you help review it? If it is OK, we will merge it later. |
We use the exception handler to implement cpuid. To get that to work we have to ensure more of the stack is available.
continue_executionshould not store the instruction address for resumption in a partof the stack that is likely in use. Otherwise, it stores it at the next stack location, which at least OPENSSL_cpuid has in use at the time of CPUID execution, corrupting important information.