From 79eba8c924f7decfa71ddf187d38cb9f5f2cd7b3 Mon Sep 17 00:00:00 2001 From: Jo Van Bulck Date: Thu, 5 Oct 2023 17:38:42 +0200 Subject: selftests/sgx: Fix uninitialized pointer dereference in error path Ensure ctx is zero-initialized, such that the encl_measure function will not call EVP_MD_CTX_destroy with an uninitialized ctx pointer in case of an early error during key generation. Fixes: 2adcba79e69d ("selftests/x86: Add a selftest for SGX") Signed-off-by: Jo Van Bulck Signed-off-by: Dave Hansen Reviewed-by: Jarkko Sakkinen Acked-by: Kai Huang Link: https://lore.kernel.org/all/20231005153854.25566-2-jo.vanbulck%40cs.kuleuven.be --- tools/testing/selftests/sgx/sigstruct.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'tools') diff --git a/tools/testing/selftests/sgx/sigstruct.c b/tools/testing/selftests/sgx/sigstruct.c index a07896a46364..d73b29becf5b 100644 --- a/tools/testing/selftests/sgx/sigstruct.c +++ b/tools/testing/selftests/sgx/sigstruct.c @@ -318,9 +318,9 @@ bool encl_measure(struct encl *encl) struct sgx_sigstruct *sigstruct = &encl->sigstruct; struct sgx_sigstruct_payload payload; uint8_t digest[SHA256_DIGEST_LENGTH]; + EVP_MD_CTX *ctx = NULL; unsigned int siglen; RSA *key = NULL; - EVP_MD_CTX *ctx; int i; memset(sigstruct, 0, sizeof(*sigstruct)); @@ -384,7 +384,8 @@ bool encl_measure(struct encl *encl) return true; err: - EVP_MD_CTX_destroy(ctx); + if (ctx) + EVP_MD_CTX_destroy(ctx); RSA_free(key); return false; } -- cgit v1.2.3 From b84fc2e0139ba4b23b8039bd7cfd242894fe8f8b Mon Sep 17 00:00:00 2001 From: Jo Van Bulck Date: Thu, 5 Oct 2023 17:38:43 +0200 Subject: selftests/sgx: Fix uninitialized pointer dereferences in encl_get_entry Ensure sym_tab and sym_names are zero-initialized and add an early-out condition in the unlikely (erroneous) case that the enclave ELF file would not contain a symbol table. This addresses -Werror=maybe-uninitialized compiler warnings for gcc -O2. Fixes: 33c5aac3bf32 ("selftests/sgx: Test complete changing of page type flow") Signed-off-by: Jo Van Bulck Signed-off-by: Dave Hansen Reviewed-by: Jarkko Sakkinen Link: https://lore.kernel.org/all/20231005153854.25566-3-jo.vanbulck%40cs.kuleuven.be --- tools/testing/selftests/sgx/load.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'tools') diff --git a/tools/testing/selftests/sgx/load.c b/tools/testing/selftests/sgx/load.c index 94bdeac1cf04..c9f658e44de6 100644 --- a/tools/testing/selftests/sgx/load.c +++ b/tools/testing/selftests/sgx/load.c @@ -136,11 +136,11 @@ static bool encl_ioc_add_pages(struct encl *encl, struct encl_segment *seg) */ uint64_t encl_get_entry(struct encl *encl, const char *symbol) { + Elf64_Sym *symtab = NULL; + char *sym_names = NULL; Elf64_Shdr *sections; - Elf64_Sym *symtab; Elf64_Ehdr *ehdr; - char *sym_names; - int num_sym; + int num_sym = 0; int i; ehdr = encl->bin; @@ -161,6 +161,9 @@ uint64_t encl_get_entry(struct encl *encl, const char *symbol) } } + if (!symtab || !sym_names) + return 0; + for (i = 0; i < num_sym; i++) { Elf64_Sym *sym = &symtab[i]; -- cgit v1.2.3 From 853a57a43ebdb8c024160c1a0990bae85f4bcc2f Mon Sep 17 00:00:00 2001 From: Jo Van Bulck Date: Thu, 5 Oct 2023 17:38:44 +0200 Subject: selftests/sgx: Include memory clobber for inline asm in test enclave Add the "memory" clobber to the EMODPE and EACCEPT asm blocks to tell the compiler the assembly code accesses to the secinfo struct. This ensures the compiler treats the asm block as a memory barrier and the write to secinfo will be visible to ENCLU. Fixes: 20404a808593 ("selftests/sgx: Add test for EPCM permission changes") Signed-off-by: Jo Van Bulck Signed-off-by: Dave Hansen Reviewed-by: Kai Huang Reviewed-by: Jarkko Sakkinen Link: https://lore.kernel.org/all/20231005153854.25566-4-jo.vanbulck%40cs.kuleuven.be --- tools/testing/selftests/sgx/test_encl.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'tools') diff --git a/tools/testing/selftests/sgx/test_encl.c b/tools/testing/selftests/sgx/test_encl.c index c0d6397295e3..ae791df3e5a5 100644 --- a/tools/testing/selftests/sgx/test_encl.c +++ b/tools/testing/selftests/sgx/test_encl.c @@ -24,10 +24,11 @@ static void do_encl_emodpe(void *_op) secinfo.flags = op->flags; asm volatile(".byte 0x0f, 0x01, 0xd7" - : + : /* no outputs */ : "a" (EMODPE), "b" (&secinfo), - "c" (op->epc_addr)); + "c" (op->epc_addr) + : "memory" /* read from secinfo pointer */); } static void do_encl_eaccept(void *_op) @@ -42,7 +43,8 @@ static void do_encl_eaccept(void *_op) : "=a" (rax) : "a" (EACCEPT), "b" (&secinfo), - "c" (op->epc_addr)); + "c" (op->epc_addr) + : "memory" /* read from secinfo pointer */); op->ret = rax; } -- cgit v1.2.3 From f79464658d858e07af0c4c9b30f5baedeeae0c04 Mon Sep 17 00:00:00 2001 From: Jo Van Bulck Date: Thu, 5 Oct 2023 17:38:45 +0200 Subject: selftests/sgx: Separate linker options Fixes "'linker' input unused [-Wunused-command-line-argument]" errors when compiling with clang. Signed-off-by: Jo Van Bulck Signed-off-by: Dave Hansen Reviewed-by: Jarkko Sakkinen Link: https://lore.kernel.org/all/20231005153854.25566-5-jo.vanbulck%40cs.kuleuven.be --- tools/testing/selftests/sgx/Makefile | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'tools') diff --git a/tools/testing/selftests/sgx/Makefile b/tools/testing/selftests/sgx/Makefile index 50aab6b57da3..dcdd04b322f8 100644 --- a/tools/testing/selftests/sgx/Makefile +++ b/tools/testing/selftests/sgx/Makefile @@ -12,9 +12,11 @@ OBJCOPY := $(CROSS_COMPILE)objcopy endif INCLUDES := -I$(top_srcdir)/tools/include -HOST_CFLAGS := -Wall -Werror -g $(INCLUDES) -fPIC -z noexecstack -ENCL_CFLAGS := -Wall -Werror -static -nostdlib -nostartfiles -fPIC \ +HOST_CFLAGS := -Wall -Werror -g $(INCLUDES) -fPIC +HOST_LDFLAGS := -z noexecstack -lcrypto +ENCL_CFLAGS += -Wall -Werror -static -nostdlib -nostartfiles -fPIC \ -fno-stack-protector -mrdrnd $(INCLUDES) +ENCL_LDFLAGS := -Wl,-T,test_encl.lds,--build-id=none TEST_CUSTOM_PROGS := $(OUTPUT)/test_sgx TEST_FILES := $(OUTPUT)/test_encl.elf @@ -28,7 +30,7 @@ $(OUTPUT)/test_sgx: $(OUTPUT)/main.o \ $(OUTPUT)/sigstruct.o \ $(OUTPUT)/call.o \ $(OUTPUT)/sign_key.o - $(CC) $(HOST_CFLAGS) -o $@ $^ -lcrypto + $(CC) $(HOST_CFLAGS) -o $@ $^ $(HOST_LDFLAGS) $(OUTPUT)/main.o: main.c $(CC) $(HOST_CFLAGS) -c $< -o $@ @@ -45,8 +47,8 @@ $(OUTPUT)/call.o: call.S $(OUTPUT)/sign_key.o: sign_key.S $(CC) $(HOST_CFLAGS) -c $< -o $@ -$(OUTPUT)/test_encl.elf: test_encl.lds test_encl.c test_encl_bootstrap.S - $(CC) $(ENCL_CFLAGS) -T $^ -o $@ -Wl,--build-id=none +$(OUTPUT)/test_encl.elf: test_encl.c test_encl_bootstrap.S + $(CC) $(ENCL_CFLAGS) $^ -o $@ $(ENCL_LDFLAGS) EXTRA_CLEAN := \ $(OUTPUT)/test_encl.elf \ -- cgit v1.2.3 From 304b259e63b94c4f825e0648b33b15f8962955c7 Mon Sep 17 00:00:00 2001 From: Jo Van Bulck Date: Thu, 5 Oct 2023 17:38:46 +0200 Subject: selftests/sgx: Specify freestanding environment for enclave compilation Use -ffreestanding to assert the enclave compilation targets a freestanding environment (i.e., without "main" or standard libraries). This fixes clang reporting "undefined reference to `memset'" after erroneously optimizing away the provided memset/memcpy implementations. Still need to instruct the linker from using standard system startup functions, but drop -nostartfiles as it is implied by -nostdlib. Signed-off-by: Jo Van Bulck Signed-off-by: Dave Hansen Reviewed-by: Jarkko Sakkinen Acked-by: Kai Huang Link: https://lore.kernel.org/all/20231005153854.25566-6-jo.vanbulck%40cs.kuleuven.be --- tools/testing/selftests/sgx/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/testing/selftests/sgx/Makefile b/tools/testing/selftests/sgx/Makefile index dcdd04b322f8..7eb890bdd3f0 100644 --- a/tools/testing/selftests/sgx/Makefile +++ b/tools/testing/selftests/sgx/Makefile @@ -14,7 +14,7 @@ endif INCLUDES := -I$(top_srcdir)/tools/include HOST_CFLAGS := -Wall -Werror -g $(INCLUDES) -fPIC HOST_LDFLAGS := -z noexecstack -lcrypto -ENCL_CFLAGS += -Wall -Werror -static -nostdlib -nostartfiles -fPIC \ +ENCL_CFLAGS += -Wall -Werror -static -nostdlib -ffreestanding -fPIC \ -fno-stack-protector -mrdrnd $(INCLUDES) ENCL_LDFLAGS := -Wl,-T,test_encl.lds,--build-id=none -- cgit v1.2.3 From 4f812df8f37463767c2a74c2bd77de94acdb2be6 Mon Sep 17 00:00:00 2001 From: Jo Van Bulck Date: Thu, 5 Oct 2023 17:38:47 +0200 Subject: selftests/sgx: Remove redundant enclave base address save/restore Remove redundant push/pop pair that stores and restores the enclave base address in the test enclave, as it is never used after the pop and can anyway be easily retrieved via the __encl_base symbol. Signed-off-by: Jo Van Bulck Signed-off-by: Dave Hansen Reviewed-by: Jarkko Sakkinen Acked-by: Kai Huang Link: https://lore.kernel.org/all/20231005153854.25566-7-jo.vanbulck%40cs.kuleuven.be --- tools/testing/selftests/sgx/test_encl_bootstrap.S | 3 --- 1 file changed, 3 deletions(-) (limited to 'tools') diff --git a/tools/testing/selftests/sgx/test_encl_bootstrap.S b/tools/testing/selftests/sgx/test_encl_bootstrap.S index 03ae0f57e29d..e0ce993d3f2c 100644 --- a/tools/testing/selftests/sgx/test_encl_bootstrap.S +++ b/tools/testing/selftests/sgx/test_encl_bootstrap.S @@ -55,12 +55,9 @@ encl_entry_core: push %rax push %rcx # push the address after EENTER - push %rbx # push the enclave base address call encl_body - pop %rbx # pop the enclave base address - /* Clear volatile GPRs, except RAX (EEXIT function). */ xor %rcx, %rcx xor %rdx, %rdx -- cgit v1.2.3 From f7884e732841450de36c2b1ed49b91e8e854a7d1 Mon Sep 17 00:00:00 2001 From: Jo Van Bulck Date: Thu, 5 Oct 2023 17:38:48 +0200 Subject: selftests/sgx: Produce static-pie executable for test enclave The current combination of -static and -fPIC creates a static executable with position-dependent addresses for global variables. Use -static-pie and -fPIE to create a proper static position independent executable that can be loaded at any address without a dynamic linker. When building the original "lea (encl_stack)(%rbx), %rax" assembly code with -static-pie -fPIE, the linker complains about a relocation it cannot resolve: /usr/local/bin/ld: /tmp/cchIWyfG.o: relocation R_X86_64_32S against `.data' can not be used when making a PIE object; recompile with -fPIE collect2: error: ld returned 1 exit status Thus, since only RIP-relative addressing is legit for local symbols, use "encl_stack(%rip)" and declare an explicit "__encl_base" symbol at the start of the linker script to be able to calculate the stack address relative to the current TCS in the enclave assembly entry code. Signed-off-by: Jo Van Bulck Signed-off-by: Dave Hansen Reviewed-by: Jarkko Sakkinen Acked-by: Kai Huang Link: https://lore.kernel.org/all/f9c24d89-ed72-7d9e-c650-050d722c6b04@cs.kuleuven.be/ Link: https://lore.kernel.org/all/20231005153854.25566-8-jo.vanbulck%40cs.kuleuven.be --- tools/testing/selftests/sgx/Makefile | 2 +- tools/testing/selftests/sgx/test_encl.lds | 1 + tools/testing/selftests/sgx/test_encl_bootstrap.S | 9 ++++++--- 3 files changed, 8 insertions(+), 4 deletions(-) (limited to 'tools') diff --git a/tools/testing/selftests/sgx/Makefile b/tools/testing/selftests/sgx/Makefile index 7eb890bdd3f0..8d2ba6adc92b 100644 --- a/tools/testing/selftests/sgx/Makefile +++ b/tools/testing/selftests/sgx/Makefile @@ -14,7 +14,7 @@ endif INCLUDES := -I$(top_srcdir)/tools/include HOST_CFLAGS := -Wall -Werror -g $(INCLUDES) -fPIC HOST_LDFLAGS := -z noexecstack -lcrypto -ENCL_CFLAGS += -Wall -Werror -static -nostdlib -ffreestanding -fPIC \ +ENCL_CFLAGS += -Wall -Werror -static-pie -nostdlib -ffreestanding -fPIE \ -fno-stack-protector -mrdrnd $(INCLUDES) ENCL_LDFLAGS := -Wl,-T,test_encl.lds,--build-id=none diff --git a/tools/testing/selftests/sgx/test_encl.lds b/tools/testing/selftests/sgx/test_encl.lds index a1ec64f7d91f..62d37160f59b 100644 --- a/tools/testing/selftests/sgx/test_encl.lds +++ b/tools/testing/selftests/sgx/test_encl.lds @@ -10,6 +10,7 @@ PHDRS SECTIONS { . = 0; + __encl_base = .; .tcs : { *(.tcs*) } : tcs diff --git a/tools/testing/selftests/sgx/test_encl_bootstrap.S b/tools/testing/selftests/sgx/test_encl_bootstrap.S index e0ce993d3f2c..28fe5d2ac0af 100644 --- a/tools/testing/selftests/sgx/test_encl_bootstrap.S +++ b/tools/testing/selftests/sgx/test_encl_bootstrap.S @@ -42,9 +42,12 @@ encl_entry: # RBX contains the base address for TCS, which is the first address # inside the enclave for TCS #1 and one page into the enclave for - # TCS #2. By adding the value of encl_stack to it, we get - # the absolute address for the stack. - lea (encl_stack)(%rbx), %rax + # TCS #2. First make it relative by substracting __encl_base and + # then add the address of encl_stack to get the address for the stack. + lea __encl_base(%rip), %rax + sub %rax, %rbx + lea encl_stack(%rip), %rax + add %rbx, %rax jmp encl_entry_core encl_dyn_entry: # Entry point for dynamically created TCS page expected to follow -- cgit v1.2.3 From d06978e8e47a348c0d33462a8c2bf8f46d2b7df5 Mon Sep 17 00:00:00 2001 From: Jo Van Bulck Date: Thu, 5 Oct 2023 17:38:49 +0200 Subject: selftests/sgx: Handle relocations in test enclave Static-pie binaries normally include a startup routine to perform any ELF relocations from .rela.dyn. Since the enclave loading process is different and glibc is not included, do the necessary relocation for encl_op_array entries manually at runtime relative to the enclave base to ensure correct function pointers. When keeping encl_op_array as a local variable on the stack, gcc without optimizations generates code that explicitly gets the right function addresses and stores them to create the array on the stack: encl_body: /* snipped */ lea do_encl_op_put_to_buf(%rip), %rax mov %rax, -0x50(%rbp) lea do_encl_op_get_from_buf(%rip), %rax mov %rax,-0x48(%rbp) lea do_encl_op_put_to_addr(%rip), %rax /* snipped */ However, gcc -Os or clang generate more efficient code that initializes encl_op_array by copying a "prepared copy" containing the absolute addresses of the functions (i.e., relative to the image base starting from 0) generated by the compiler/linker: encl_body: /* snipped */ lea prepared_copy(%rip), %rsi lea -0x48(%rsp), %rdi mov $0x10,%ecx rep movsl %ds:(%rsi),%es:(%rdi) /* snipped */ When building the enclave with -static-pie, the compiler/linker includes relocation entries for the function symbols in the "prepared copy": Relocation section '.rela.dyn' at offset 0x4000 contains 12 entries: Offset Info Type Symbol /* snipped; "prepared_copy" starts at 0x6000 */ 000000006000 000000000008 R_X86_64_RELATIVE 000000006008 000000000008 R_X86_64_RELATIVE 000000006010 000000000008 R_X86_64_RELATIVE 000000006018 000000000008 R_X86_64_RELATIVE 000000006020 000000000008 R_X86_64_RELATIVE 000000006028 000000000008 R_X86_64_RELATIVE 000000006030 000000000008 R_X86_64_RELATIVE 000000006038 000000000008 R_X86_64_RELATIVE Static-pie binaries normally include a glibc "_dl_relocate_static_pie" routine that will perform these relocations as part of the startup. However, since the enclave loading process is different and glibc is not included, we cannot rely on these relocations to be performed. Without relocations, the code would erroneously jump to the _absolute_ function address loaded from the local copy. Thus, declare "encl_op_array" as global and manually relocate the loaded function-pointer entries relative to the enclave base at runtime. This generates the following code: encl_body: /* snipped */ lea encl_op_array(%rip), %rcx lea __encl_base(%rip), %rax add (%rcx,%rdx,8),%rax jmp *%rax Signed-off-by: Jo Van Bulck Signed-off-by: Dave Hansen Reviewed-by: Jarkko Sakkinen Acked-by: Kai Huang Link: https://lore.kernel.org/all/150d8ca8-2c66-60d1-f9fc-8e6279824e94@cs.kuleuven.be/ Link: https://lore.kernel.org/all/5c22de5a-4b3b-1f38-9771-409b4ec7f96d@cs.kuleuven.be/#r Link: https://lore.kernel.org/all/20231005153854.25566-9-jo.vanbulck%40cs.kuleuven.be --- tools/testing/selftests/sgx/test_encl.c | 50 +++++++++++++++++++++++---------- 1 file changed, 35 insertions(+), 15 deletions(-) (limited to 'tools') diff --git a/tools/testing/selftests/sgx/test_encl.c b/tools/testing/selftests/sgx/test_encl.c index ae791df3e5a5..649604c526e7 100644 --- a/tools/testing/selftests/sgx/test_encl.c +++ b/tools/testing/selftests/sgx/test_encl.c @@ -121,21 +121,41 @@ static void do_encl_op_nop(void *_op) } +/* + * Symbol placed at the start of the enclave image by the linker script. + * Declare this extern symbol with visibility "hidden" to ensure the compiler + * does not access it through the GOT and generates position-independent + * addressing as __encl_base(%rip), so we can get the actual enclave base + * during runtime. + */ +extern const uint8_t __attribute__((visibility("hidden"))) __encl_base; + +typedef void (*encl_op_t)(void *); +static const encl_op_t encl_op_array[ENCL_OP_MAX] = { + do_encl_op_put_to_buf, + do_encl_op_get_from_buf, + do_encl_op_put_to_addr, + do_encl_op_get_from_addr, + do_encl_op_nop, + do_encl_eaccept, + do_encl_emodpe, + do_encl_init_tcs_page, +}; + void encl_body(void *rdi, void *rsi) { - const void (*encl_op_array[ENCL_OP_MAX])(void *) = { - do_encl_op_put_to_buf, - do_encl_op_get_from_buf, - do_encl_op_put_to_addr, - do_encl_op_get_from_addr, - do_encl_op_nop, - do_encl_eaccept, - do_encl_emodpe, - do_encl_init_tcs_page, - }; - - struct encl_op_header *op = (struct encl_op_header *)rdi; - - if (op->type < ENCL_OP_MAX) - (*encl_op_array[op->type])(op); + struct encl_op_header *header = (struct encl_op_header *)rdi; + encl_op_t op; + + if (header->type >= ENCL_OP_MAX) + return; + + /* + * The enclave base address needs to be added, as this call site + * *cannot be* made rip-relative by the compiler, or fixed up by + * any other possible means. + */ + op = ((uint64_t)&__encl_base) + encl_op_array[header->type]; + + (*op)(header); } -- cgit v1.2.3 From 9fd552ee32c6c1e27c125016b87d295bea6faea7 Mon Sep 17 00:00:00 2001 From: Jo Van Bulck Date: Thu, 5 Oct 2023 17:38:50 +0200 Subject: selftests/sgx: Fix linker script asserts DEFINED only considers symbols, not section names. Hence, replace the check for .got.plt with the _GLOBAL_OFFSET_TABLE_ symbol and remove other (non-essential) asserts. Signed-off-by: Jo Van Bulck Signed-off-by: Dave Hansen Reviewed-by: Jarkko Sakkinen Link: https://lore.kernel.org/all/20231005153854.25566-10-jo.vanbulck%40cs.kuleuven.be --- tools/testing/selftests/sgx/test_encl.lds | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'tools') diff --git a/tools/testing/selftests/sgx/test_encl.lds b/tools/testing/selftests/sgx/test_encl.lds index 62d37160f59b..6ffdfc9fb4cf 100644 --- a/tools/testing/selftests/sgx/test_encl.lds +++ b/tools/testing/selftests/sgx/test_encl.lds @@ -35,8 +35,4 @@ SECTIONS } } -ASSERT(!DEFINED(.altinstructions), "ALTERNATIVES are not supported in enclaves") -ASSERT(!DEFINED(.altinstr_replacement), "ALTERNATIVES are not supported in enclaves") -ASSERT(!DEFINED(.discard.retpoline_safe), "RETPOLINE ALTERNATIVES are not supported in enclaves") -ASSERT(!DEFINED(.discard.nospec), "RETPOLINE ALTERNATIVES are not supported in enclaves") -ASSERT(!DEFINED(.got.plt), "Libcalls are not supported in enclaves") +ASSERT(!DEFINED(_GLOBAL_OFFSET_TABLE_), "Libcalls through GOT are not supported in enclaves") -- cgit v1.2.3 From a4c39ef4ed43103caef80029cd30427a9ff342d8 Mon Sep 17 00:00:00 2001 From: Jo Van Bulck Date: Thu, 5 Oct 2023 17:38:51 +0200 Subject: selftests/sgx: Ensure test enclave buffer is entirely preserved Attach the "used" attribute to instruct the compiler to preserve the static encl_buffer, even if it appears it is not entirely referenced in the enclave code, as expected by the external tests manipulating page permissions. Signed-off-by: Jo Van Bulck Signed-off-by: Dave Hansen Reviewed-by: Jarkko Sakkinen Acked-by: Kai Huang Link: https://lore.kernel.org/all/a2732938-f3db-a0af-3d68-a18060f66e79@cs.kuleuven.be/ Link: https://lore.kernel.org/all/20231005153854.25566-11-jo.vanbulck%40cs.kuleuven.be --- tools/testing/selftests/sgx/defines.h | 1 + tools/testing/selftests/sgx/test_encl.c | 9 +++++---- 2 files changed, 6 insertions(+), 4 deletions(-) (limited to 'tools') diff --git a/tools/testing/selftests/sgx/defines.h b/tools/testing/selftests/sgx/defines.h index d8587c971941..b8f482667ce1 100644 --- a/tools/testing/selftests/sgx/defines.h +++ b/tools/testing/selftests/sgx/defines.h @@ -13,6 +13,7 @@ #define __aligned(x) __attribute__((__aligned__(x))) #define __packed __attribute__((packed)) +#define __used __attribute__((used)) #include "../../../../arch/x86/include/asm/sgx.h" #include "../../../../arch/x86/include/asm/enclu.h" diff --git a/tools/testing/selftests/sgx/test_encl.c b/tools/testing/selftests/sgx/test_encl.c index 649604c526e7..7465f121fb74 100644 --- a/tools/testing/selftests/sgx/test_encl.c +++ b/tools/testing/selftests/sgx/test_encl.c @@ -5,11 +5,12 @@ #include "defines.h" /* - * Data buffer spanning two pages that will be placed first in .data - * segment. Even if not used internally the second page is needed by - * external test manipulating page permissions. + * Data buffer spanning two pages that will be placed first in the .data + * segment. Even if not used internally the second page is needed by external + * test manipulating page permissions, so mark encl_buffer as "used" to make + * sure it is entirely preserved by the compiler. */ -static uint8_t encl_buffer[8192] = { 1 }; +static uint8_t __used encl_buffer[8192] = { 1 }; enum sgx_enclu_function { EACCEPT = 0x5, -- cgit v1.2.3 From 022416496008bd51cb7b33975cc0008749329a86 Mon Sep 17 00:00:00 2001 From: Jo Van Bulck Date: Thu, 5 Oct 2023 17:38:52 +0200 Subject: selftests/sgx: Ensure expected location of test enclave buffer The external tests manipulating page permissions expect encl_buffer to be placed at the start of the test enclave's .data section. As this is not guaranteed per the C standard, explicitly place encl_buffer in a separate section that is explicitly placed at the start of the .data segment in the linker script to avoid the compiler placing it somewhere else in .data. Signed-off-by: Jo Van Bulck Signed-off-by: Dave Hansen Reviewed-by: Jarkko Sakkinen Acked-by: Kai Huang Link: https://lore.kernel.org/all/20231005153854.25566-12-jo.vanbulck%40cs.kuleuven.be --- tools/testing/selftests/sgx/defines.h | 1 + tools/testing/selftests/sgx/test_encl.c | 8 ++++---- tools/testing/selftests/sgx/test_encl.lds | 1 + 3 files changed, 6 insertions(+), 4 deletions(-) (limited to 'tools') diff --git a/tools/testing/selftests/sgx/defines.h b/tools/testing/selftests/sgx/defines.h index b8f482667ce1..402f8787a71c 100644 --- a/tools/testing/selftests/sgx/defines.h +++ b/tools/testing/selftests/sgx/defines.h @@ -14,6 +14,7 @@ #define __aligned(x) __attribute__((__aligned__(x))) #define __packed __attribute__((packed)) #define __used __attribute__((used)) +#define __section(x)__attribute__((__section__(x))) #include "../../../../arch/x86/include/asm/sgx.h" #include "../../../../arch/x86/include/asm/enclu.h" diff --git a/tools/testing/selftests/sgx/test_encl.c b/tools/testing/selftests/sgx/test_encl.c index 7465f121fb74..2c4d709cce2d 100644 --- a/tools/testing/selftests/sgx/test_encl.c +++ b/tools/testing/selftests/sgx/test_encl.c @@ -6,11 +6,11 @@ /* * Data buffer spanning two pages that will be placed first in the .data - * segment. Even if not used internally the second page is needed by external - * test manipulating page permissions, so mark encl_buffer as "used" to make - * sure it is entirely preserved by the compiler. + * segment via the linker script. Even if not used internally the second page + * is needed by external test manipulating page permissions, so mark + * encl_buffer as "used" to make sure it is entirely preserved by the compiler. */ -static uint8_t __used encl_buffer[8192] = { 1 }; +static uint8_t __used __section(".data.encl_buffer") encl_buffer[8192] = { 1 }; enum sgx_enclu_function { EACCEPT = 0x5, diff --git a/tools/testing/selftests/sgx/test_encl.lds b/tools/testing/selftests/sgx/test_encl.lds index 6ffdfc9fb4cf..333a3e78fdc9 100644 --- a/tools/testing/selftests/sgx/test_encl.lds +++ b/tools/testing/selftests/sgx/test_encl.lds @@ -24,6 +24,7 @@ SECTIONS } : text .data : { + *(.data.encl_buffer) *(.data*) } : data -- cgit v1.2.3 From ec44ca1e34bc3a9864a8c1bf8636066ec6d5d2e5 Mon Sep 17 00:00:00 2001 From: Jo Van Bulck Date: Thu, 5 Oct 2023 17:38:53 +0200 Subject: selftests/sgx: Discard unsupported ELF sections Building the test enclave with -static-pie may produce a dynamic symbol table, but this is not supported for enclaves and any relocations need to happen manually (e.g., as for "encl_op_array"). Thus, opportunistically discard ".dyn*" and ".gnu.hash" which the enclave loader cannot handle. Signed-off-by: Jo Van Bulck Signed-off-by: Dave Hansen Reviewed-by: Jarkko Sakkinen Link: https://lore.kernel.org/all/20231005153854.25566-13-jo.vanbulck%40cs.kuleuven.be --- tools/testing/selftests/sgx/test_encl.lds | 2 ++ 1 file changed, 2 insertions(+) (limited to 'tools') diff --git a/tools/testing/selftests/sgx/test_encl.lds b/tools/testing/selftests/sgx/test_encl.lds index 333a3e78fdc9..ffe851a1cac4 100644 --- a/tools/testing/selftests/sgx/test_encl.lds +++ b/tools/testing/selftests/sgx/test_encl.lds @@ -33,6 +33,8 @@ SECTIONS *(.note*) *(.debug*) *(.eh_frame*) + *(.dyn*) + *(.gnu.hash) } } -- cgit v1.2.3 From 886c5be0b12e89b2905c26c4f24d50ae91f261da Mon Sep 17 00:00:00 2001 From: Jo Van Bulck Date: Thu, 5 Oct 2023 17:38:54 +0200 Subject: selftests/sgx: Remove incomplete ABI sanitization code in test enclave As the selftest enclave is *not* intended for production, simplify the code by not initializing CPU configuration registers as expected by the ABI on enclave entry or cleansing caller-save registers on enclave exit. Signed-off-by: Jo Van Bulck Signed-off-by: Dave Hansen Reviewed-by: Jarkko Sakkinen Link: https://lore.kernel.org/all/da0cfb1e-e347-f7f2-ac72-aec0ee0d867d@intel.com/ Link: https://lore.kernel.org/all/20231005153854.25566-14-jo.vanbulck%40cs.kuleuven.be --- tools/testing/selftests/sgx/test_encl_bootstrap.S | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) (limited to 'tools') diff --git a/tools/testing/selftests/sgx/test_encl_bootstrap.S b/tools/testing/selftests/sgx/test_encl_bootstrap.S index 28fe5d2ac0af..d8c4ac94e032 100644 --- a/tools/testing/selftests/sgx/test_encl_bootstrap.S +++ b/tools/testing/selftests/sgx/test_encl_bootstrap.S @@ -59,21 +59,11 @@ encl_entry_core: push %rcx # push the address after EENTER + # NOTE: as the selftest enclave is *not* intended for production, + # simplify the code by not initializing ABI registers on entry or + # cleansing caller-save registers on exit. call encl_body - /* Clear volatile GPRs, except RAX (EEXIT function). */ - xor %rcx, %rcx - xor %rdx, %rdx - xor %rdi, %rdi - xor %rsi, %rsi - xor %r8, %r8 - xor %r9, %r9 - xor %r10, %r10 - xor %r11, %r11 - - # Reset status flags. - add %rdx, %rdx # OF = SF = AF = CF = 0; ZF = PF = 1 - # Prepare EEXIT target by popping the address of the instruction after # EENTER to RBX. pop %rbx -- cgit v1.2.3 From 981cf568a8644161c2f15c02278ebc2834b51ba6 Mon Sep 17 00:00:00 2001 From: Zhao Mengmeng Date: Tue, 5 Dec 2023 21:56:05 -0500 Subject: selftests/sgx: Skip non X86_64 platform When building whole selftests on arm64, rsync gives an erorr about sgx: rsync: [sender] link_stat "/root/linux-next/tools/testing/selftests/sgx/test_encl.elf" failed: No such file or directory (2) rsync error: some files/attrs were not transferred (see previous errors) (code 23) at main.c(1327) [sender=3.2.5] The root casue is sgx only used on X86_64, and shall be skipped on other platforms. Fix this by moving TEST_CUSTOM_PROGS and TEST_FILES inside the if check, then the build result will be "Skipping non-existent dir: sgx". Fixes: 2adcba79e69d ("selftests/x86: Add a selftest for SGX") Signed-off-by: Zhao Mengmeng Signed-off-by: Dave Hansen Reviewed-by: Jarkko Sakkinen Link: https://lore.kernel.org/all/20231206025605.3965302-1-zhaomzhao%40126.com --- tools/testing/selftests/sgx/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/testing/selftests/sgx/Makefile b/tools/testing/selftests/sgx/Makefile index 8d2ba6adc92b..867f88ce2570 100644 --- a/tools/testing/selftests/sgx/Makefile +++ b/tools/testing/selftests/sgx/Makefile @@ -18,10 +18,10 @@ ENCL_CFLAGS += -Wall -Werror -static-pie -nostdlib -ffreestanding -fPIE \ -fno-stack-protector -mrdrnd $(INCLUDES) ENCL_LDFLAGS := -Wl,-T,test_encl.lds,--build-id=none +ifeq ($(CAN_BUILD_X86_64), 1) TEST_CUSTOM_PROGS := $(OUTPUT)/test_sgx TEST_FILES := $(OUTPUT)/test_encl.elf -ifeq ($(CAN_BUILD_X86_64), 1) all: $(TEST_CUSTOM_PROGS) $(OUTPUT)/test_encl.elf endif -- cgit v1.2.3