Make the test enclave aware of its protected virtual address range to allow
untrusted pointer argument range checks.
Add a linker symbol for __enclave_base at the start of the enclave binary.
Similar to real-world enclave runtimes, rely on the untrusted loader to
fill in __enclave_size (measured as part of MRENCLAVE), as the final size
of the enclave image is determined during loading.
Signed-off-by: Jo Van Bulck <jo.vanbulck@cs.kuleuven.be>
---
tools/testing/selftests/sgx/load.c | 3 +-
tools/testing/selftests/sgx/main.c | 29 +++++++++++++++++++
tools/testing/selftests/sgx/test_encl.lds | 1 +
.../selftests/sgx/test_encl_bootstrap.S | 17 +++++++++++
4 files changed, 49 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/sgx/load.c b/tools/testing/selftests/sgx/load.c
index 94bdeac1c..968a656a3 100644
--- a/tools/testing/selftests/sgx/load.c
+++ b/tools/testing/selftests/sgx/load.c
@@ -60,7 +60,8 @@ static bool encl_map_bin(const char *path, struct encl *encl)
goto err;
}
- bin = mmap(NULL, sb.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
+ /* NOTE: map read|write to allow __enclave_size to be filled in */
+ bin = mmap(NULL, sb.st_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
if (bin == MAP_FAILED) {
perror("enclave executable mmap()");
goto err;
diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c
index d3c7a39f4..bad963c79 100644
--- a/tools/testing/selftests/sgx/main.c
+++ b/tools/testing/selftests/sgx/main.c
@@ -182,6 +182,7 @@ static bool setup_test_encl(unsigned long heap_size, struct encl *encl,
FILE *maps_file;
unsigned int i;
void *addr;
+ uint64_t encl_size_addr;
if (!encl_load("test_encl.elf", encl, heap_size)) {
encl_delete(encl);
@@ -189,6 +190,16 @@ static bool setup_test_encl(unsigned long heap_size, struct encl *encl,
return false;
}
+ /*
+ * Fill in the expected symbol location with the final size of the
+ * constructed enclave image.
+ */
+ encl_size_addr = encl_get_entry(encl, "__enclave_size");
+ if (encl_size_addr) {
+ encl_size_addr += (uint64_t) encl->src;
+ *((uint64_t *) encl_size_addr) = encl->encl_size;
+ }
+
if (!encl_measure(encl))
goto err;
@@ -307,6 +318,24 @@ TEST_F(enclave, unclobbered_vdso)
EXPECT_EQ(self->run.user_data, 0);
}
+TEST_F(enclave, init_size)
+{
+ struct encl_op_get_from_addr get_addr_op;
+
+ ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata));
+
+ memset(&self->run, 0, sizeof(self->run));
+ self->run.tcs = self->encl.encl_base;
+
+ /* __enclave_size is initialized by loader in measured enclave image */
+ get_addr_op.value = 0;
+ get_addr_op.addr = self->encl.encl_base + encl_get_entry(&self->encl, "__enclave_size");
+ get_addr_op.header.type = ENCL_OP_GET_FROM_ADDRESS;
+ EXPECT_EQ(ENCL_CALL(&get_addr_op, &self->run, false), 0);
+ EXPECT_EEXIT(&self->run);
+ EXPECT_EQ(get_addr_op.value, self->encl.encl_size);
+}
+
TEST_F(enclave, poison_args)
{
struct encl_op_header nop_op;
diff --git a/tools/testing/selftests/sgx/test_encl.lds b/tools/testing/selftests/sgx/test_encl.lds
index a1ec64f7d..ca659db2a 100644
--- a/tools/testing/selftests/sgx/test_encl.lds
+++ b/tools/testing/selftests/sgx/test_encl.lds
@@ -10,6 +10,7 @@ PHDRS
SECTIONS
{
. = 0;
+ __enclave_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 3b69fea61..444a075c0 100644
--- a/tools/testing/selftests/sgx/test_encl_bootstrap.S
+++ b/tools/testing/selftests/sgx/test_encl_bootstrap.S
@@ -98,6 +98,23 @@ encl_entry_core:
mov $4, %rax
enclu
+ .global get_enclave_base
+get_enclave_base:
+ lea __enclave_base(%rip), %rax
+ ret
+
+ .global get_enclave_size
+get_enclave_size:
+ mov __enclave_size(%rip), %rax
+ ret
+
+ # The following 8 bytes (measured as part of MRENCLAVE) will be
+ # filled in by the untrusted loader with the total size of the
+ # loaded enclave.
+ .global __enclave_size
+__enclave_size:
+ .quad 0x0
+
.section ".data", "aw"
encl_ssa_tcs1:
--
2.34.1On Wed Jul 19, 2023 at 5:24 PM EEST, Jo Van Bulck wrote:
> Make the test enclave aware of its protected virtual address range to allow
> untrusted pointer argument range checks.
>
> Add a linker symbol for __enclave_base at the start of the enclave binary.
> Similar to real-world enclave runtimes, rely on the untrusted loader to
> fill in __enclave_size (measured as part of MRENCLAVE), as the final size
> of the enclave image is determined during loading.
>
> Signed-off-by: Jo Van Bulck <jo.vanbulck@cs.kuleuven.be>
> ---
> tools/testing/selftests/sgx/load.c | 3 +-
> tools/testing/selftests/sgx/main.c | 29 +++++++++++++++++++
> tools/testing/selftests/sgx/test_encl.lds | 1 +
> .../selftests/sgx/test_encl_bootstrap.S | 17 +++++++++++
> 4 files changed, 49 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/sgx/load.c b/tools/testing/selftests/sgx/load.c
> index 94bdeac1c..968a656a3 100644
> --- a/tools/testing/selftests/sgx/load.c
> +++ b/tools/testing/selftests/sgx/load.c
> @@ -60,7 +60,8 @@ static bool encl_map_bin(const char *path, struct encl *encl)
> goto err;
> }
>
> - bin = mmap(NULL, sb.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
> + /* NOTE: map read|write to allow __enclave_size to be filled in */
> + bin = mmap(NULL, sb.st_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
> if (bin == MAP_FAILED) {
> perror("enclave executable mmap()");
> goto err;
> diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c
> index d3c7a39f4..bad963c79 100644
> --- a/tools/testing/selftests/sgx/main.c
> +++ b/tools/testing/selftests/sgx/main.c
> @@ -182,6 +182,7 @@ static bool setup_test_encl(unsigned long heap_size, struct encl *encl,
> FILE *maps_file;
> unsigned int i;
> void *addr;
> + uint64_t encl_size_addr;
Should be the first declaration (reverse xmas tree order).
I'd rename this as encl_end, as the current name is cryptic.
>
> if (!encl_load("test_encl.elf", encl, heap_size)) {
> encl_delete(encl);
> @@ -189,6 +190,16 @@ static bool setup_test_encl(unsigned long heap_size, struct encl *encl,
> return false;
> }
>
> + /*
> + * Fill in the expected symbol location with the final size of the
> + * constructed enclave image.
> + */
> + encl_size_addr = encl_get_entry(encl, "__enclave_size");
> + if (encl_size_addr) {
> + encl_size_addr += (uint64_t) encl->src;
> + *((uint64_t *) encl_size_addr) = encl->encl_size;
> + }
> +
> if (!encl_measure(encl))
> goto err;
>
> @@ -307,6 +318,24 @@ TEST_F(enclave, unclobbered_vdso)
> EXPECT_EQ(self->run.user_data, 0);
> }
>
> +TEST_F(enclave, init_size)
> +{
> + struct encl_op_get_from_addr get_addr_op;
> +
> + ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata));
> +
> + memset(&self->run, 0, sizeof(self->run));
> + self->run.tcs = self->encl.encl_base;
> +
> + /* __enclave_size is initialized by loader in measured enclave image */
> + get_addr_op.value = 0;
> + get_addr_op.addr = self->encl.encl_base + encl_get_entry(&self->encl, "__enclave_size");
> + get_addr_op.header.type = ENCL_OP_GET_FROM_ADDRESS;
> + EXPECT_EQ(ENCL_CALL(&get_addr_op, &self->run, false), 0);
> + EXPECT_EEXIT(&self->run);
> + EXPECT_EQ(get_addr_op.value, self->encl.encl_size);
> +}
> +
> TEST_F(enclave, poison_args)
> {
> struct encl_op_header nop_op;
> diff --git a/tools/testing/selftests/sgx/test_encl.lds b/tools/testing/selftests/sgx/test_encl.lds
> index a1ec64f7d..ca659db2a 100644
> --- a/tools/testing/selftests/sgx/test_encl.lds
> +++ b/tools/testing/selftests/sgx/test_encl.lds
> @@ -10,6 +10,7 @@ PHDRS
> SECTIONS
> {
> . = 0;
> + __enclave_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 3b69fea61..444a075c0 100644
> --- a/tools/testing/selftests/sgx/test_encl_bootstrap.S
> +++ b/tools/testing/selftests/sgx/test_encl_bootstrap.S
> @@ -98,6 +98,23 @@ encl_entry_core:
> mov $4, %rax
> enclu
>
> + .global get_enclave_base
> +get_enclave_base:
> + lea __enclave_base(%rip), %rax
> + ret
> +
> + .global get_enclave_size
> +get_enclave_size:
> + mov __enclave_size(%rip), %rax
> + ret
> +
> + # The following 8 bytes (measured as part of MRENCLAVE) will be
> + # filled in by the untrusted loader with the total size of the
> + # loaded enclave.
> + .global __enclave_size
> +__enclave_size:
> + .quad 0x0
> +
> .section ".data", "aw"
>
> encl_ssa_tcs1:
> --
> 2.34.1
BR, Jarkko
On 20.07.23 19:29, Jarkko Sakkinen wrote: >> diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c >> index d3c7a39f4..bad963c79 100644 >> --- a/tools/testing/selftests/sgx/main.c >> +++ b/tools/testing/selftests/sgx/main.c >> @@ -182,6 +1827 @@ static bool setup_test_encl(unsigned long heap_size, struct encl *encl, >> FILE *maps_file; >> unsigned int i; >> void *addr; >> + uint64_t encl_size_addr; > > Should be the first declaration (reverse xmas tree order). Will do > I'd rename this as encl_end, as the current name is cryptic. Thank you, agreed the current name is cryptic. However, encl_end is incorrect as this is the location in the enclave that stores the _size_ and the location itself is also _not_ at the end of the enclave. I'll rename this in the next patch revision to encl_size_pt and make it of type uint64_t*, which seems more logical to me. Let me know if you prefer otherwise! Best, Jo
© 2016 - 2025 Red Hat, Inc.