Skip to content

Commit ad57b94

Browse files
author
Simon Leet
committed
Clear xstates on enclave entry to prevent ABI poisoning
- Refactor oe_cleanup_xstates() out of exception.c as a macro in asmcommon.inc - Remove workaround for #144 - Add OE_XSAVE_INITIAL_STATE definition in new asmdefs.c file - oe_cleanup_xstates now does an xrstor to OE_XSAVE_INITIAL_STATE - oe_enter now also calls oe_cleanup_xstates as part of .call_function - Remove workaround for #144 Signed-off-by: Simon Leet <simon.leet@microsoft.com>
1 parent fde4e46 commit ad57b94

File tree

6 files changed

+96
-85
lines changed

6 files changed

+96
-85
lines changed

‎enclave/core/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ if (OE_SGX)
7878
../../common/sgx/endorsements.c
7979
../../common/sgx/rand.S
8080
sgx/arena.c
81+
sgx/asmdefs.c
8182
sgx/backtrace.c
8283
sgx/calls.c
8384
sgx/cpuid.c

‎enclave/core/sgx/asmcommon.inc‎

Lines changed: 46 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6,31 +6,56 @@
66

77
//==============================================================================
88
//
9-
// This macro is used to cleanup the enclave Registers. Following registers will be scrubbed:
10-
// 1. General Purpose registers excluding RAX, RBX, RCX, RDI, RSI, RBP, and RSP
11-
// RAX and RBX are input registers of EEXIT.
12-
// RCX is output register of EEXIT.
13-
// RDI and RSI is output parameters defined by SDK.
14-
// RBP and RSP will be set to host rbp and rsp right before EEXIT is executed.
9+
// This macro is used to reset all XSAVE supported state components to a
10+
// clean initial state. This includes:
11+
//
12+
// - The legacy SSE state:
13+
// - Initializes the FPU control word to ABI-specified value 0x037F
14+
// - Initializes MXCSR to ABI-specified value 0x1F80 and MXCSR_MASK to 0xFFFF
15+
// - Clears FPU Status, Tag, OpCode, and FIP words
16+
// - Clears all FDP bits
17+
// - Clears all MMX/FPU registers
18+
// - Clears all XMM registers
19+
//
20+
// - The extended XSAVE state components:
21+
// - Clears all XSTATE_BV bits
22+
// - Sets XCOMP_BV bit-63 to support compaction mode
23+
// - Clears all other XCOMP_BV bits
24+
//
25+
//==============================================================================
26+
.macro oe_cleanup_xstates
27+
// Preserve registers being used
28+
push %rax
29+
push %rdx
30+
31+
// Set the XSAVE_MASK
32+
mov $0xFFFFFFFF, %eax
33+
mov $0xFFFFFFFF, %edx
34+
35+
// Restore initial enclave XSAVE state
36+
xrstor64 OE_XSAVE_INITIAL_STATE(%rip)
37+
38+
// Restore the registers
39+
pop %rdx
40+
pop %rax
41+
.endm
42+
43+
//==============================================================================
44+
//
45+
// This macro is used to clean up the enclave registers in addition to the
46+
// extended states (see oe_cleanup_xstates).
47+
//
48+
// It scrubs all general purpose registers excluding:
49+
// - RAX and RBX, which are used as input registers of EEXIT.
50+
// - RCX, which is used as the output register of EEXIT.
51+
// - RDI and RSI, which are used as output parameters defined by SDK.
52+
// - RBP and RSP, which will be set to host values of RBP & RSP right before
53+
// EEXIT is executed.
1554
//
16-
// 2. Legacy SSE.
17-
// 3. Extended XState components.
1855
//==============================================================================
1956
.macro oe_cleanup_registers
2057
// Scrub both Legacy SSE and extended XSTATEs.
21-
// Save the rax, rcx, rdi and rsi.
22-
mov %rax, %r12
23-
mov %rcx, %r13
24-
mov %rdi, %r14
25-
mov %rsi, %r15
26-
27-
call oe_cleanup_xstates
28-
29-
// Restore the rax, rcx, rdi and rsi.
30-
mov %r12, %rax
31-
mov %r13, %rcx
32-
mov %r14, %rdi
33-
mov %r15, %rsi
58+
oe_cleanup_xstates
3459

3560
// Zero out GPRs.
3661
// Retain r11 since it is used as a reference to the td structure.

‎enclave/core/sgx/asmdefs.c‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// Copyright (c) Open Enclave SDK contributors.
2+
// Licensed under the MIT License.
3+
4+
#include <openenclave/bits/types.h>
5+
#include <openenclave/internal/constants_x64.h>
6+
7+
/*
8+
* Initialization state for XSAVE area in the enclave.
9+
*/
10+
/* clang-format off */
11+
OE_ALIGNED(XSAVE_ALIGNMENT) const uint32_t
12+
OE_XSAVE_INITIAL_STATE[MINIMAL_XSTATE_AREA_LENGTH/sizeof(uint32_t)] = {
13+
14+
/* LEGACY_XSAVE_AREA */
15+
/* Set FPU Control Word to ABI init value of 0x037F,
16+
* clear Status, Tag, OpCode, FIP words */
17+
0x037F, 0, 0, 0,
18+
19+
/* Clear FDP bits, set MXCSR to ABI init value of 0x1F80
20+
* and MXCSR_MASK to all bits (0XFFFF) */
21+
0, 0, 0x1F80, 0xFFFF,
22+
23+
/* Clear ST/MM0-7 */
24+
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
25+
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
26+
27+
/* Clear XMM0-15 */
28+
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
29+
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
30+
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
31+
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
32+
33+
/* Reserved bits up to end of LEGACY_XSAVE_AREA */
34+
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
35+
0, 0, 0, 0, 0, 0, 0, 0,
36+
37+
/* XSAVE Header */
38+
/* Clear XSTATE_BV, set XCOMP_BV[63] = 1 to support compaction mode;
39+
* SGX-capable systems should support it */
40+
0, 0, 0, 0x80000000,
41+
42+
/* Reserved XSAVE header bits */
43+
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
44+
};
45+
/* clang-format on */

‎enclave/core/sgx/enter.S‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -133,18 +133,6 @@ oe_enter:
133133
mov %rsp, %rbp
134134

135135
.call_function:
136-
// Set the MXCSR according to the Linux x86_64 ABI
137-
mov $ABI_MXCSR_INIT, %r10
138-
push %r10
139-
ldmxcsr (%rsp)
140-
pop %r10
141-
142-
// Set the FPU Control Word according to the Linux x86_64 ABI
143-
mov $ABI_FPUCW_INIT, %r10
144-
push %r10
145-
fldcw (%rsp)
146-
pop %r10
147-
148136
// Initialize the RFLAGS prior to calling enclave functions
149137
// This only clears the DF and state flag bits since
150138
// the system flags and reserved bits are not writable here
@@ -189,6 +177,9 @@ oe_enter:
189177
// Save reference to the td structure to enclave stack.
190178
mov %r11, OM_ENC_TD
191179

180+
// Clear the XSTATE so that enclave has clean legacy SSE and extended states
181+
oe_cleanup_xstates
182+
192183
// Call __oe_handle_main(ARG1=RDI, ARG2=RSI, CSSA=RDX, TCS=RCX, OUTPUTARG1=R8, OUTPUTARG2=R9)
193184
mov %rax, %rdx
194185
mov %rbx, %rcx

‎enclave/core/sgx/exception.c‎

Lines changed: 0 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,10 @@
33

44
#include <openenclave/bits/sgx/sgxtypes.h>
55
#include <openenclave/enclave.h>
6-
#include <openenclave/internal/calls.h>
7-
#include <openenclave/internal/constants_x64.h>
86
#include <openenclave/internal/context.h>
97
#include <openenclave/internal/cpuid.h>
10-
#include <openenclave/internal/fault.h>
11-
#include <openenclave/internal/globals.h>
12-
#include <openenclave/internal/jump.h>
13-
#include <openenclave/internal/safecrt.h>
148
#include <openenclave/internal/sgx/td.h>
159
#include <openenclave/internal/thread.h>
16-
#include <openenclave/internal/trace.h>
17-
#include "asmdefs.h"
1810
#include "cpuid.h"
1911
#include "init.h"
2012
#include "td.h"
@@ -386,38 +378,3 @@ void oe_virtual_exception_dispatcher(
386378
*arg_out = OE_EXCEPTION_CONTINUE_EXECUTION;
387379
return;
388380
}
389-
390-
/*
391-
**==============================================================================
392-
**
393-
** void oe_cleanup_xstates(void)
394-
**
395-
** Cleanup all XSTATE registers that include both legacy registers and extended
396-
** registers.
397-
**
398-
**==============================================================================
399-
*/
400-
401-
void oe_cleanup_xstates(void)
402-
{
403-
// Temporary workaround for #144 xrstor64 fault with optimized builds as
404-
// reserved guard pages
405-
// are incorrectly accessed. Xsave area is increased from 0x240 to 0x1000.
406-
// Making these static
407-
OE_ALIGNED(XSAVE_ALIGNMENT)
408-
static uint8_t
409-
xsave_area[MINIMAL_XSTATE_AREA_LENGTH]; //#144 Making this static
410-
//__builtin_ia32_xrstor64 has different argument types in clang and gcc
411-
#ifdef __clang__
412-
uint64_t restore_mask = ~((uint64_t)0x0);
413-
#else
414-
int64_t restore_mask = ~(0x0);
415-
#endif
416-
417-
// The legacy registers(F87, SSE) values will be loaded from the
418-
// LEGACY_XSAVE_AREA that at beginning of xsave_area.The extended registers
419-
// will be initialized to their default values.
420-
__builtin_ia32_xrstor64(xsave_area, restore_mask);
421-
422-
return;
423-
}

‎include/openenclave/internal/constants_x64.h‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,7 @@
6060
#define XSAVE_ALIGNMENT 0x40
6161
#define LEGACY_XSAVE_AREA 0X200
6262
#define XSAVE_HEADER_LENGTH 0X40
63-
//#define MINIMAL_XSTATE_AREA_LENGTH (LEGACY_XSAVE_AREA + XSAVE_HEADER_LENGTH)
64-
// Workaround for #144 xrstor64 fault - Increasing from 0x240 to 0x1000
65-
#define MINIMAL_XSTATE_AREA_LENGTH 0x1000
63+
#define MINIMAL_XSTATE_AREA_LENGTH (LEGACY_XSAVE_AREA + XSAVE_HEADER_LENGTH)
6664

6765
//
6866
// AMD64 ABI related constants.
@@ -71,10 +69,4 @@
7169
// AMD64 ABI needs a 128 bytes red zone.
7270
#define ABI_REDZONE_BYTE_SIZE 0x80
7371

74-
// MXCSR initialization value for Linux x86_64 ABI in enclave
75-
#define ABI_MXCSR_INIT 0x1F80
76-
77-
// x87 FPU control word initialization value for Linux x86_64 ABI in enclave
78-
#define ABI_FPUCW_INIT 0x037F
79-
8072
#endif /* _OE_INTERNAL_CONSTANTS_X64_H */

0 commit comments

Comments
 (0)