Merge #3984

3984: Support the CapturePFGPExceptions preference r=mingweishih a=mingweishih Expose the CapturePFGPExceptions configuration as part of SGX2 features, allowing the enclave to capture #PF and #GP exceptions. More detailed about the PR - Add the CapturePFGPExceptions preference to the enclave config file. `oesign` now is able to parse the option (expected to be a binary value) - oesign can sign the enclave with CapturePFGPExceptions=1 on both SGX1 and SGX2 machines. - When CapturePFGPExceptions=1, the OE loader will enable the feature when running on an SGX2-capable CPU. - When feature is enabled on SGX2 machines, the `OE_EXCEPTION_PAGE_FAULT` and `OE_EXCEPTION_ACCESS_VIOLATION` code can be used to check if the exception is #PF or #GP, respectively. - `faulting_address` and `error_code` of the `oe_exception_record_t` struct will contain the information of the exceptions. - `include/openenclave/bits/exception.h` includes the information based on Intel SDM (Table 37.14) used to interpret the `error_code` Fixes #3923 Signed-off-by: Ming-Wei Shih <mishih@microsoft.com> Co-authored-by: Ming-Wei Shih <mishih@microsoft.com>
openenclave · Jun 1, 2021 · 056f78f · 056f78f
2 parents 152ec28 + fe5f678
commit 056f78f
Show file tree

Hide file tree

Showing 23 changed files with 504 additions and 13 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 [Unreleased][Unreleased_log]
 --------------
 
+### Added
+- Add the CapturePFGPExceptions preference for the SGX2 feature of capturing #PF and #GP exceptions inside an enclave.
+  - Developers can specify the CapturePFGPExceptions with a binary value in the enclave config file or set the value via the newly added OE_SET_ENCLAVE_SGX2 macro, which is used to set SGX2-specific properties.
+  - When setting CapturePFGPExceptions=1, the OE loader will enable the feature when running on an SGX2-capable CPU.
+  - Once enabled, the in-enclave exception handler can capture the #PF (with the OE_EXCEPTION_PAGE_FAULT code) and #GP (with the code OE_EXCEPTION_ACCESS_VIOLATION code) exceptions.
+  - More information about the exceptions can be found in the `faulting_address` and `error_code` members of the `oe_exception_record_t` structure passed into the handler.
+
 [v0.16.1][v0.16.1_log]
 --------------
 ### Added

diff --git a/enclave/core/sgx/exception.c b/enclave/core/sgx/exception.c
@@ -249,6 +249,8 @@ void oe_real_exception_dispatcher(oe_context_t* oe_context)
     oe_exception_record.code = td->exception_code;
     oe_exception_record.flags = td->exception_flags;
     oe_exception_record.address = td->exception_address;
+    oe_exception_record.faulting_address = td->faulting_address;
+    oe_exception_record.error_code = td->error_code;
     oe_exception_record.context = oe_context;
 
     // Refer to oe_enter in host/sgx/enter.c. The contract we defined for EENTER
@@ -272,6 +274,13 @@ void oe_real_exception_dispatcher(oe_context_t* oe_context)
         }
     }
 
+    // Clear information after all the handlers are done
+    td->exception_code = 0;
+    td->exception_flags = 0;
+    td->exception_address = 0;
+    td->faulting_address = 0;
+    td->error_code = 0;
+
     // Jump to the point where oe_context refers to and continue.
     if (handler_ret == OE_EXCEPTION_CONTINUE_EXECUTION)
     {
@@ -317,8 +326,9 @@ void oe_virtual_exception_dispatcher(
         return;
     }
 
-    sgx_ssa_gpr_t* ssa_gpr =
-        (sgx_ssa_gpr_t*)(((uint8_t*)ssa_info.base_address) + ssa_info.frame_byte_size - OE_SGX_GPR_BYTE_SIZE);
+    uint64_t gprsgx_offset = (uint64_t)ssa_info.base_address +
+                             ssa_info.frame_byte_size - OE_SGX_GPR_BYTE_SIZE;
+    sgx_ssa_gpr_t* ssa_gpr = (sgx_ssa_gpr_t*)gprsgx_offset;
     if (!ssa_gpr->exit_info.as_fields.valid)
     {
         // Not a valid/expected enclave exception;
@@ -364,6 +374,16 @@ void oe_virtual_exception_dispatcher(
     }
     else
     {
+        /* The following codes can only be captured with SGX2 and the
+         * MISCSELECT[0] bit is set to 1. */
+        if (td->exception_code == OE_EXCEPTION_PAGE_FAULT ||
+            td->exception_code == OE_EXCEPTION_ACCESS_VIOLATION)
+        {
+            sgx_exinfo_t* exinfo =
+                (sgx_exinfo_t*)(gprsgx_offset - OE_SGX_MISC_BYTE_SIZE);
+            td->faulting_address = exinfo->maddr;
+            td->error_code = exinfo->errcd;
+        }
         // Modify the ssa_gpr so that e_resume will go to second pass exception
         // handler.
         ssa_gpr->rip = (uint64_t)oe_exception_dispatcher;

diff --git a/host/sgx/cpuid.h b/host/sgx/cpuid.h
@@ -13,6 +13,10 @@
 #error "oe_get_cpuid(): no cpuid intrinsic mapping for this compiler"
 #endif
 
+#define CPUID_SGX_LEAF 0x12
+#define CPUID_SGX_KSS_MASK 0x80
+#define CPUID_SGX_MISC_EXINFO_MASK 0x01
+
 /* Same as __get_cpuid, but sub-leaf can be specified.
    Need this function as cpuid level 4 needs the sub-leaf to be specified in ECX
 */

diff --git a/host/sgx/create.c b/host/sgx/create.c
@@ -97,10 +97,25 @@ static bool _is_kss_supported()
     eax = ebx = ecx = edx = 0;
 
     // Obtain feature information using CPUID
-    oe_get_cpuid(0x12, 0x1, &eax, &ebx, &ecx, &edx);
+    oe_get_cpuid(CPUID_SGX_LEAF, 0x1, &eax, &ebx, &ecx, &edx);
 
     // Check if KSS (bit 7) is supported by the processor
-    if (!(eax & (1 << 7)))
+    if (!(eax & CPUID_SGX_KSS_MASK))
+        return false;
+    else
+        return true;
+}
+
+static bool _is_misc_region_supported()
+{
+    uint32_t eax, ebx, ecx, edx;
+    eax = ebx = ecx = edx = 0;
+
+    // Obtain feature information using CPUID
+    oe_get_cpuid(CPUID_SGX_LEAF, 0x0, &eax, &ebx, &ecx, &edx);
+
+    // Check if EXINFO is supported by the processor
+    if (!(ebx & CPUID_SGX_MISC_EXINFO_MASK))
         return false;
     else
         return true;
@@ -721,6 +736,7 @@ static oe_result_t _eeid_resign(
             properties->config.attributes,
             properties->config.product_id,
             properties->config.security_version,
+            &properties->config.flags,
             OE_DEBUG_SIGN_KEY,
             OE_DEBUG_SIGN_KEY_SIZE,
             properties->config.family_id,
@@ -883,6 +899,14 @@ oe_result_t oe_sgx_build_enclave(
         &loaded_enclave_pages_size,
         &enclave_size));
 
+    /* Check if the enclave is configured with CapturePFGPExceptions=1 */
+    if (props.config.flags.capture_pf_gp_exceptions)
+    {
+        /* Only opt into the feature if CPU (SGX2) supports the MISC region. */
+        if (_is_misc_region_supported())
+            context->capture_pf_gp_exceptions_enabled = 1;
+    }
+
     if (props.config.attributes & OE_SGX_FLAGS_KSS)
     {
         if ((context->type == OE_SGX_LOAD_TYPE_CREATE) && !_is_kss_supported())

diff --git a/host/sgx/sgxload.c b/host/sgx/sgxload.c
@@ -162,6 +162,10 @@ static sgx_secs_t* _new_secs(
         secs->config_svn = context->config_data->config_svn;
     }
 
+    /* Set the EXINFO bit if CapturePFGPExceptions=1 */
+    if (context->capture_pf_gp_exceptions_enabled)
+        secs->misc_select |= SGX_SECS_MISCSELECT_EXINFO;
+
     return secs;
 }
 
@@ -262,6 +266,7 @@ static oe_result_t _get_sig_struct(
             properties->config.attributes,
             properties->config.product_id,
             properties->config.security_version,
+            &properties->config.flags,
             OE_DEBUG_SIGN_KEY,
             OE_DEBUG_SIGN_KEY_SIZE,
             properties->config.family_id,

diff --git a/host/sgx/sgxsign.c b/host/sgx/sgxsign.c
@@ -571,6 +571,7 @@ static oe_result_t _init_sigstruct(
     uint64_t attributes,
     uint16_t product_id,
     uint16_t security_version,
+    const oe_sgx_enclave_flags_t* flags,
     const uint8_t* family_id,
     const uint8_t* extended_product_id,
     sgx_sigstruct_t* sigstruct)
@@ -620,7 +621,10 @@ static oe_result_t _init_sigstruct(
     sigstruct->miscselect = SGX_SIGSTRUCT_MISCSELECT;
 
     /* sgx_sigstruct_t.miscmask */
-    sigstruct->miscmask = SGX_SIGSTRUCT_MISCMASK;
+    if (flags && flags->capture_pf_gp_exceptions)
+        sigstruct->miscmask = SGX_SIGSTRUCT_MISCMASK_EXINFO;
+    else
+        sigstruct->miscmask = SGX_SIGSTRUCT_MISCMASK;
 
     /* sgx_sigstruct_t.attributes */
     sigstruct->attributes.flags = attributes;
@@ -681,6 +685,7 @@ oe_result_t oe_sgx_sign_enclave_from_engine(
     uint64_t attributes,
     uint16_t product_id,
     uint16_t security_version,
+    const oe_sgx_enclave_flags_t* flags,
     const char* engine_id,
     const char* engine_load_path,
     const char* key_id,
@@ -709,6 +714,7 @@ oe_result_t oe_sgx_sign_enclave_from_engine(
         attributes,
         product_id,
         security_version,
+        flags,
         family_id,
         extended_product_id,
         sigstruct));
@@ -728,6 +734,7 @@ oe_result_t oe_sgx_sign_enclave(
     uint64_t attributes,
     uint16_t product_id,
     uint16_t security_version,
+    const oe_sgx_enclave_flags_t* flags,
     const uint8_t* pem_data,
     size_t pem_size,
     const uint8_t* family_id,
@@ -755,6 +762,7 @@ oe_result_t oe_sgx_sign_enclave(
         attributes,
         product_id,
         security_version,
+        flags,
         family_id,
         extended_product_id,
         sigstruct));
@@ -774,6 +782,7 @@ oe_result_t oe_sgx_get_sigstruct_digest(
     uint64_t attributes,
     uint16_t product_id,
     uint16_t security_version,
+    const oe_sgx_enclave_flags_t* flags,
     const uint8_t* family_id,
     const uint8_t* extended_product_id,
     OE_SHA256* digest)
@@ -794,6 +803,7 @@ oe_result_t oe_sgx_get_sigstruct_digest(
         attributes,
         product_id,
         security_version,
+        flags,
         family_id,
         extended_product_id,
         &sigstruct));
@@ -810,6 +820,7 @@ oe_result_t oe_sgx_digest_sign_enclave(
     uint64_t attributes,
     uint16_t product_id,
     uint16_t security_version,
+    const oe_sgx_enclave_flags_t* flags,
     const uint8_t* cert_pem_data,
     size_t cert_pem_size,
     const uint8_t* digest_signature,
@@ -845,6 +856,7 @@ oe_result_t oe_sgx_digest_sign_enclave(
         attributes,
         product_id,
         security_version,
+        flags,
         family_id,
         extended_product_id,
         sigstruct));

diff --git a/include/openenclave/bits/exception.h b/include/openenclave/bits/exception.h
@@ -76,6 +76,56 @@ OE_EXTERNC_BEGIN
  */
 #define OE_EXCEPTION_FLAGS_SOFTWARE 0x2
 
+/**
+ * The following flags are used to interpret the error_code of the
+ * oe_exception_record_t struct when a PF or GP exception occurs. Note that
+ * these exceptions are captured only with SGX2 CPU and the MISCSELCT[0] is set
+ * to 1.
+ */
+
+/**
+ * Page-protection violation flag
+ * 0 - The fault was caused by a non-present page.
+ * 1 - The fault was caused by a page-protection violation.
+ */
+#define OE_SGX_PAGE_FAULT_P_FLAG 0x1
+/**
+ * Read/Write flag
+ * 0 - The fault was caused by a read.
+ * 1 - The fault was caused by a write.
+ */
+#define OE_SGX_PAGE_FAULT_WR_FLAG 0x2
+/**
+ * U/S flag
+ * 0 - The fault was caused by a supervisor-mode access.
+ * 1 - The fault was caused by a user-mode access.
+ */
+#define OE_SGX_PAGE_FAULT_US_FLAG 0x4
+/**
+ * RSVD flag
+ * 0 - The fault was not caused by a reserved bit violation.
+ * 1 - The fault was caused by a reserved bit set to 1
+ */
+#define OE_SGX_PAGE_FAULT_RSVD 0x8
+/**
+ * I/D flag
+ * 0 - The fault was not caused by an instruction fetch.
+ * 1 - The fault was caused by an instruction fetch.
+ */
+#define OE_SGX_PAGE_FAULT_ID_FLAG 0x10
+/**
+ * Protection Key flag
+ * 0 - The fault was not caused by protection keys.
+ * 1 - The fault was caused by protection-key violation.
+ */
+#define OE_SGX_PAGE_FAULT_PK_FLAG 0x20
+/**
+ * SGX flag
+ * 0 - The fault was not related to SGX.
+ * 1 - The fault is SGX-specific (e.g., access violation).
+ */
+#define OE_SGX_PAGE_FAULT_SGX_FLAG 0x8000
+
 /**
  * Blob that contains X87 and SSE data.
  */
@@ -154,6 +204,12 @@ typedef struct _oe_exception_record
 
     uint64_t address; /**< Exception address */
 
+    /* Information for PF/GP exceptions, which are only available
+     * for SGX2 and requires application opt-in (set CapturePFGPExceptions=1).
+     */
+    uint64_t faulting_address;
+    uint32_t error_code;
+
     oe_context_t* context; /**< Exception context */
 } oe_exception_record_t;
 /**< typedef struct _oe_exception_record oe_exception_record_t*/