Skip to content

Commit ba7dd85

Browse files
committed
Merge tag 'x86_sgx_for_6.8' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
Pull x86 SGX updates from Dave Hansen: "This time, these are entirely confined to SGX selftests fixes. The mini SGX enclave built by the selftests has garnered some attention because it stands alone and does not need the sizable infrastructure of the official SGX SDK. I think that's why folks are suddently interested in cleaning it up. - Clean up selftest compilation issues, mostly from non-gcc compilers - Avoid building selftests when not on x86" * tag 'x86_sgx_for_6.8' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip: selftests/sgx: Skip non X86_64 platform selftests/sgx: Remove incomplete ABI sanitization code in test enclave selftests/sgx: Discard unsupported ELF sections selftests/sgx: Ensure expected location of test enclave buffer selftests/sgx: Ensure test enclave buffer is entirely preserved selftests/sgx: Fix linker script asserts selftests/sgx: Handle relocations in test enclave selftests/sgx: Produce static-pie executable for test enclave selftests/sgx: Remove redundant enclave base address save/restore selftests/sgx: Specify freestanding environment for enclave compilation selftests/sgx: Separate linker options selftests/sgx: Include memory clobber for inline asm in test enclave selftests/sgx: Fix uninitialized pointer dereferences in encl_get_entry selftests/sgx: Fix uninitialized pointer dereference in error path
2 parents b0d326d + 981cf56 commit ba7dd85

7 files changed

Lines changed: 78 additions & 57 deletions

File tree

tools/testing/selftests/sgx/Makefile

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,16 @@ OBJCOPY := $(CROSS_COMPILE)objcopy
1212
endif
1313

1414
INCLUDES := -I$(top_srcdir)/tools/include
15-
HOST_CFLAGS := -Wall -Werror -g $(INCLUDES) -fPIC -z noexecstack
16-
ENCL_CFLAGS := -Wall -Werror -static -nostdlib -nostartfiles -fPIC \
15+
HOST_CFLAGS := -Wall -Werror -g $(INCLUDES) -fPIC
16+
HOST_LDFLAGS := -z noexecstack -lcrypto
17+
ENCL_CFLAGS += -Wall -Werror -static-pie -nostdlib -ffreestanding -fPIE \
1718
-fno-stack-protector -mrdrnd $(INCLUDES)
19+
ENCL_LDFLAGS := -Wl,-T,test_encl.lds,--build-id=none
1820

21+
ifeq ($(CAN_BUILD_X86_64), 1)
1922
TEST_CUSTOM_PROGS := $(OUTPUT)/test_sgx
2023
TEST_FILES := $(OUTPUT)/test_encl.elf
2124

22-
ifeq ($(CAN_BUILD_X86_64), 1)
2325
all: $(TEST_CUSTOM_PROGS) $(OUTPUT)/test_encl.elf
2426
endif
2527

@@ -28,7 +30,7 @@ $(OUTPUT)/test_sgx: $(OUTPUT)/main.o \
2830
$(OUTPUT)/sigstruct.o \
2931
$(OUTPUT)/call.o \
3032
$(OUTPUT)/sign_key.o
31-
$(CC) $(HOST_CFLAGS) -o $@ $^ -lcrypto
33+
$(CC) $(HOST_CFLAGS) -o $@ $^ $(HOST_LDFLAGS)
3234

3335
$(OUTPUT)/main.o: main.c
3436
$(CC) $(HOST_CFLAGS) -c $< -o $@
@@ -45,8 +47,8 @@ $(OUTPUT)/call.o: call.S
4547
$(OUTPUT)/sign_key.o: sign_key.S
4648
$(CC) $(HOST_CFLAGS) -c $< -o $@
4749

48-
$(OUTPUT)/test_encl.elf: test_encl.lds test_encl.c test_encl_bootstrap.S
49-
$(CC) $(ENCL_CFLAGS) -T $^ -o $@ -Wl,--build-id=none
50+
$(OUTPUT)/test_encl.elf: test_encl.c test_encl_bootstrap.S
51+
$(CC) $(ENCL_CFLAGS) $^ -o $@ $(ENCL_LDFLAGS)
5052

5153
EXTRA_CLEAN := \
5254
$(OUTPUT)/test_encl.elf \

tools/testing/selftests/sgx/defines.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313

1414
#define __aligned(x) __attribute__((__aligned__(x)))
1515
#define __packed __attribute__((packed))
16+
#define __used __attribute__((used))
17+
#define __section(x)__attribute__((__section__(x)))
1618

1719
#include "../../../../arch/x86/include/asm/sgx.h"
1820
#include "../../../../arch/x86/include/asm/enclu.h"

tools/testing/selftests/sgx/load.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,11 +136,11 @@ static bool encl_ioc_add_pages(struct encl *encl, struct encl_segment *seg)
136136
*/
137137
uint64_t encl_get_entry(struct encl *encl, const char *symbol)
138138
{
139+
Elf64_Sym *symtab = NULL;
140+
char *sym_names = NULL;
139141
Elf64_Shdr *sections;
140-
Elf64_Sym *symtab;
141142
Elf64_Ehdr *ehdr;
142-
char *sym_names;
143-
int num_sym;
143+
int num_sym = 0;
144144
int i;
145145

146146
ehdr = encl->bin;
@@ -161,6 +161,9 @@ uint64_t encl_get_entry(struct encl *encl, const char *symbol)
161161
}
162162
}
163163

164+
if (!symtab || !sym_names)
165+
return 0;
166+
164167
for (i = 0; i < num_sym; i++) {
165168
Elf64_Sym *sym = &symtab[i];
166169

tools/testing/selftests/sgx/sigstruct.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -318,9 +318,9 @@ bool encl_measure(struct encl *encl)
318318
struct sgx_sigstruct *sigstruct = &encl->sigstruct;
319319
struct sgx_sigstruct_payload payload;
320320
uint8_t digest[SHA256_DIGEST_LENGTH];
321+
EVP_MD_CTX *ctx = NULL;
321322
unsigned int siglen;
322323
RSA *key = NULL;
323-
EVP_MD_CTX *ctx;
324324
int i;
325325

326326
memset(sigstruct, 0, sizeof(*sigstruct));
@@ -384,7 +384,8 @@ bool encl_measure(struct encl *encl)
384384
return true;
385385

386386
err:
387-
EVP_MD_CTX_destroy(ctx);
387+
if (ctx)
388+
EVP_MD_CTX_destroy(ctx);
388389
RSA_free(key);
389390
return false;
390391
}

tools/testing/selftests/sgx/test_encl.c

Lines changed: 45 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,12 @@
55
#include "defines.h"
66

77
/*
8-
* Data buffer spanning two pages that will be placed first in .data
9-
* segment. Even if not used internally the second page is needed by
10-
* external test manipulating page permissions.
8+
* Data buffer spanning two pages that will be placed first in the .data
9+
* segment via the linker script. Even if not used internally the second page
10+
* is needed by external test manipulating page permissions, so mark
11+
* encl_buffer as "used" to make sure it is entirely preserved by the compiler.
1112
*/
12-
static uint8_t encl_buffer[8192] = { 1 };
13+
static uint8_t __used __section(".data.encl_buffer") encl_buffer[8192] = { 1 };
1314

1415
enum sgx_enclu_function {
1516
EACCEPT = 0x5,
@@ -24,10 +25,11 @@ static void do_encl_emodpe(void *_op)
2425
secinfo.flags = op->flags;
2526

2627
asm volatile(".byte 0x0f, 0x01, 0xd7"
27-
:
28+
: /* no outputs */
2829
: "a" (EMODPE),
2930
"b" (&secinfo),
30-
"c" (op->epc_addr));
31+
"c" (op->epc_addr)
32+
: "memory" /* read from secinfo pointer */);
3133
}
3234

3335
static void do_encl_eaccept(void *_op)
@@ -42,7 +44,8 @@ static void do_encl_eaccept(void *_op)
4244
: "=a" (rax)
4345
: "a" (EACCEPT),
4446
"b" (&secinfo),
45-
"c" (op->epc_addr));
47+
"c" (op->epc_addr)
48+
: "memory" /* read from secinfo pointer */);
4649

4750
op->ret = rax;
4851
}
@@ -119,21 +122,41 @@ static void do_encl_op_nop(void *_op)
119122

120123
}
121124

125+
/*
126+
* Symbol placed at the start of the enclave image by the linker script.
127+
* Declare this extern symbol with visibility "hidden" to ensure the compiler
128+
* does not access it through the GOT and generates position-independent
129+
* addressing as __encl_base(%rip), so we can get the actual enclave base
130+
* during runtime.
131+
*/
132+
extern const uint8_t __attribute__((visibility("hidden"))) __encl_base;
133+
134+
typedef void (*encl_op_t)(void *);
135+
static const encl_op_t encl_op_array[ENCL_OP_MAX] = {
136+
do_encl_op_put_to_buf,
137+
do_encl_op_get_from_buf,
138+
do_encl_op_put_to_addr,
139+
do_encl_op_get_from_addr,
140+
do_encl_op_nop,
141+
do_encl_eaccept,
142+
do_encl_emodpe,
143+
do_encl_init_tcs_page,
144+
};
145+
122146
void encl_body(void *rdi, void *rsi)
123147
{
124-
const void (*encl_op_array[ENCL_OP_MAX])(void *) = {
125-
do_encl_op_put_to_buf,
126-
do_encl_op_get_from_buf,
127-
do_encl_op_put_to_addr,
128-
do_encl_op_get_from_addr,
129-
do_encl_op_nop,
130-
do_encl_eaccept,
131-
do_encl_emodpe,
132-
do_encl_init_tcs_page,
133-
};
134-
135-
struct encl_op_header *op = (struct encl_op_header *)rdi;
136-
137-
if (op->type < ENCL_OP_MAX)
138-
(*encl_op_array[op->type])(op);
148+
struct encl_op_header *header = (struct encl_op_header *)rdi;
149+
encl_op_t op;
150+
151+
if (header->type >= ENCL_OP_MAX)
152+
return;
153+
154+
/*
155+
* The enclave base address needs to be added, as this call site
156+
* *cannot be* made rip-relative by the compiler, or fixed up by
157+
* any other possible means.
158+
*/
159+
op = ((uint64_t)&__encl_base) + encl_op_array[header->type];
160+
161+
(*op)(header);
139162
}

tools/testing/selftests/sgx/test_encl.lds

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ PHDRS
1010
SECTIONS
1111
{
1212
. = 0;
13+
__encl_base = .;
1314
.tcs : {
1415
*(.tcs*)
1516
} : tcs
@@ -23,6 +24,7 @@ SECTIONS
2324
} : text
2425

2526
.data : {
27+
*(.data.encl_buffer)
2628
*(.data*)
2729
} : data
2830

@@ -31,11 +33,9 @@ SECTIONS
3133
*(.note*)
3234
*(.debug*)
3335
*(.eh_frame*)
36+
*(.dyn*)
37+
*(.gnu.hash)
3438
}
3539
}
3640

37-
ASSERT(!DEFINED(.altinstructions), "ALTERNATIVES are not supported in enclaves")
38-
ASSERT(!DEFINED(.altinstr_replacement), "ALTERNATIVES are not supported in enclaves")
39-
ASSERT(!DEFINED(.discard.retpoline_safe), "RETPOLINE ALTERNATIVES are not supported in enclaves")
40-
ASSERT(!DEFINED(.discard.nospec), "RETPOLINE ALTERNATIVES are not supported in enclaves")
41-
ASSERT(!DEFINED(.got.plt), "Libcalls are not supported in enclaves")
41+
ASSERT(!DEFINED(_GLOBAL_OFFSET_TABLE_), "Libcalls through GOT are not supported in enclaves")

tools/testing/selftests/sgx/test_encl_bootstrap.S

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,12 @@
4242
encl_entry:
4343
# RBX contains the base address for TCS, which is the first address
4444
# inside the enclave for TCS #1 and one page into the enclave for
45-
# TCS #2. By adding the value of encl_stack to it, we get
46-
# the absolute address for the stack.
47-
lea (encl_stack)(%rbx), %rax
45+
# TCS #2. First make it relative by substracting __encl_base and
46+
# then add the address of encl_stack to get the address for the stack.
47+
lea __encl_base(%rip), %rax
48+
sub %rax, %rbx
49+
lea encl_stack(%rip), %rax
50+
add %rbx, %rax
4851
jmp encl_entry_core
4952
encl_dyn_entry:
5053
# Entry point for dynamically created TCS page expected to follow
@@ -55,25 +58,12 @@ encl_entry_core:
5558
push %rax
5659

5760
push %rcx # push the address after EENTER
58-
push %rbx # push the enclave base address
5961

62+
# NOTE: as the selftest enclave is *not* intended for production,
63+
# simplify the code by not initializing ABI registers on entry or
64+
# cleansing caller-save registers on exit.
6065
call encl_body
6166

62-
pop %rbx # pop the enclave base address
63-
64-
/* Clear volatile GPRs, except RAX (EEXIT function). */
65-
xor %rcx, %rcx
66-
xor %rdx, %rdx
67-
xor %rdi, %rdi
68-
xor %rsi, %rsi
69-
xor %r8, %r8
70-
xor %r9, %r9
71-
xor %r10, %r10
72-
xor %r11, %r11
73-
74-
# Reset status flags.
75-
add %rdx, %rdx # OF = SF = AF = CF = 0; ZF = PF = 1
76-
7767
# Prepare EEXIT target by popping the address of the instruction after
7868
# EENTER to RBX.
7969
pop %rbx

0 commit comments

Comments
 (0)