Browse Source

[Pal/Linux-SGX] Remove calculation of frame from _DkExceptionHandler()

Graphene-SGX has two logical paths: (1) starting from _DkHandleExternalEvent()
if thread received a signal while not in enclave mode, and (2) starting from
_DkExceptionHandler() if thread was in enclave mode (both in db_exception.c).

_DkExceptionHandler() contained two racy bugs pertaining to the frame variable.
Bugs are described in detail in issue #230. The frame tries to find the outer-
most enclosing Dk* PAL function, but since _DkExceptionHandler() happens
during enclave-mode execution, there may be no such frame at all. That is why
_DkExceptionHandler() always calculates and passes further the enclave
context. Thus, the frame is not needed at all and can be removed, thus
eliminating the corresponding bugs. As a result, an intermediate function
_DkExceptionRealHandler() was also removed.
Dmitrii Kuvaiskii 4 years ago
parent
commit
c6622ba12e

+ 0 - 2
Documentation/oldwiki/Signal-Handling-in-Graphene.md

@@ -154,8 +154,6 @@ On the example of SIGINT, until we arrive into `_DkGenericSignalHandle()`.
 | | | +                                                                                           |a
 | | | | PAL_CONTEXT ctx = copy(uc)  <ctx contains interrupted-context frame>                      |v
 | | | |                                                                                           |e
-| | | | _DkExceptionRealHandler(PAL_EVENT_SUSPEND, ctx)                                           |
-| | | |                                                                                           |
 + + + + _DkGenericSignalHandle(PAL_EVENT_SUSPEND, ctx)                                            v
           < ... >
 ```

+ 1 - 36
Pal/src/host/Linux-SGX/db_exception.c

@@ -121,39 +121,6 @@ __asm__ (".type arch_exception_return_asm, @function;"
 
 extern void arch_exception_return (void) __asm__ ("arch_exception_return_asm");
 
-void _DkExceptionRealHandler (int event, PAL_NUM arg, struct pal_frame * frame,
-                              PAL_CONTEXT * context)
-{
-    if (frame) {
-        frame = __alloca(sizeof(struct pal_frame));
-        frame->identifier = PAL_FRAME_IDENTIFIER;
-        frame->func     = &_DkExceptionRealHandler;
-        frame->funcname = "_DkExceptionRealHandler";
-
-        store_register(rsp, frame->arch.rsp);
-        store_register(rbp, frame->arch.rbp);
-        unsigned long * last_frame = ((unsigned long *) frame->arch.rbp) + 1;
-        last_frame[0]  = (unsigned long) arch_exception_return;
-        last_frame[1]  = context->rax;
-        last_frame[2]  = context->rbx;
-        last_frame[3]  = context->rcx;
-        last_frame[4]  = context->rdx;
-        last_frame[5]  = context->rsi;
-        last_frame[6]  = context->rdi;
-        last_frame[7]  = context->r8;
-        last_frame[8]  = context->r9;
-        last_frame[9]  = context->r10;
-        last_frame[10] = context->r11;
-        last_frame[11] = context->r12;
-        last_frame[12] = context->r13;
-        last_frame[13] = context->r14;
-        last_frame[14] = context->r15;
-        last_frame[15] = context->rip;
-    }
-
-    _DkGenericSignalHandle(event, arg, frame, context);
-}
-
 /*
  * return value:
  *  true:  #UD is handled.
@@ -281,8 +248,6 @@ void _DkExceptionHandler (unsigned int exit_info, sgx_cpu_context_t * uc)
     ctx.efl = uc->rflags;
     ctx.rip = uc->rip;
 
-    struct pal_frame * frame = get_frame(uc);
-
     PAL_NUM arg = 0;
     switch (event_num) {
     case PAL_EVENT_ILLEGAL:
@@ -298,7 +263,7 @@ void _DkExceptionHandler (unsigned int exit_info, sgx_cpu_context_t * uc)
         /* nothing */
         break;
     }
-    _DkExceptionRealHandler(event_num, arg, frame, &ctx);
+    _DkGenericSignalHandle(event_num, arg, NULL, &ctx);
     restore_sgx_context(uc);
 }