linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 00/13] selftests/sgx: Fix compilation errors
@ 2023-08-25 13:32 Jo Van Bulck
  2023-08-25 13:32 ` [PATCH v4 01/13] selftests/sgx: Fix uninitialized pointer dereference in error path Jo Van Bulck
                   ` (12 more replies)
  0 siblings, 13 replies; 25+ messages in thread
From: Jo Van Bulck @ 2023-08-25 13:32 UTC (permalink / raw)
  To: jarkko, kai.huang, linux-sgx, linux-kernel; +Cc: dave.hansen, Jo Van Bulck

Hi,

This is the fourth iteration of a patch series to ensure that all SGX selftests
succeed when compiling with optimizations (as tested with -O{0,1,2,3,s} for
both gcc 11.3.0 and clang 14.0.0). The aim of the patches is to avoid reliance
on undefined, compiler-specific behavior that can make the test results
fragile.

This series appends 4 new commits compared to the previous series (do let me
know if appending commits to this series is not the preferred way to handle
these?):

1. Another fix for possibly uninitialized pointer dereferences in
   encl_get_entry. Note that this was only brought up as a compiler warning
   when compiling the loader itself (i.e., not only the enclave) with
   optimizations. Also not that the uninitialized pointers shouldn't show up
   for "well-formed" enclave ELF files that properly contain a symbol table
   section, but I considered it is nevertheless good practice to harden the
   code against this (unlikely) case.
2. Split off the ".dyn*" and ".gnu.hash" discarding in a separate commit, as
   suggested by Kay [1].
3. Split off the removal of the redundant push/pop pair in a separate commit,
   as suggested by Kai [2].
4. Remove all (incomplete) CPU register cleansing assembly code on enclave
   exit, reflecting earlier discussions that highlight that the test enclave
   should *not* be confused with exemplary, security-hardened enclave code, and
   in line with Dave's suggestion to make the test enclave more *obviously*
   insecure [3].

If useful, I can also include an elementary wrapper shell script to compile and
run the tests for different compilers (gcc/clang) and optimization levels.
Reference output below:

.. Testing gcc   -O0    [OK]
.. Testing gcc   -O1    [OK]
.. Testing gcc   -O2    [OK]
.. Testing gcc   -O3    [OK]
.. Testing gcc   -Os    [OK]
.. Testing gcc   -Ofast [OK]
.. Testing gcc   -Og    [OK]
.. Testing clang -O0    [OK]
.. Testing clang -O1    [OK]
.. Testing clang -O2    [OK]
.. Testing clang -O3    [OK]
.. Testing clang -Os    [OK]
.. Testing clang -Ofast [OK]
.. Testing clang -Og    [OK]

Changelog
---------

v4
  - Remove redundant -nostartfiles compiler flag (Jarkko)
  - Split dynamic symbol table removal in separate commit (Kai)
  - Split redundant push/pop elimination in separate commit (Kai)
  - Remove (incomplete) register cleansing on enclave exit
  - Fix possibly uninitialized pointer dereferences in load.c

v3
  - Refactor encl_op_array declaration and indexing (Jarkko)
  - Annotate encl_buffer with "used" attribute (Kai)
  - Split encl_buffer size and placement commits (Kai)

v2
  - Add additional check for NULL pointer (Kai)
  - Refine to produce proper static-pie executable
  - Fix linker script assertions
  - Specify memory clobber for inline asm instead of volatile (Kai)
  - Clarify why encl_buffer non-static (Jarkko, Kai)
  - Clarify -ffreestanding (Jarkko)

References
----------

[1] https://lore.kernel.org/all/90ad8638bc1c26505e33b3f436fdbc22c8d74ba9.camel@intel.com/
[2] https://lore.kernel.org/all/71ad6389da7db8541dada0276db33f98e2a4fdcf.camel@intel.com/
[3] https://lore.kernel.org/all/da0cfb1e-e347-f7f2-ac72-aec0ee0d867d@intel.com/

Best,
Jo


Jo Van Bulck (13):
  selftests/sgx: Fix uninitialized pointer dereference in error path
  selftests/sgx: Produce static-pie executable for test enclave
  selftests/sgx: Handle relocations in test enclave
  selftests/sgx: Fix linker script asserts
  selftests/sgx: Include memory clobber for inline asm in test enclave
  selftests/sgx: Ensure test enclave buffer is entirely preserved
  selftests/sgx: Ensure expected location of test enclave buffer
  selftests/sgx: Separate linker options
  selftests/sgx: Specify freestanding environment for enclave
    compilation
  selftests/sgx: Fix uninitialized pointer dereferences
  selftests/sgx: Discard unsupported ELF sections
  selftests/sgx: Remove redundant enclave base address save/restore
  selftests/sgx: Remove incomplete ABI sanitization code in test enclave

 tools/testing/selftests/sgx/Makefile          | 14 ++--
 tools/testing/selftests/sgx/defines.h         |  2 +
 tools/testing/selftests/sgx/load.c            |  9 ++-
 tools/testing/selftests/sgx/sigstruct.c       |  5 +-
 tools/testing/selftests/sgx/test_encl.c       | 65 ++++++++++++-------
 tools/testing/selftests/sgx/test_encl.lds     | 10 +--
 .../selftests/sgx/test_encl_bootstrap.S       | 28 +++-----
 7 files changed, 76 insertions(+), 57 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v4 01/13] selftests/sgx: Fix uninitialized pointer dereference in error path
  2023-08-25 13:32 [PATCH v4 00/13] selftests/sgx: Fix compilation errors Jo Van Bulck
@ 2023-08-25 13:32 ` Jo Van Bulck
  2023-08-25 13:32 ` [PATCH v4 02/13] selftests/sgx: Produce static-pie executable for test enclave Jo Van Bulck
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Jo Van Bulck @ 2023-08-25 13:32 UTC (permalink / raw)
  To: jarkko, kai.huang, linux-sgx, linux-kernel; +Cc: dave.hansen, Jo Van Bulck

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 <jo.vanbulck@cs.kuleuven.be>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Acked-by: Kai Huang <kai.huang@intel.com>
---
 tools/testing/selftests/sgx/sigstruct.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

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;
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v4 02/13] selftests/sgx: Produce static-pie executable for test enclave
  2023-08-25 13:32 [PATCH v4 00/13] selftests/sgx: Fix compilation errors Jo Van Bulck
  2023-08-25 13:32 ` [PATCH v4 01/13] selftests/sgx: Fix uninitialized pointer dereference in error path Jo Van Bulck
@ 2023-08-25 13:32 ` Jo Van Bulck
  2023-08-29 10:55   ` Huang, Kai
  2023-08-25 13:32 ` [PATCH v4 03/13] selftests/sgx: Handle relocations in " Jo Van Bulck
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 25+ messages in thread
From: Jo Van Bulck @ 2023-08-25 13:32 UTC (permalink / raw)
  To: jarkko, kai.huang, linux-sgx, linux-kernel; +Cc: dave.hansen, Jo Van Bulck

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.

Link: https://lore.kernel.org/all/f9c24d89-ed72-7d9e-c650-050d722c6b04@cs.kuleuven.be/
Signed-off-by: Jo Van Bulck <jo.vanbulck@cs.kuleuven.be>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 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(-)

diff --git a/tools/testing/selftests/sgx/Makefile b/tools/testing/selftests/sgx/Makefile
index 50aab6b57da3..1d6315a2e5f5 100644
--- a/tools/testing/selftests/sgx/Makefile
+++ b/tools/testing/selftests/sgx/Makefile
@@ -13,7 +13,7 @@ endif
 
 INCLUDES := -I$(top_srcdir)/tools/include
 HOST_CFLAGS := -Wall -Werror -g $(INCLUDES) -fPIC -z noexecstack
-ENCL_CFLAGS := -Wall -Werror -static -nostdlib -nostartfiles -fPIC \
+ENCL_CFLAGS := -Wall -Werror -static-pie -nostdlib -nostartfiles -fPIE \
 	       -fno-stack-protector -mrdrnd $(INCLUDES)
 
 TEST_CUSTOM_PROGS := $(OUTPUT)/test_sgx
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 03ae0f57e29d..4e55d91566c4 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
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v4 03/13] selftests/sgx: Handle relocations in test enclave
  2023-08-25 13:32 [PATCH v4 00/13] selftests/sgx: Fix compilation errors Jo Van Bulck
  2023-08-25 13:32 ` [PATCH v4 01/13] selftests/sgx: Fix uninitialized pointer dereference in error path Jo Van Bulck
  2023-08-25 13:32 ` [PATCH v4 02/13] selftests/sgx: Produce static-pie executable for test enclave Jo Van Bulck
@ 2023-08-25 13:32 ` Jo Van Bulck
  2023-08-28 13:15   ` Huang, Kai
  2023-08-25 13:32 ` [PATCH v4 04/13] selftests/sgx: Fix linker script asserts Jo Van Bulck
                   ` (9 subsequent siblings)
  12 siblings, 1 reply; 25+ messages in thread
From: Jo Van Bulck @ 2023-08-25 13:32 UTC (permalink / raw)
  To: jarkko, kai.huang, linux-sgx, linux-kernel; +Cc: dave.hansen, Jo Van Bulck

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.

Signed-off-by: Jo Van Bulck <jo.vanbulck@cs.kuleuven.be>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 tools/testing/selftests/sgx/test_encl.c | 48 +++++++++++++++++--------
 1 file changed, 33 insertions(+), 15 deletions(-)

diff --git a/tools/testing/selftests/sgx/test_encl.c b/tools/testing/selftests/sgx/test_encl.c
index c0d6397295e3..706c1f7260ff 100644
--- a/tools/testing/selftests/sgx/test_encl.c
+++ b/tools/testing/selftests/sgx/test_encl.c
@@ -119,21 +119,39 @@ 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.
+ */
+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);
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v4 04/13] selftests/sgx: Fix linker script asserts
  2023-08-25 13:32 [PATCH v4 00/13] selftests/sgx: Fix compilation errors Jo Van Bulck
                   ` (2 preceding siblings ...)
  2023-08-25 13:32 ` [PATCH v4 03/13] selftests/sgx: Handle relocations in " Jo Van Bulck
@ 2023-08-25 13:32 ` Jo Van Bulck
  2023-08-25 13:32 ` [PATCH v4 05/13] selftests/sgx: Include memory clobber for inline asm in test enclave Jo Van Bulck
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Jo Van Bulck @ 2023-08-25 13:32 UTC (permalink / raw)
  To: jarkko, kai.huang, linux-sgx, linux-kernel; +Cc: dave.hansen, Jo Van Bulck

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 <jo.vanbulck@cs.kuleuven.be>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 tools/testing/selftests/sgx/test_encl.lds | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

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")
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v4 05/13] selftests/sgx: Include memory clobber for inline asm in test enclave
  2023-08-25 13:32 [PATCH v4 00/13] selftests/sgx: Fix compilation errors Jo Van Bulck
                   ` (3 preceding siblings ...)
  2023-08-25 13:32 ` [PATCH v4 04/13] selftests/sgx: Fix linker script asserts Jo Van Bulck
@ 2023-08-25 13:32 ` Jo Van Bulck
  2023-08-29 11:07   ` Huang, Kai
  2023-08-25 13:32 ` [PATCH v4 06/13] selftests/sgx: Ensure test enclave buffer is entirely preserved Jo Van Bulck
                   ` (7 subsequent siblings)
  12 siblings, 1 reply; 25+ messages in thread
From: Jo Van Bulck @ 2023-08-25 13:32 UTC (permalink / raw)
  To: jarkko, kai.huang, linux-sgx, linux-kernel; +Cc: dave.hansen, Jo Van Bulck

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.

Signed-off-by: Jo Van Bulck <jo.vanbulck@cs.kuleuven.be>
Reviewed-by: Kai Huang <kai.huang@intel.com>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 tools/testing/selftests/sgx/test_encl.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/sgx/test_encl.c b/tools/testing/selftests/sgx/test_encl.c
index 706c1f7260ff..dfe531c5f560 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;
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v4 06/13] selftests/sgx: Ensure test enclave buffer is entirely preserved
  2023-08-25 13:32 [PATCH v4 00/13] selftests/sgx: Fix compilation errors Jo Van Bulck
                   ` (4 preceding siblings ...)
  2023-08-25 13:32 ` [PATCH v4 05/13] selftests/sgx: Include memory clobber for inline asm in test enclave Jo Van Bulck
@ 2023-08-25 13:32 ` Jo Van Bulck
  2023-08-25 13:32 ` [PATCH v4 07/13] selftests/sgx: Ensure expected location of test enclave buffer Jo Van Bulck
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Jo Van Bulck @ 2023-08-25 13:32 UTC (permalink / raw)
  To: jarkko, kai.huang, linux-sgx, linux-kernel; +Cc: dave.hansen, Jo Van Bulck

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.

Link: https://lore.kernel.org/all/a2732938-f3db-a0af-3d68-a18060f66e79@cs.kuleuven.be/
Signed-off-by: Jo Van Bulck <jo.vanbulck@cs.kuleuven.be>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Acked-by: Kai Huang <kai.huang@intel.com>
---
 tools/testing/selftests/sgx/defines.h   | 1 +
 tools/testing/selftests/sgx/test_encl.c | 9 +++++----
 2 files changed, 6 insertions(+), 4 deletions(-)

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 dfe531c5f560..81070492230f 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,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v4 07/13] selftests/sgx: Ensure expected location of test enclave buffer
  2023-08-25 13:32 [PATCH v4 00/13] selftests/sgx: Fix compilation errors Jo Van Bulck
                   ` (5 preceding siblings ...)
  2023-08-25 13:32 ` [PATCH v4 06/13] selftests/sgx: Ensure test enclave buffer is entirely preserved Jo Van Bulck
@ 2023-08-25 13:32 ` Jo Van Bulck
  2023-08-25 13:32 ` [PATCH v4 08/13] selftests/sgx: Separate linker options Jo Van Bulck
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Jo Van Bulck @ 2023-08-25 13:32 UTC (permalink / raw)
  To: jarkko, kai.huang, linux-sgx, linux-kernel; +Cc: dave.hansen, Jo Van Bulck

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 <jo.vanbulck@cs.kuleuven.be>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Acked-by: Kai Huang <kai.huang@intel.com>
---
 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(-)

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 81070492230f..2ee597b9d14b 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
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v4 08/13] selftests/sgx: Separate linker options
  2023-08-25 13:32 [PATCH v4 00/13] selftests/sgx: Fix compilation errors Jo Van Bulck
                   ` (6 preceding siblings ...)
  2023-08-25 13:32 ` [PATCH v4 07/13] selftests/sgx: Ensure expected location of test enclave buffer Jo Van Bulck
@ 2023-08-25 13:32 ` Jo Van Bulck
  2023-08-25 13:32 ` [PATCH v4 09/13] selftests/sgx: Specify freestanding environment for enclave compilation Jo Van Bulck
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Jo Van Bulck @ 2023-08-25 13:32 UTC (permalink / raw)
  To: jarkko, kai.huang, linux-sgx, linux-kernel; +Cc: dave.hansen, Jo Van Bulck

Fixes "'linker' input unused [-Wunused-command-line-argument]" errors when
compiling with clang.

Signed-off-by: Jo Van Bulck <jo.vanbulck@cs.kuleuven.be>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 tools/testing/selftests/sgx/Makefile | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/sgx/Makefile b/tools/testing/selftests/sgx/Makefile
index 1d6315a2e5f5..2de970f7237c 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
+HOST_CFLAGS := -Wall -Werror -g $(INCLUDES) -fPIC
+HOST_LDFLAGS := -z noexecstack -lcrypto
 ENCL_CFLAGS := -Wall -Werror -static-pie -nostdlib -nostartfiles -fPIE \
 	       -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 \
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v4 09/13] selftests/sgx: Specify freestanding environment for enclave compilation
  2023-08-25 13:32 [PATCH v4 00/13] selftests/sgx: Fix compilation errors Jo Van Bulck
                   ` (7 preceding siblings ...)
  2023-08-25 13:32 ` [PATCH v4 08/13] selftests/sgx: Separate linker options Jo Van Bulck
@ 2023-08-25 13:32 ` Jo Van Bulck
  2023-08-27 18:35   ` Jarkko Sakkinen
  2023-08-25 13:32 ` [PATCH v4 10/13] selftests/sgx: Fix uninitialized pointer dereferences Jo Van Bulck
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 25+ messages in thread
From: Jo Van Bulck @ 2023-08-25 13:32 UTC (permalink / raw)
  To: jarkko, kai.huang, linux-sgx, linux-kernel; +Cc: dave.hansen, Jo Van Bulck

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 <jo.vanbulck@cs.kuleuven.be>
---
 tools/testing/selftests/sgx/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/sgx/Makefile b/tools/testing/selftests/sgx/Makefile
index 2de970f7237c..f96667bdf9f2 100644
--- a/tools/testing/selftests/sgx/Makefile
+++ b/tools/testing/selftests/sgx/Makefile
@@ -14,8 +14,8 @@ endif
 INCLUDES := -I$(top_srcdir)/tools/include
 HOST_CFLAGS := -Wall -Werror -g $(INCLUDES) -fPIC
 HOST_LDFLAGS := -z noexecstack -lcrypto
-ENCL_CFLAGS := -Wall -Werror -static-pie -nostdlib -nostartfiles -fPIE \
-	       -fno-stack-protector -mrdrnd $(INCLUDES)
+ENCL_CFLAGS := -Wall -Werror -static-pie -nostdlib -ffreestanding -fPIE \
+		-fno-stack-protector -mrdrnd $(INCLUDES)
 ENCL_LDFLAGS := -Wl,-T,test_encl.lds,--build-id=none
 
 TEST_CUSTOM_PROGS := $(OUTPUT)/test_sgx
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v4 10/13] selftests/sgx: Fix uninitialized pointer dereferences
  2023-08-25 13:32 [PATCH v4 00/13] selftests/sgx: Fix compilation errors Jo Van Bulck
                   ` (8 preceding siblings ...)
  2023-08-25 13:32 ` [PATCH v4 09/13] selftests/sgx: Specify freestanding environment for enclave compilation Jo Van Bulck
@ 2023-08-25 13:32 ` Jo Van Bulck
  2023-08-27 18:36   ` Jarkko Sakkinen
  2023-08-25 13:32 ` [PATCH v4 11/13] selftests/sgx: Discard unsupported ELF sections Jo Van Bulck
                   ` (2 subsequent siblings)
  12 siblings, 1 reply; 25+ messages in thread
From: Jo Van Bulck @ 2023-08-25 13:32 UTC (permalink / raw)
  To: jarkko, kai.huang, linux-sgx, linux-kernel; +Cc: dave.hansen, Jo Van Bulck

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 <jo.vanbulck@cs.kuleuven.be>
---
 tools/testing/selftests/sgx/load.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

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];
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v4 11/13] selftests/sgx: Discard unsupported ELF sections
  2023-08-25 13:32 [PATCH v4 00/13] selftests/sgx: Fix compilation errors Jo Van Bulck
                   ` (9 preceding siblings ...)
  2023-08-25 13:32 ` [PATCH v4 10/13] selftests/sgx: Fix uninitialized pointer dereferences Jo Van Bulck
@ 2023-08-25 13:32 ` Jo Van Bulck
  2023-08-25 13:32 ` [PATCH v4 12/13] selftests/sgx: Remove redundant enclave base address save/restore Jo Van Bulck
  2023-08-25 13:32 ` [PATCH v4 13/13] selftests/sgx: Remove incomplete ABI sanitization code in test enclave Jo Van Bulck
  12 siblings, 0 replies; 25+ messages in thread
From: Jo Van Bulck @ 2023-08-25 13:32 UTC (permalink / raw)
  To: jarkko, kai.huang, linux-sgx, linux-kernel; +Cc: dave.hansen, Jo Van Bulck

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 <jo.vanbulck@cs.kuleuven.be>
---
 tools/testing/selftests/sgx/test_encl.lds | 2 ++
 1 file changed, 2 insertions(+)

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)
 	}
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v4 12/13] selftests/sgx: Remove redundant enclave base address save/restore
  2023-08-25 13:32 [PATCH v4 00/13] selftests/sgx: Fix compilation errors Jo Van Bulck
                   ` (10 preceding siblings ...)
  2023-08-25 13:32 ` [PATCH v4 11/13] selftests/sgx: Discard unsupported ELF sections Jo Van Bulck
@ 2023-08-25 13:32 ` Jo Van Bulck
  2023-08-29 11:00   ` Huang, Kai
  2023-08-25 13:32 ` [PATCH v4 13/13] selftests/sgx: Remove incomplete ABI sanitization code in test enclave Jo Van Bulck
  12 siblings, 1 reply; 25+ messages in thread
From: Jo Van Bulck @ 2023-08-25 13:32 UTC (permalink / raw)
  To: jarkko, kai.huang, linux-sgx, linux-kernel; +Cc: dave.hansen, Jo Van Bulck

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 <jo.vanbulck@cs.kuleuven.be>
---
 tools/testing/selftests/sgx/test_encl_bootstrap.S | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/tools/testing/selftests/sgx/test_encl_bootstrap.S b/tools/testing/selftests/sgx/test_encl_bootstrap.S
index 4e55d91566c4..28fe5d2ac0af 100644
--- a/tools/testing/selftests/sgx/test_encl_bootstrap.S
+++ b/tools/testing/selftests/sgx/test_encl_bootstrap.S
@@ -58,12 +58,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
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v4 13/13] selftests/sgx: Remove incomplete ABI sanitization code in test enclave
  2023-08-25 13:32 [PATCH v4 00/13] selftests/sgx: Fix compilation errors Jo Van Bulck
                   ` (11 preceding siblings ...)
  2023-08-25 13:32 ` [PATCH v4 12/13] selftests/sgx: Remove redundant enclave base address save/restore Jo Van Bulck
@ 2023-08-25 13:32 ` Jo Van Bulck
  12 siblings, 0 replies; 25+ messages in thread
From: Jo Van Bulck @ 2023-08-25 13:32 UTC (permalink / raw)
  To: jarkko, kai.huang, linux-sgx, linux-kernel; +Cc: dave.hansen, Jo Van Bulck

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.

Link: https://lore.kernel.org/all/da0cfb1e-e347-f7f2-ac72-aec0ee0d867d@intel.com/
Signed-off-by: Jo Van Bulck <jo.vanbulck@cs.kuleuven.be>
---
 .../testing/selftests/sgx/test_encl_bootstrap.S  | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

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
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 09/13] selftests/sgx: Specify freestanding environment for enclave compilation
  2023-08-25 13:32 ` [PATCH v4 09/13] selftests/sgx: Specify freestanding environment for enclave compilation Jo Van Bulck
@ 2023-08-27 18:35   ` Jarkko Sakkinen
  0 siblings, 0 replies; 25+ messages in thread
From: Jarkko Sakkinen @ 2023-08-27 18:35 UTC (permalink / raw)
  To: Jo Van Bulck, kai.huang, linux-sgx, linux-kernel; +Cc: dave.hansen

On Fri Aug 25, 2023 at 4:32 PM EEST, Jo Van Bulck wrote:
> 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 <jo.vanbulck@cs.kuleuven.be>
> ---
>  tools/testing/selftests/sgx/Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/sgx/Makefile b/tools/testing/selftests/sgx/Makefile
> index 2de970f7237c..f96667bdf9f2 100644
> --- a/tools/testing/selftests/sgx/Makefile
> +++ b/tools/testing/selftests/sgx/Makefile
> @@ -14,8 +14,8 @@ endif
>  INCLUDES := -I$(top_srcdir)/tools/include
>  HOST_CFLAGS := -Wall -Werror -g $(INCLUDES) -fPIC
>  HOST_LDFLAGS := -z noexecstack -lcrypto
> -ENCL_CFLAGS := -Wall -Werror -static-pie -nostdlib -nostartfiles -fPIE \
> -	       -fno-stack-protector -mrdrnd $(INCLUDES)
> +ENCL_CFLAGS := -Wall -Werror -static-pie -nostdlib -ffreestanding -fPIE \
> +		-fno-stack-protector -mrdrnd $(INCLUDES)
>  ENCL_LDFLAGS := -Wl,-T,test_encl.lds,--build-id=none
>  
>  TEST_CUSTOM_PROGS := $(OUTPUT)/test_sgx
> -- 
> 2.25.1

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 10/13] selftests/sgx: Fix uninitialized pointer dereferences
  2023-08-25 13:32 ` [PATCH v4 10/13] selftests/sgx: Fix uninitialized pointer dereferences Jo Van Bulck
@ 2023-08-27 18:36   ` Jarkko Sakkinen
  2023-08-31  8:12     ` Jo Van Bulck
  0 siblings, 1 reply; 25+ messages in thread
From: Jarkko Sakkinen @ 2023-08-27 18:36 UTC (permalink / raw)
  To: Jo Van Bulck, kai.huang, linux-sgx, linux-kernel; +Cc: dave.hansen

On Fri Aug 25, 2023 at 4:32 PM EEST, Jo Van Bulck wrote:
> 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 <jo.vanbulck@cs.kuleuven.be>
> ---
>  tools/testing/selftests/sgx/load.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> 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];
>  
> -- 
> 2.25.1

Bug fixes should be always in the head of the patch set.

BR, Jarkko

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 03/13] selftests/sgx: Handle relocations in test enclave
  2023-08-25 13:32 ` [PATCH v4 03/13] selftests/sgx: Handle relocations in " Jo Van Bulck
@ 2023-08-28 13:15   ` Huang, Kai
  2023-08-31 12:48     ` Jo Van Bulck
  0 siblings, 1 reply; 25+ messages in thread
From: Huang, Kai @ 2023-08-28 13:15 UTC (permalink / raw)
  To: linux-sgx, jarkko, linux-kernel, Van Bulck, Jo; +Cc: dave.hansen

On Fri, 2023-08-25 at 15:32 +0200, Jo Van Bulck wrote:
> 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.

Sorry for late reply.

I am wondering is this the right justification for _this_ particular patch?

Even above paragraph is true, the existing code w/o this patch can work because
the generated asm code uses "lea (-xxx)(%rip), %<reg>" to get the right address
and store it to the array on the stack.

It stops to work because you want to use -Os, in which case the generated asm
code instead initializes the array by copying an array (which has function
addresses starting from 0) generated by the compiler/linker.

So to me the true justification should be "using -Os breaks the code".  Or do
you think "the compiler generating code to initialize the array on the stack
using RIP-relative addressing to get the function address" is completely a lucky
thing?

Anyway, it will be very helpful to include the assembly code generated both w/
and w/o using -Os here to the changelog to demonstrate the problem and we need
this patch to fixup.

Without those information, it's basically very hard for people to understand why
this is needed.  This will save maintainer's time, and make git blamer's life
easy in the future.

> 
> Signed-off-by: Jo Van Bulck <jo.vanbulck@cs.kuleuven.be>
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
>  tools/testing/selftests/sgx/test_encl.c | 48 +++++++++++++++++--------
>  1 file changed, 33 insertions(+), 15 deletions(-)
> 
> diff --git a/tools/testing/selftests/sgx/test_encl.c b/tools/testing/selftests/sgx/test_encl.c
> index c0d6397295e3..706c1f7260ff 100644
> --- a/tools/testing/selftests/sgx/test_encl.c
> +++ b/tools/testing/selftests/sgx/test_encl.c
> @@ -119,21 +119,39 @@ 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.
> + */
> +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.
> +	 */

Is it better to explicitly call out the compiler generates RIP-relative
addressing code to get the address associated with '__encl_base' symbol, so we
can get the actual enclave base during runtime?

Maybe it's obvious, but I am not sure :-)

> +	op = ((uint64_t)&__encl_base) + encl_op_array[header->type];
> +
> +	(*op)(header);
>  }



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 02/13] selftests/sgx: Produce static-pie executable for test enclave
  2023-08-25 13:32 ` [PATCH v4 02/13] selftests/sgx: Produce static-pie executable for test enclave Jo Van Bulck
@ 2023-08-29 10:55   ` Huang, Kai
  2023-08-31  8:49     ` Jo Van Bulck
  0 siblings, 1 reply; 25+ messages in thread
From: Huang, Kai @ 2023-08-29 10:55 UTC (permalink / raw)
  To: linux-sgx, jarkko, linux-kernel, Van Bulck, Jo; +Cc: dave.hansen

On Fri, 2023-08-25 at 15:32 +0200, Jo Van Bulck wrote:
> 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.
> 
> Link: https://lore.kernel.org/all/f9c24d89-ed72-7d9e-c650-050d722c6b04@cs.kuleuven.be/
> Signed-off-by: Jo Van Bulck <jo.vanbulck@cs.kuleuven.be>
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
>  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(-)
> 
> diff --git a/tools/testing/selftests/sgx/Makefile b/tools/testing/selftests/sgx/Makefile
> index 50aab6b57da3..1d6315a2e5f5 100644
> --- a/tools/testing/selftests/sgx/Makefile
> +++ b/tools/testing/selftests/sgx/Makefile
> @@ -13,7 +13,7 @@ endif
>  
>  INCLUDES := -I$(top_srcdir)/tools/include
>  HOST_CFLAGS := -Wall -Werror -g $(INCLUDES) -fPIC -z noexecstack
> -ENCL_CFLAGS := -Wall -Werror -static -nostdlib -nostartfiles -fPIC \
> +ENCL_CFLAGS := -Wall -Werror -static-pie -nostdlib -nostartfiles -fPIE \
>  	       -fno-stack-protector -mrdrnd $(INCLUDES)
>  
>  TEST_CUSTOM_PROGS := $(OUTPUT)/test_sgx
> 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 03ae0f57e29d..4e55d91566c4 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

Could you please add the linker error (as you mentioned in the v3) to the
changelog to justify this code change?

And sigh.. I still don't quite understand why linker complains

	lea	(encl_stack)(%rbx), %rax

because ....

>  	jmp encl_entry_core
>  encl_dyn_entry:
>  	# Entry point for dynamically created TCS page expected to follow

we have a 

	lea -1(%rbx), %rax

right here.

Can't the compiler/linker just treat it as an immediate just like -1?  :-(

Anyway, I like this code change:

Acked-by: Kai Huang <kai.huang@intel.com>


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 12/13] selftests/sgx: Remove redundant enclave base address save/restore
  2023-08-25 13:32 ` [PATCH v4 12/13] selftests/sgx: Remove redundant enclave base address save/restore Jo Van Bulck
@ 2023-08-29 11:00   ` Huang, Kai
  2023-08-31  8:25     ` Jo Van Bulck
  0 siblings, 1 reply; 25+ messages in thread
From: Huang, Kai @ 2023-08-29 11:00 UTC (permalink / raw)
  To: linux-sgx, jarkko, linux-kernel, Van Bulck, Jo; +Cc: dave.hansen

On Fri, 2023-08-25 at 15:32 +0200, Jo Van Bulck wrote:
> 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 <jo.vanbulck@cs.kuleuven.be>

Acked-by: Kai Huang <kai.huang@intel.com>

Nit below...

> ---
>  tools/testing/selftests/sgx/test_encl_bootstrap.S | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/sgx/test_encl_bootstrap.S b/tools/testing/selftests/sgx/test_encl_bootstrap.S
> index 4e55d91566c4..28fe5d2ac0af 100644
> --- a/tools/testing/selftests/sgx/test_encl_bootstrap.S
> +++ b/tools/testing/selftests/sgx/test_encl_bootstrap.S
> @@ -58,12 +58,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

... please move this patch before patch 2?

Otherwise, strictly speaking patch 2 isn't complete, because after you apply
patch 2 you still have this code which is obviously wrong -- %rbx is no longer
enclave base address (although it never was even in the current upstream code).

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 05/13] selftests/sgx: Include memory clobber for inline asm in test enclave
  2023-08-25 13:32 ` [PATCH v4 05/13] selftests/sgx: Include memory clobber for inline asm in test enclave Jo Van Bulck
@ 2023-08-29 11:07   ` Huang, Kai
  2023-08-31  8:31     ` Jo Van Bulck
  0 siblings, 1 reply; 25+ messages in thread
From: Huang, Kai @ 2023-08-29 11:07 UTC (permalink / raw)
  To: linux-sgx, jarkko, linux-kernel, Van Bulck, Jo; +Cc: dave.hansen

On Fri, 2023-08-25 at 15:32 +0200, Jo Van Bulck wrote:
> 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.
> 
> Signed-off-by: Jo Van Bulck <jo.vanbulck@cs.kuleuven.be>
> Reviewed-by: Kai Huang <kai.huang@intel.com>
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
>  tools/testing/selftests/sgx/test_encl.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/sgx/test_encl.c b/tools/testing/selftests/sgx/test_encl.c
> index 706c1f7260ff..dfe531c5f560 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;
>  }

To me this is also a bug fix patch, thus as Jarkko said should be ahead of other
non-bug fix patches.

And also include the Fixes tag?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 10/13] selftests/sgx: Fix uninitialized pointer dereferences
  2023-08-27 18:36   ` Jarkko Sakkinen
@ 2023-08-31  8:12     ` Jo Van Bulck
  0 siblings, 0 replies; 25+ messages in thread
From: Jo Van Bulck @ 2023-08-31  8:12 UTC (permalink / raw)
  To: Jarkko Sakkinen, kai.huang, linux-sgx, linux-kernel; +Cc: dave.hansen

On 27.08.23 20:36, Jarkko Sakkinen wrote:> Bug fixes should be always in 
the head of the patch set.

Thanks for pointing this out. I'll make sure to move this to the head in 
the next patch set revision.

Best,
Jo

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 12/13] selftests/sgx: Remove redundant enclave base address save/restore
  2023-08-29 11:00   ` Huang, Kai
@ 2023-08-31  8:25     ` Jo Van Bulck
  0 siblings, 0 replies; 25+ messages in thread
From: Jo Van Bulck @ 2023-08-31  8:25 UTC (permalink / raw)
  To: Huang, Kai, linux-sgx, jarkko, linux-kernel; +Cc: dave.hansen

On 29.08.23 13:00, Huang, Kai wrote:
> ... please move this patch before patch 2?
> 
> Otherwise, strictly speaking patch 2 isn't complete, because after you apply
> patch 2 you still have this code which is obviously wrong -- %rbx is no longer
> enclave base address (although it never was even in the current upstream code).

Okay, moving this ahead.

Best,
Jo

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 05/13] selftests/sgx: Include memory clobber for inline asm in test enclave
  2023-08-29 11:07   ` Huang, Kai
@ 2023-08-31  8:31     ` Jo Van Bulck
  0 siblings, 0 replies; 25+ messages in thread
From: Jo Van Bulck @ 2023-08-31  8:31 UTC (permalink / raw)
  To: Huang, Kai, linux-sgx, jarkko, linux-kernel; +Cc: dave.hansen

On 29.08.23 13:07, Huang, Kai wrote:> To me this is also a bug fix 
patch, thus as Jarkko said should be ahead of other
> non-bug fix patches.
> 
> And also include the Fixes tag?

Thanks, moving this ahead and including the tag Fixes: 20404a808593 
("selftests/sgx: Add test for EPCM permission changes")

Best,
Jo

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 02/13] selftests/sgx: Produce static-pie executable for test enclave
  2023-08-29 10:55   ` Huang, Kai
@ 2023-08-31  8:49     ` Jo Van Bulck
  0 siblings, 0 replies; 25+ messages in thread
From: Jo Van Bulck @ 2023-08-31  8:49 UTC (permalink / raw)
  To: Huang, Kai, linux-sgx, jarkko, linux-kernel; +Cc: dave.hansen

On 29.08.23 12:55, Huang, Kai wrote:> Could you please add the linker 
error (as you mentioned in the v3) to the
> changelog to justify this code change?

Makes sense, done.

> And sigh.. I still don't quite understand why linker complains
> 
> 	lea	(encl_stack)(%rbx), %rax
> 
> because ....
> 
>>   	jmp encl_entry_core
>>   encl_dyn_entry:
>>   	# Entry point for dynamically created TCS page expected to follow
> 
> we have a
> 
> 	lea -1(%rbx), %rax
> 
> right here.
> 
> Can't the compiler/linker just treat it as an immediate just like -1?  :-(

I think the linker complains because with -static-pie -fPIE afaik all 
local symbols need to have RIP-relative addressing "encl_stack(%rip)".

Best,
Jo

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 03/13] selftests/sgx: Handle relocations in test enclave
  2023-08-28 13:15   ` Huang, Kai
@ 2023-08-31 12:48     ` Jo Van Bulck
  0 siblings, 0 replies; 25+ messages in thread
From: Jo Van Bulck @ 2023-08-31 12:48 UTC (permalink / raw)
  To: Huang, Kai, linux-sgx, jarkko, linux-kernel; +Cc: dave.hansen

On 28.08.23 15:15, Huang, Kai wrote:
> I am wondering is this the right justification for _this_ particular patch?
> 
> Even above paragraph is true, the existing code w/o this patch can work because
> the generated asm code uses "lea (-xxx)(%rip), %<reg>" to get the right address
> and store it to the array on the stack.

Yes, I agree the current code *happens* to work with this explicit array 
initialization.

> It stops to work because you want to use -Os, in which case the generated asm
> code instead initializes the array by copying an array (which has function
> addresses starting from 0) generated by the compiler/linker.

I'd say the compiler is free to perform this sensible optimization, as 
long as it marks any relocations in .rela.dyn. Thus, the *real* reason 
why it stops to work is that the enclave does not include a startup 
routine to perform any ELF relocations from .rela.dyn (as included in 
glibc).

The minimal fix, done in this patch, is to not include a full .rela.dyn 
relocation routine with all the overheads of parsing, but simply 
manually relocate the only place where this may be needed, ie the 
function pointer table. Ultimately, I could imagine a further 
enhancement may also be to parse .rela.dyn at build time and make sure 
no other relocations are there (outside the false positives for the TCS 
as discussed earlier).

> So to me the true justification should be "using -Os breaks the code". 

I'd say compiler optimizations should not break correct code. In other 
words, the main objective of this patch series is to avoid reliance on 
undefined, compiler-specific behavior that can make the test results 
unpredictable and fragile as compiler versions or options may change in 
the future.

> Or do
> you think "the compiler generating code to initialize the array on the stack
> using RIP-relative addressing to get the function address" is completely a lucky
> thing?

To some extent, yes. While I only saw this with -Os for gcc, I found 
that clang never initializes the array on the stack and this may also 
change for gcc at any point I'd expect.

For reference, I'm including the full encl_body assembly for both gcc 
and clang for -O{1,2,3,s,g} at the bottom of this email.

> Anyway, it will be very helpful to include the assembly code generated both w/
> and w/o using -Os here to the changelog to demonstrate the problem and we need
> this patch to fixup. >
> Without those information, it's basically very hard for people to understand why
> this is needed.  This will save maintainer's time, and make git blamer's life
> easy in the future.

Makes sense, will do this for the next revision.

>> +	/*
>> +	 * 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.
>> +	 */
> 
> Is it better to explicitly call out the compiler generates RIP-relative
> addressing code to get the address associated with '__encl_base' symbol, so we
> can get the actual enclave base during runtime?
> 
> Maybe it's obvious, but I am not sure :-)
> 
>> +	op = ((uint64_t)&__encl_base) + encl_op_array[header->type];
>> +
>> +	(*op)(header);
>>   }

I'm including a comment on this a few lines higher, where __encl_base is 
declared.

Best,
Jo

------
clang.-O0.log.elf

0000000000002000 <encl_body>:
     2000:	55                   	push   %rbp
     2001:	48 89 e5             	mov    %rsp,%rbp
     2004:	48 83 ec 60          	sub    $0x60,%rsp
     2008:	48 8d 05 f1 1f 00 00 	lea    0x1ff1(%rip),%rax        # 4000 
<encl_entry_core+0x1b77>
     200f:	48 89 7d f8          	mov    %rdi,-0x8(%rbp)
     2013:	48 89 75 f0          	mov    %rsi,-0x10(%rbp)
     2017:	48 8d 4d b0          	lea    -0x50(%rbp),%rcx
     201b:	48 89 cf             	mov    %rcx,%rdi
     201e:	48 89 c6             	mov    %rax,%rsi
     2021:	ba 40 00 00 00       	mov    $0x40,%edx
     2026:	e8 95 03 00 00       	call   23c0 <memcpy>
     202b:	48 8b 45 f8          	mov    -0x8(%rbp),%rax
     202f:	48 89 45 a8          	mov    %rax,-0x58(%rbp)
     2033:	48 8b 45 a8          	mov    -0x58(%rbp),%rax
     2037:	48 83 38 08          	cmpq   $0x8,(%rax)
     203b:	0f 83 15 00 00 00    	jae    2056 <encl_body+0x56>
     2041:	48 8b 45 a8          	mov    -0x58(%rbp),%rax
     2045:	48 8b 00             	mov    (%rax),%rax
     2048:	48 8b 44 c5 b0       	mov    -0x50(%rbp,%rax,8),%rax
     204d:	48 8b 4d a8          	mov    -0x58(%rbp),%rcx
     2051:	48 89 cf             	mov    %rcx,%rdi
     2054:	ff d0                	call   *%rax
     2056:	48 83 c4 60          	add    $0x60,%rsp
     205a:	5d                   	pop    %rbp
     205b:	c3                   	ret

------
clang.-O1.log.elf

0000000000002000 <encl_body>:
     2000:	50                   	push   %rax
     2001:	48 8b 07             	mov    (%rdi),%rax
     2004:	48 83 f8 07          	cmp    $0x7,%rax
     2008:	77 0a                	ja     2014 <encl_body+0x14>
     200a:	48 8d 0d ef 1f 00 00 	lea    0x1fef(%rip),%rcx        # 4000 
<encl_entry_core+0x1d86>
     2011:	ff 14 c1             	call   *(%rcx,%rax,8)
     2014:	58                   	pop    %rax
     2015:	c3                   	ret

------
clang.-O2.log.elf

0000000000002000 <encl_body>:
     2000:	48 8b 07             	mov    (%rdi),%rax
     2003:	48 83 f8 07          	cmp    $0x7,%rax
     2007:	77 0a                	ja     2013 <encl_body+0x13>
     2009:	48 8d 0d f0 1f 00 00 	lea    0x1ff0(%rip),%rcx        # 4000 
<encl_entry_core+0x1cfa>
     2010:	ff 24 c1             	jmp    *(%rcx,%rax,8)
     2013:	c3                   	ret

------
clang.-O3.log.elf

0000000000002000 <encl_body>:
     2000:	48 8b 07             	mov    (%rdi),%rax
     2003:	48 83 f8 07          	cmp    $0x7,%rax
     2007:	77 0a                	ja     2013 <encl_body+0x13>
     2009:	48 8d 0d f0 1f 00 00 	lea    0x1ff0(%rip),%rcx        # 4000 
<encl_entry_core+0x1cfa>
     2010:	ff 24 c1             	jmp    *(%rcx,%rax,8)
     2013:	c3                   	ret

------
clang.-Ofast.log.elf

0000000000002000 <encl_body>:
     2000:	48 8b 07             	mov    (%rdi),%rax
     2003:	48 83 f8 07          	cmp    $0x7,%rax
     2007:	77 0a                	ja     2013 <encl_body+0x13>
     2009:	48 8d 0d f0 1f 00 00 	lea    0x1ff0(%rip),%rcx        # 4000 
<encl_entry_core+0x1cfa>
     2010:	ff 24 c1             	jmp    *(%rcx,%rax,8)
     2013:	c3                   	ret

------
clang.-Og.log.elf

0000000000002000 <encl_body>:
     2000:	50                   	push   %rax
     2001:	48 8b 07             	mov    (%rdi),%rax
     2004:	48 83 f8 07          	cmp    $0x7,%rax
     2008:	77 0a                	ja     2014 <encl_body+0x14>
     200a:	48 8d 0d ef 1f 00 00 	lea    0x1fef(%rip),%rcx        # 4000 
<encl_entry_core+0x1d86>
     2011:	ff 14 c1             	call   *(%rcx,%rax,8)
     2014:	58                   	pop    %rax
     2015:	c3                   	ret

------
clang.-Os.log.elf

0000000000002000 <encl_body>:
     2000:	48 8b 07             	mov    (%rdi),%rax
     2003:	48 83 f8 07          	cmp    $0x7,%rax
     2007:	77 0a                	ja     2013 <encl_body+0x13>
     2009:	48 8d 0d f0 1f 00 00 	lea    0x1ff0(%rip),%rcx        # 4000 
<encl_entry_core+0x1e36>
     2010:	ff 24 c1             	jmp    *(%rcx,%rax,8)
     2013:	c3                   	ret

------
gcc.-O0.log.elf

00000000000023f4 <encl_body>:
     23f4:	f3 0f 1e fa          	endbr64
     23f8:	55                   	push   %rbp
     23f9:	48 89 e5             	mov    %rsp,%rbp
     23fc:	48 83 ec 60          	sub    $0x60,%rsp
     2400:	48 89 7d a8          	mov    %rdi,-0x58(%rbp)
     2404:	48 89 75 a0          	mov    %rsi,-0x60(%rbp)
     2408:	48 8d 05 ec fe ff ff 	lea    -0x114(%rip),%rax        # 22fb 
<do_encl_op_put_to_buf>
     240f:	48 89 45 b0          	mov    %rax,-0x50(%rbp)
     2413:	48 8d 05 18 ff ff ff 	lea    -0xe8(%rip),%rax        # 2332 
<do_encl_op_get_from_buf>
     241a:	48 89 45 b8          	mov    %rax,-0x48(%rbp)
     241e:	48 8d 05 44 ff ff ff 	lea    -0xbc(%rip),%rax        # 2369 
<do_encl_op_put_to_addr>
     2425:	48 89 45 c0          	mov    %rax,-0x40(%rbp)
     2429:	48 8d 05 77 ff ff ff 	lea    -0x89(%rip),%rax        # 23a7 
<do_encl_op_get_from_addr>
     2430:	48 89 45 c8          	mov    %rax,-0x38(%rbp)
     2434:	48 8d 05 aa ff ff ff 	lea    -0x56(%rip),%rax        # 23e5 
<do_encl_op_nop>
     243b:	48 89 45 d0          	mov    %rax,-0x30(%rbp)
     243f:	48 8d 05 4f fc ff ff 	lea    -0x3b1(%rip),%rax        # 2095 
<do_encl_eaccept>
     2446:	48 89 45 d8          	mov    %rax,-0x28(%rbp)
     244a:	48 8d 05 af fb ff ff 	lea    -0x451(%rip),%rax        # 2000 
<do_encl_emodpe>
     2451:	48 89 45 e0          	mov    %rax,-0x20(%rbp)
     2455:	48 8d 05 72 fd ff ff 	lea    -0x28e(%rip),%rax        # 21ce 
<do_encl_init_tcs_page>
     245c:	48 89 45 e8          	mov    %rax,-0x18(%rbp)
     2460:	48 8b 45 a8          	mov    -0x58(%rbp),%rax
     2464:	48 89 45 f8          	mov    %rax,-0x8(%rbp)
     2468:	48 8b 45 f8          	mov    -0x8(%rbp),%rax
     246c:	48 8b 00             	mov    (%rax),%rax
     246f:	48 83 f8 07          	cmp    $0x7,%rax
     2473:	77 15                	ja     248a <encl_body+0x96>
     2475:	48 8b 45 f8          	mov    -0x8(%rbp),%rax
     2479:	48 8b 00             	mov    (%rax),%rax
     247c:	48 8b 54 c5 b0       	mov    -0x50(%rbp,%rax,8),%rdx
     2481:	48 8b 45 f8          	mov    -0x8(%rbp),%rax
     2485:	48 89 c7             	mov    %rax,%rdi
     2488:	ff d2                	call   *%rdx
     248a:	90                   	nop
     248b:	c9                   	leave
     248c:	c3                   	ret

------
gcc.-O1.log.elf

0000000000002239 <encl_body>:
     2239:	f3 0f 1e fa          	endbr64
     223d:	48 83 ec 48          	sub    $0x48,%rsp
     2241:	48 8d 05 b6 fe ff ff 	lea    -0x14a(%rip),%rax        # 20fe 
<do_encl_op_put_to_buf>
     2248:	48 89 04 24          	mov    %rax,(%rsp)
     224c:	48 8d 05 c5 fe ff ff 	lea    -0x13b(%rip),%rax        # 2118 
<do_encl_op_get_from_buf>
     2253:	48 89 44 24 08       	mov    %rax,0x8(%rsp)
     2258:	48 8d 05 d3 fe ff ff 	lea    -0x12d(%rip),%rax        # 2132 
<do_encl_op_put_to_addr>
     225f:	48 89 44 24 10       	mov    %rax,0x10(%rsp)
     2264:	48 8d 05 de fe ff ff 	lea    -0x122(%rip),%rax        # 2149 
<do_encl_op_get_from_addr>
     226b:	48 89 44 24 18       	mov    %rax,0x18(%rsp)
     2270:	48 8d 05 e9 fe ff ff 	lea    -0x117(%rip),%rax        # 2160 
<do_encl_op_nop>
     2277:	48 89 44 24 20       	mov    %rax,0x20(%rsp)
     227c:	48 8d 05 e9 fd ff ff 	lea    -0x217(%rip),%rax        # 206c 
<do_encl_eaccept>
     2283:	48 89 44 24 28       	mov    %rax,0x28(%rsp)
     2288:	48 8d 05 71 fd ff ff 	lea    -0x28f(%rip),%rax        # 2000 
<do_encl_emodpe>
     228f:	48 89 44 24 30       	mov    %rax,0x30(%rsp)
     2294:	48 8d 05 ca fe ff ff 	lea    -0x136(%rip),%rax        # 2165 
<do_encl_init_tcs_page>
     229b:	48 89 44 24 38       	mov    %rax,0x38(%rsp)
     22a0:	48 8b 07             	mov    (%rdi),%rax
     22a3:	48 83 f8 07          	cmp    $0x7,%rax
     22a7:	77 03                	ja     22ac <encl_body+0x73>
     22a9:	ff 14 c4             	call   *(%rsp,%rax,8)
     22ac:	48 83 c4 48          	add    $0x48,%rsp
     22b0:	c3                   	ret

------
gcc.-O2.log.elf

0000000000002210 <encl_body>:
     2210:	f3 0f 1e fa          	endbr64
     2214:	48 8d 05 25 ff ff ff 	lea    -0xdb(%rip),%rax        # 2140 
<do_encl_op_put_to_buf>
     221b:	48 89 44 24 b8       	mov    %rax,-0x48(%rsp)
     2220:	48 8d 05 49 ff ff ff 	lea    -0xb7(%rip),%rax        # 2170 
<do_encl_op_get_from_buf>
     2227:	48 89 44 24 c0       	mov    %rax,-0x40(%rsp)
     222c:	48 8d 05 6d ff ff ff 	lea    -0x93(%rip),%rax        # 21a0 
<do_encl_op_put_to_addr>
     2233:	48 89 44 24 c8       	mov    %rax,-0x38(%rsp)
     2238:	48 8d 05 91 ff ff ff 	lea    -0x6f(%rip),%rax        # 21d0 
<do_encl_op_get_from_addr>
     223f:	48 89 44 24 d0       	mov    %rax,-0x30(%rsp)
     2244:	48 8d 05 b5 ff ff ff 	lea    -0x4b(%rip),%rax        # 2200 
<do_encl_op_nop>
     224b:	48 89 44 24 d8       	mov    %rax,-0x28(%rsp)
     2250:	48 8d 05 f9 fd ff ff 	lea    -0x207(%rip),%rax        # 2050 
<do_encl_eaccept>
     2257:	48 89 44 24 e0       	mov    %rax,-0x20(%rsp)
     225c:	48 8d 05 9d fd ff ff 	lea    -0x263(%rip),%rax        # 2000 
<do_encl_emodpe>
     2263:	48 89 44 24 e8       	mov    %rax,-0x18(%rsp)
     2268:	48 8d 05 31 fe ff ff 	lea    -0x1cf(%rip),%rax        # 20a0 
<do_encl_init_tcs_page>
     226f:	48 89 44 24 f0       	mov    %rax,-0x10(%rsp)
     2274:	48 8b 07             	mov    (%rdi),%rax
     2277:	48 83 f8 07          	cmp    $0x7,%rax
     227b:	77 0b                	ja     2288 <encl_body+0x78>
     227d:	ff 64 c4 b8          	jmp    *-0x48(%rsp,%rax,8)
     2281:	0f 1f 80 00 00 00 00 	nopl   0x0(%rax)
     2288:	c3                   	ret

------
gcc.-O3.log.elf

0000000000002220 <encl_body>:
     2220:	f3 0f 1e fa          	endbr64
     2224:	48 8d 05 55 ff ff ff 	lea    -0xab(%rip),%rax        # 2180 
<do_encl_op_get_from_buf>
     222b:	48 8d 15 3e ff ff ff 	lea    -0xc2(%rip),%rdx        # 2170 
<do_encl_op_put_to_buf>
     2232:	66 48 0f 6e c2       	movq   %rdx,%xmm0
     2237:	66 48 0f 6e c8       	movq   %rax,%xmm1
     223c:	48 8d 0d 4d ff ff ff 	lea    -0xb3(%rip),%rcx        # 2190 
<do_encl_op_put_to_addr>
     2243:	66 0f 6c c1          	punpcklqdq %xmm1,%xmm0
     2247:	48 8d 05 82 ff ff ff 	lea    -0x7e(%rip),%rax        # 21d0 
<do_encl_op_get_from_addr>
     224e:	48 8d 35 bb ff ff ff 	lea    -0x45(%rip),%rsi        # 2210 
<do_encl_op_nop>
     2255:	66 48 0f 6e d0       	movq   %rax,%xmm2
     225a:	0f 29 44 24 b8       	movaps %xmm0,-0x48(%rsp)
     225f:	66 48 0f 6e c1       	movq   %rcx,%xmm0
     2264:	48 8d 05 e5 fd ff ff 	lea    -0x21b(%rip),%rax        # 2050 
<do_encl_eaccept>
     226b:	66 0f 6c c2          	punpcklqdq %xmm2,%xmm0
     226f:	66 48 0f 6e d8       	movq   %rax,%xmm3
     2274:	48 8d 15 85 fd ff ff 	lea    -0x27b(%rip),%rdx        # 2000 
<do_encl_emodpe>
     227b:	0f 29 44 24 c8       	movaps %xmm0,-0x38(%rsp)
     2280:	66 48 0f 6e c6       	movq   %rsi,%xmm0
     2285:	48 8d 05 14 fe ff ff 	lea    -0x1ec(%rip),%rax        # 20a0 
<do_encl_init_tcs_page>
     228c:	66 0f 6c c3          	punpcklqdq %xmm3,%xmm0
     2290:	66 48 0f 6e e0       	movq   %rax,%xmm4
     2295:	48 8b 07             	mov    (%rdi),%rax
     2298:	0f 29 44 24 d8       	movaps %xmm0,-0x28(%rsp)
     229d:	66 48 0f 6e c2       	movq   %rdx,%xmm0
     22a2:	66 0f 6c c4          	punpcklqdq %xmm4,%xmm0
     22a6:	0f 29 44 24 e8       	movaps %xmm0,-0x18(%rsp)
     22ab:	48 83 f8 07          	cmp    $0x7,%rax
     22af:	77 07                	ja     22b8 <encl_body+0x98>
     22b1:	ff 64 c4 b8          	jmp    *-0x48(%rsp,%rax,8)
     22b5:	0f 1f 00             	nopl   (%rax)
     22b8:	c3                   	ret

------
gcc.-Ofast.log.elf

0000000000002220 <encl_body>:
     2220:	f3 0f 1e fa          	endbr64
     2224:	48 8d 05 55 ff ff ff 	lea    -0xab(%rip),%rax        # 2180 
<do_encl_op_get_from_buf>
     222b:	48 8d 15 3e ff ff ff 	lea    -0xc2(%rip),%rdx        # 2170 
<do_encl_op_put_to_buf>
     2232:	66 48 0f 6e c2       	movq   %rdx,%xmm0
     2237:	66 48 0f 6e c8       	movq   %rax,%xmm1
     223c:	48 8d 0d 4d ff ff ff 	lea    -0xb3(%rip),%rcx        # 2190 
<do_encl_op_put_to_addr>
     2243:	66 0f 6c c1          	punpcklqdq %xmm1,%xmm0
     2247:	48 8d 05 82 ff ff ff 	lea    -0x7e(%rip),%rax        # 21d0 
<do_encl_op_get_from_addr>
     224e:	48 8d 35 bb ff ff ff 	lea    -0x45(%rip),%rsi        # 2210 
<do_encl_op_nop>
     2255:	66 48 0f 6e d0       	movq   %rax,%xmm2
     225a:	0f 29 44 24 b8       	movaps %xmm0,-0x48(%rsp)
     225f:	66 48 0f 6e c1       	movq   %rcx,%xmm0
     2264:	48 8d 05 e5 fd ff ff 	lea    -0x21b(%rip),%rax        # 2050 
<do_encl_eaccept>
     226b:	66 0f 6c c2          	punpcklqdq %xmm2,%xmm0
     226f:	66 48 0f 6e d8       	movq   %rax,%xmm3
     2274:	48 8d 15 85 fd ff ff 	lea    -0x27b(%rip),%rdx        # 2000 
<do_encl_emodpe>
     227b:	0f 29 44 24 c8       	movaps %xmm0,-0x38(%rsp)
     2280:	66 48 0f 6e c6       	movq   %rsi,%xmm0
     2285:	48 8d 05 14 fe ff ff 	lea    -0x1ec(%rip),%rax        # 20a0 
<do_encl_init_tcs_page>
     228c:	66 0f 6c c3          	punpcklqdq %xmm3,%xmm0
     2290:	66 48 0f 6e e0       	movq   %rax,%xmm4
     2295:	48 8b 07             	mov    (%rdi),%rax
     2298:	0f 29 44 24 d8       	movaps %xmm0,-0x28(%rsp)
     229d:	66 48 0f 6e c2       	movq   %rdx,%xmm0
     22a2:	66 0f 6c c4          	punpcklqdq %xmm4,%xmm0
     22a6:	0f 29 44 24 e8       	movaps %xmm0,-0x18(%rsp)
     22ab:	48 83 f8 07          	cmp    $0x7,%rax
     22af:	77 07                	ja     22b8 <encl_body+0x98>
     22b1:	ff 64 c4 b8          	jmp    *-0x48(%rsp,%rax,8)
     22b5:	0f 1f 00             	nopl   (%rax)
     22b8:	c3                   	ret

------
gcc.-Og.log.elf

000000000000225f <encl_body>:
     225f:	f3 0f 1e fa          	endbr64
     2263:	48 83 ec 48          	sub    $0x48,%rsp
     2267:	48 8d 05 8a ff ff ff 	lea    -0x76(%rip),%rax        # 21f8 
<do_encl_op_put_to_buf>
     226e:	48 89 04 24          	mov    %rax,(%rsp)
     2272:	48 8d 05 99 ff ff ff 	lea    -0x67(%rip),%rax        # 2212 
<do_encl_op_get_from_buf>
     2279:	48 89 44 24 08       	mov    %rax,0x8(%rsp)
     227e:	48 8d 05 a7 ff ff ff 	lea    -0x59(%rip),%rax        # 222c 
<do_encl_op_put_to_addr>
     2285:	48 89 44 24 10       	mov    %rax,0x10(%rsp)
     228a:	48 8d 05 b2 ff ff ff 	lea    -0x4e(%rip),%rax        # 2243 
<do_encl_op_get_from_addr>
     2291:	48 89 44 24 18       	mov    %rax,0x18(%rsp)
     2296:	48 8d 05 bd ff ff ff 	lea    -0x43(%rip),%rax        # 225a 
<do_encl_op_nop>
     229d:	48 89 44 24 20       	mov    %rax,0x20(%rsp)
     22a2:	48 8d 05 cc fd ff ff 	lea    -0x234(%rip),%rax        # 2075 
<do_encl_eaccept>
     22a9:	48 89 44 24 28       	mov    %rax,0x28(%rsp)
     22ae:	48 8d 05 4b fd ff ff 	lea    -0x2b5(%rip),%rax        # 2000 
<do_encl_emodpe>
     22b5:	48 89 44 24 30       	mov    %rax,0x30(%rsp)
     22ba:	48 8d 05 64 fe ff ff 	lea    -0x19c(%rip),%rax        # 2125 
<do_encl_init_tcs_page>
     22c1:	48 89 44 24 38       	mov    %rax,0x38(%rsp)
     22c6:	48 8b 07             	mov    (%rdi),%rax
     22c9:	48 83 f8 07          	cmp    $0x7,%rax
     22cd:	77 03                	ja     22d2 <encl_body+0x73>
     22cf:	ff 14 c4             	call   *(%rsp,%rax,8)
     22d2:	48 83 c4 48          	add    $0x48,%rsp
     22d6:	c3                   	ret

------
gcc.-Os.log.elf

00000000000021a9 <encl_body>:
     21a9:	f3 0f 1e fa          	endbr64
     21ad:	49 89 f8             	mov    %rdi,%r8
     21b0:	48 8d 35 49 1e 00 00 	lea    0x1e49(%rip),%rsi        # 4000 
<encl_entry_core+0x1e0f>
     21b7:	48 8d 7c 24 b8       	lea    -0x48(%rsp),%rdi
     21bc:	b9 10 00 00 00       	mov    $0x10,%ecx
     21c1:	f3 a5                	rep movsl %ds:(%rsi),%es:(%rdi)
     21c3:	49 8b 00             	mov    (%r8),%rax
     21c6:	48 83 f8 07          	cmp    $0x7,%rax
     21ca:	77 0a                	ja     21d6 <encl_body+0x2d>
     21cc:	48 8b 44 c4 b8       	mov    -0x48(%rsp,%rax,8),%rax
     21d1:	4c 89 c7             	mov    %r8,%rdi
     21d4:	ff e0                	jmp    *%rax
     21d6:	c3                   	ret


^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2023-08-31 12:48 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-25 13:32 [PATCH v4 00/13] selftests/sgx: Fix compilation errors Jo Van Bulck
2023-08-25 13:32 ` [PATCH v4 01/13] selftests/sgx: Fix uninitialized pointer dereference in error path Jo Van Bulck
2023-08-25 13:32 ` [PATCH v4 02/13] selftests/sgx: Produce static-pie executable for test enclave Jo Van Bulck
2023-08-29 10:55   ` Huang, Kai
2023-08-31  8:49     ` Jo Van Bulck
2023-08-25 13:32 ` [PATCH v4 03/13] selftests/sgx: Handle relocations in " Jo Van Bulck
2023-08-28 13:15   ` Huang, Kai
2023-08-31 12:48     ` Jo Van Bulck
2023-08-25 13:32 ` [PATCH v4 04/13] selftests/sgx: Fix linker script asserts Jo Van Bulck
2023-08-25 13:32 ` [PATCH v4 05/13] selftests/sgx: Include memory clobber for inline asm in test enclave Jo Van Bulck
2023-08-29 11:07   ` Huang, Kai
2023-08-31  8:31     ` Jo Van Bulck
2023-08-25 13:32 ` [PATCH v4 06/13] selftests/sgx: Ensure test enclave buffer is entirely preserved Jo Van Bulck
2023-08-25 13:32 ` [PATCH v4 07/13] selftests/sgx: Ensure expected location of test enclave buffer Jo Van Bulck
2023-08-25 13:32 ` [PATCH v4 08/13] selftests/sgx: Separate linker options Jo Van Bulck
2023-08-25 13:32 ` [PATCH v4 09/13] selftests/sgx: Specify freestanding environment for enclave compilation Jo Van Bulck
2023-08-27 18:35   ` Jarkko Sakkinen
2023-08-25 13:32 ` [PATCH v4 10/13] selftests/sgx: Fix uninitialized pointer dereferences Jo Van Bulck
2023-08-27 18:36   ` Jarkko Sakkinen
2023-08-31  8:12     ` Jo Van Bulck
2023-08-25 13:32 ` [PATCH v4 11/13] selftests/sgx: Discard unsupported ELF sections Jo Van Bulck
2023-08-25 13:32 ` [PATCH v4 12/13] selftests/sgx: Remove redundant enclave base address save/restore Jo Van Bulck
2023-08-29 11:00   ` Huang, Kai
2023-08-31  8:25     ` Jo Van Bulck
2023-08-25 13:32 ` [PATCH v4 13/13] selftests/sgx: Remove incomplete ABI sanitization code in test enclave Jo Van Bulck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).