From: Vijay Dhanraj vijay.dhanraj@intel.com
Add a new test case which is same as augment_via_eaccept but adds a larger number of EPC pages to stress test EAUG via EACCEPT.
Signed-off-by: Vijay Dhanraj vijay.dhanraj@intel.com Co-developed-by: Jarkko Sakkinen jarkko@kernel.org Signed-off-by: Jarkko Sakkinen jarkko@kernel.org --- v3: - Addressed Reinette's feedback: https://lore.kernel.org/linux-sgx/bd5285dd-d6dd-8a46-fca9-728db5e2f369@intel... v2: - Addressed Reinette's feedback: https://lore.kernel.org/linux-sgx/24bd8e42-ff4e-0090-d9e1-cd81e4807f21@intel... --- tools/testing/selftests/sgx/load.c | 5 +- tools/testing/selftests/sgx/main.c | 143 +++++++++++++++++++++++++---- tools/testing/selftests/sgx/main.h | 3 +- 3 files changed, 130 insertions(+), 21 deletions(-)
diff --git a/tools/testing/selftests/sgx/load.c b/tools/testing/selftests/sgx/load.c index 94bdeac1cf04..47b2786d6a77 100644 --- a/tools/testing/selftests/sgx/load.c +++ b/tools/testing/selftests/sgx/load.c @@ -171,7 +171,8 @@ uint64_t encl_get_entry(struct encl *encl, const char *symbol) return 0; }
-bool encl_load(const char *path, struct encl *encl, unsigned long heap_size) +bool encl_load(const char *path, struct encl *encl, unsigned long heap_size, + unsigned long edmm_size) { const char device_path[] = "/dev/sgx_enclave"; struct encl_segment *seg; @@ -300,7 +301,7 @@ bool encl_load(const char *path, struct encl *encl, unsigned long heap_size)
encl->src_size = encl->segment_tbl[j].offset + encl->segment_tbl[j].size;
- for (encl->encl_size = 4096; encl->encl_size < encl->src_size; ) + for (encl->encl_size = 4096; encl->encl_size < encl->src_size + edmm_size;) encl->encl_size <<= 1;
return true; diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c index 9820b3809c69..c5aa9f323745 100644 --- a/tools/testing/selftests/sgx/main.c +++ b/tools/testing/selftests/sgx/main.c @@ -23,6 +23,10 @@
static const uint64_t MAGIC = 0x1122334455667788ULL; static const uint64_t MAGIC2 = 0x8877665544332211ULL; +/* Message-ID: DM8PR11MB55912A7F47A84EC9913A6352F6999@DM8PR11MB5591.namprd11.prod.outlook.com */ +static const uint64_t EDMM_SIZE_LONG = 8L * 1024L * 1024L * 1024L; +static const uint64_t TIMEOUT_LONG = 900; /* seconds */ + vdso_sgx_enter_enclave_t vdso_sgx_enter_enclave;
/* @@ -173,7 +177,8 @@ FIXTURE(enclave) { };
static bool setup_test_encl(unsigned long heap_size, struct encl *encl, - struct __test_metadata *_metadata) + struct __test_metadata *_metadata, + unsigned long edmm_size) { Elf64_Sym *sgx_enter_enclave_sym = NULL; struct vdso_symtab symtab; @@ -183,7 +188,7 @@ static bool setup_test_encl(unsigned long heap_size, struct encl *encl, unsigned int i; void *addr;
- if (!encl_load("test_encl.elf", encl, heap_size)) { + if (!encl_load("test_encl.elf", encl, heap_size, edmm_size)) { encl_delete(encl); TH_LOG("Failed to load the test enclave."); return false; @@ -284,7 +289,7 @@ TEST_F(enclave, unclobbered_vdso) struct encl_op_get_from_buf get_op; struct encl_op_put_to_buf put_op;
- ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata)); + ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata, 0));
memset(&self->run, 0, sizeof(self->run)); self->run.tcs = self->encl.encl_base; @@ -357,7 +362,7 @@ TEST_F(enclave, unclobbered_vdso_oversubscribed)
total_mem = get_total_epc_mem(); ASSERT_NE(total_mem, 0); - ASSERT_TRUE(setup_test_encl(total_mem, &self->encl, _metadata)); + ASSERT_TRUE(setup_test_encl(total_mem, &self->encl, _metadata, 0));
memset(&self->run, 0, sizeof(self->run)); self->run.tcs = self->encl.encl_base; @@ -401,7 +406,7 @@ TEST_F_TIMEOUT(enclave, unclobbered_vdso_oversubscribed_remove, 900) ASSERT_NE(total_mem, 0); TH_LOG("Creating an enclave with %lu bytes heap may take a while ...", total_mem); - ASSERT_TRUE(setup_test_encl(total_mem, &self->encl, _metadata)); + ASSERT_TRUE(setup_test_encl(total_mem, &self->encl, _metadata, 0));
/* * Hardware (SGX2) and kernel support is needed for this test. Start @@ -506,7 +511,7 @@ TEST_F(enclave, clobbered_vdso) struct encl_op_get_from_buf get_op; struct encl_op_put_to_buf put_op;
- ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata)); + ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata, 0));
memset(&self->run, 0, sizeof(self->run)); self->run.tcs = self->encl.encl_base; @@ -542,7 +547,7 @@ TEST_F(enclave, clobbered_vdso_and_user_function) struct encl_op_get_from_buf get_op; struct encl_op_put_to_buf put_op;
- ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata)); + ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata, 0));
memset(&self->run, 0, sizeof(self->run)); self->run.tcs = self->encl.encl_base; @@ -575,7 +580,7 @@ TEST_F(enclave, tcs_entry) { struct encl_op_header op;
- ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata)); + ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata, 0));
memset(&self->run, 0, sizeof(self->run)); self->run.tcs = self->encl.encl_base; @@ -620,7 +625,7 @@ TEST_F(enclave, pte_permissions) unsigned long data_start; int ret;
- ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata)); + ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata, 0));
memset(&self->run, 0, sizeof(self->run)); self->run.tcs = self->encl.encl_base; @@ -722,7 +727,7 @@ TEST_F(enclave, tcs_permissions) struct sgx_enclave_restrict_permissions ioc; int ret, errno_save;
- ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata)); + ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata, 0));
memset(&self->run, 0, sizeof(self->run)); self->run.tcs = self->encl.encl_base; @@ -785,7 +790,7 @@ TEST_F(enclave, epcm_permissions) unsigned long data_start; int ret, errno_save;
- ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata)); + ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata, 0));
memset(&self->run, 0, sizeof(self->run)); self->run.tcs = self->encl.encl_base; @@ -986,7 +991,7 @@ TEST_F(enclave, augment) if (!sgx2_supported()) SKIP(return, "SGX2 not supported");
- ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata)); + ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata, 0));
memset(&self->run, 0, sizeof(self->run)); self->run.tcs = self->encl.encl_base; @@ -1116,7 +1121,7 @@ TEST_F(enclave, augment_via_eaccept) if (!sgx2_supported()) SKIP(return, "SGX2 not supported");
- ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata)); + ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata, 0));
memset(&self->run, 0, sizeof(self->run)); self->run.tcs = self->encl.encl_base; @@ -1210,6 +1215,108 @@ TEST_F(enclave, augment_via_eaccept) munmap(addr, PAGE_SIZE); }
+/* + * Test for the addition of large number of pages to an initialized enclave via + * a pre-emptive run of EACCEPT on every page to be added. + */ +TEST_F_TIMEOUT(enclave, augment_via_eaccept_long, TIMEOUT_LONG) +{ + struct encl_op_get_from_addr get_addr_op; + struct encl_op_put_to_addr put_addr_op; + struct encl_op_eaccept eaccept_op; + size_t total_size = 0; + unsigned long i; + void *addr; + + if (!sgx2_supported()) + SKIP(return, "SGX2 not supported"); + + ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata, + EDMM_SIZE_LONG)); + + memset(&self->run, 0, sizeof(self->run)); + self->run.tcs = self->encl.encl_base; + + for (i = 0; i < self->encl.nr_segments; i++) { + struct encl_segment *seg = &self->encl.segment_tbl[i]; + + total_size += seg->size; + } + + /* + * mmap() every page at end of existing enclave to be used for + * EDMM. + */ + addr = mmap((void *)self->encl.encl_base + total_size, EDMM_SIZE_LONG, + PROT_READ | PROT_WRITE | PROT_EXEC, MAP_SHARED | MAP_FIXED, + self->encl.fd, 0); + EXPECT_NE(addr, MAP_FAILED); + + self->run.exception_vector = 0; + self->run.exception_error_code = 0; + self->run.exception_addr = 0; + + /* + * Run EACCEPT on every page to trigger the #PF->EAUG->EACCEPT(again + * without a #PF). All should be transparent to userspace. + */ + eaccept_op.flags = SGX_SECINFO_R | SGX_SECINFO_W | SGX_SECINFO_REG | SGX_SECINFO_PENDING; + eaccept_op.ret = 0; + eaccept_op.header.type = ENCL_OP_EACCEPT; + + for (i = 0; i < EDMM_SIZE_LONG; i += 4096) { + eaccept_op.epc_addr = (uint64_t)(addr + i); + + EXPECT_EQ(ENCL_CALL(&eaccept_op, &self->run, true), 0); + if (self->run.exception_vector == 14 && + self->run.exception_error_code == 4 && + self->run.exception_addr == self->encl.encl_base) { + munmap(addr, EDMM_SIZE_LONG); + SKIP(return, "Kernel does not support adding pages to initialized enclave"); + } + + EXPECT_EQ(self->run.exception_vector, 0); + EXPECT_EQ(self->run.exception_error_code, 0); + EXPECT_EQ(self->run.exception_addr, 0); + ASSERT_EQ(eaccept_op.ret, 0); + ASSERT_EQ(self->run.function, EEXIT); + } + + /* + * Pool of pages were successfully added to enclave. Perform sanity + * check on first page of the pool only to ensure data can be written + * to and read from a dynamically added enclave page. + */ + put_addr_op.value = MAGIC; + put_addr_op.addr = (unsigned long)addr; + put_addr_op.header.type = ENCL_OP_PUT_TO_ADDRESS; + + EXPECT_EQ(ENCL_CALL(&put_addr_op, &self->run, true), 0); + + EXPECT_EEXIT(&self->run); + EXPECT_EQ(self->run.exception_vector, 0); + EXPECT_EQ(self->run.exception_error_code, 0); + EXPECT_EQ(self->run.exception_addr, 0); + + /* + * Read memory from newly added page that was just written to, + * confirming that data previously written (MAGIC) is present. + */ + get_addr_op.value = 0; + get_addr_op.addr = (unsigned long)addr; + get_addr_op.header.type = ENCL_OP_GET_FROM_ADDRESS; + + EXPECT_EQ(ENCL_CALL(&get_addr_op, &self->run, true), 0); + + EXPECT_EQ(get_addr_op.value, MAGIC); + EXPECT_EEXIT(&self->run); + EXPECT_EQ(self->run.exception_vector, 0); + EXPECT_EQ(self->run.exception_error_code, 0); + EXPECT_EQ(self->run.exception_addr, 0); + + munmap(addr, EDMM_SIZE_LONG); +} + /* * SGX2 page type modification test in two phases: * Phase 1: @@ -1238,7 +1345,7 @@ TEST_F(enclave, tcs_create) int ret, i;
ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, - _metadata)); + _metadata, 0));
memset(&self->run, 0, sizeof(self->run)); self->run.tcs = self->encl.encl_base; @@ -1568,7 +1675,7 @@ TEST_F(enclave, remove_added_page_no_eaccept) unsigned long data_start; int ret, errno_save;
- ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata)); + ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata, 0));
memset(&self->run, 0, sizeof(self->run)); self->run.tcs = self->encl.encl_base; @@ -1679,7 +1786,7 @@ TEST_F(enclave, remove_added_page_invalid_access) unsigned long data_start; int ret, errno_save;
- ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata)); + ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata, 0));
memset(&self->run, 0, sizeof(self->run)); self->run.tcs = self->encl.encl_base; @@ -1794,7 +1901,7 @@ TEST_F(enclave, remove_added_page_invalid_access_after_eaccept) unsigned long data_start; int ret, errno_save;
- ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata)); + ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata, 0));
memset(&self->run, 0, sizeof(self->run)); self->run.tcs = self->encl.encl_base; @@ -1918,7 +2025,7 @@ TEST_F(enclave, remove_untouched_page) unsigned long data_start; int ret, errno_save;
- ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata)); + ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata, 0));
/* * Hardware (SGX2) and kernel support is needed for this test. Start diff --git a/tools/testing/selftests/sgx/main.h b/tools/testing/selftests/sgx/main.h index fc585be97e2f..5c190e21a5ff 100644 --- a/tools/testing/selftests/sgx/main.h +++ b/tools/testing/selftests/sgx/main.h @@ -35,7 +35,8 @@ extern unsigned char sign_key[]; extern unsigned char sign_key_end[];
void encl_delete(struct encl *ctx); -bool encl_load(const char *path, struct encl *encl, unsigned long heap_size); +bool encl_load(const char *path, struct encl *encl, unsigned long heap_size, + unsigned long edmm_size); bool encl_measure(struct encl *encl); bool encl_build(struct encl *encl); uint64_t encl_get_entry(struct encl *encl, const char *symbol);
Hi Jarkko,
On 8/31/2022 10:38 AM, Jarkko Sakkinen wrote:
From: Vijay Dhanraj vijay.dhanraj@intel.com
Add a new test case which is same as augment_via_eaccept but adds a larger number of EPC pages to stress test EAUG via EACCEPT.
Signed-off-by: Vijay Dhanraj vijay.dhanraj@intel.com Co-developed-by: Jarkko Sakkinen jarkko@kernel.org Signed-off-by: Jarkko Sakkinen jarkko@kernel.org
v3:
- Addressed Reinette's feedback: https://lore.kernel.org/linux-sgx/bd5285dd-d6dd-8a46-fca9-728db5e2f369@intel...
v2:
- Addressed Reinette's feedback: https://lore.kernel.org/linux-sgx/24bd8e42-ff4e-0090-d9e1-cd81e4807f21@intel...
tools/testing/selftests/sgx/load.c | 5 +- tools/testing/selftests/sgx/main.c | 143 +++++++++++++++++++++++++---- tools/testing/selftests/sgx/main.h | 3 +-
Is this test passing on your system? This version is missing the change to mrenclave_ecreate() that causes SGX_IOC_ENCLAVE_INIT to fail when I try it out.
3 files changed, 130 insertions(+), 21 deletions(-)
diff --git a/tools/testing/selftests/sgx/load.c b/tools/testing/selftests/sgx/load.c index 94bdeac1cf04..47b2786d6a77 100644 --- a/tools/testing/selftests/sgx/load.c +++ b/tools/testing/selftests/sgx/load.c @@ -171,7 +171,8 @@ uint64_t encl_get_entry(struct encl *encl, const char *symbol) return 0; } -bool encl_load(const char *path, struct encl *encl, unsigned long heap_size) +bool encl_load(const char *path, struct encl *encl, unsigned long heap_size,
unsigned long edmm_size){ const char device_path[] = "/dev/sgx_enclave"; struct encl_segment *seg; @@ -300,7 +301,7 @@ bool encl_load(const char *path, struct encl *encl, unsigned long heap_size) encl->src_size = encl->segment_tbl[j].offset + encl->segment_tbl[j].size;
- for (encl->encl_size = 4096; encl->encl_size < encl->src_size; )
- for (encl->encl_size = 4096; encl->encl_size < encl->src_size + edmm_size;) encl->encl_size <<= 1;
return true; diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c index 9820b3809c69..c5aa9f323745 100644 --- a/tools/testing/selftests/sgx/main.c +++ b/tools/testing/selftests/sgx/main.c @@ -23,6 +23,10 @@ static const uint64_t MAGIC = 0x1122334455667788ULL; static const uint64_t MAGIC2 = 0x8877665544332211ULL; +/* Message-ID: DM8PR11MB55912A7F47A84EC9913A6352F6999@DM8PR11MB5591.namprd11.prod.outlook.com */ +static const uint64_t EDMM_SIZE_LONG = 8L * 1024L * 1024L * 1024L; +static const uint64_t TIMEOUT_LONG = 900; /* seconds */
Apologies if my feedback was vague - I actually think that the comments in V1 added valuable information, it was just the variation in formatting that was distracting.
Reinette
On Wed, Aug 31, 2022 at 01:07:35PM -0700, Reinette Chatre wrote:
Hi Jarkko,
On 8/31/2022 10:38 AM, Jarkko Sakkinen wrote:
From: Vijay Dhanraj vijay.dhanraj@intel.com
Add a new test case which is same as augment_via_eaccept but adds a larger number of EPC pages to stress test EAUG via EACCEPT.
Signed-off-by: Vijay Dhanraj vijay.dhanraj@intel.com Co-developed-by: Jarkko Sakkinen jarkko@kernel.org Signed-off-by: Jarkko Sakkinen jarkko@kernel.org
v3:
- Addressed Reinette's feedback: https://lore.kernel.org/linux-sgx/bd5285dd-d6dd-8a46-fca9-728db5e2f369@intel...
v2:
- Addressed Reinette's feedback: https://lore.kernel.org/linux-sgx/24bd8e42-ff4e-0090-d9e1-cd81e4807f21@intel...
tools/testing/selftests/sgx/load.c | 5 +- tools/testing/selftests/sgx/main.c | 143 +++++++++++++++++++++++++---- tools/testing/selftests/sgx/main.h | 3 +-
Is this test passing on your system? This version is missing the change to mrenclave_ecreate() that causes SGX_IOC_ENCLAVE_INIT to fail when I try it out.
I *did* get a pass in my test machine. Hmm... I'll check if the kernel tree was out-of-sync, which could be the reason.
I do not compile kernel on that machine but have the kernel tree for running selftests. So there is a possiblity for a human error. Thanks for pointing this out.
3 files changed, 130 insertions(+), 21 deletions(-)
diff --git a/tools/testing/selftests/sgx/load.c b/tools/testing/selftests/sgx/load.c index 94bdeac1cf04..47b2786d6a77 100644 --- a/tools/testing/selftests/sgx/load.c +++ b/tools/testing/selftests/sgx/load.c @@ -171,7 +171,8 @@ uint64_t encl_get_entry(struct encl *encl, const char *symbol) return 0; } -bool encl_load(const char *path, struct encl *encl, unsigned long heap_size) +bool encl_load(const char *path, struct encl *encl, unsigned long heap_size,
unsigned long edmm_size){ const char device_path[] = "/dev/sgx_enclave"; struct encl_segment *seg; @@ -300,7 +301,7 @@ bool encl_load(const char *path, struct encl *encl, unsigned long heap_size) encl->src_size = encl->segment_tbl[j].offset + encl->segment_tbl[j].size;
- for (encl->encl_size = 4096; encl->encl_size < encl->src_size; )
- for (encl->encl_size = 4096; encl->encl_size < encl->src_size + edmm_size;) encl->encl_size <<= 1;
return true; diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c index 9820b3809c69..c5aa9f323745 100644 --- a/tools/testing/selftests/sgx/main.c +++ b/tools/testing/selftests/sgx/main.c @@ -23,6 +23,10 @@ static const uint64_t MAGIC = 0x1122334455667788ULL; static const uint64_t MAGIC2 = 0x8877665544332211ULL; +/* Message-ID: DM8PR11MB55912A7F47A84EC9913A6352F6999@DM8PR11MB5591.namprd11.prod.outlook.com */ +static const uint64_t EDMM_SIZE_LONG = 8L * 1024L * 1024L * 1024L; +static const uint64_t TIMEOUT_LONG = 900; /* seconds */
Apologies if my feedback was vague - I actually think that the comments in V1 added valuable information, it was just the variation in formatting that was distracting.
IMHO message ID is pretty good reference. Can you propose how would you redo it to minimize the number of iterations in the series?
Reinette
BR, Jarkko
Hi Jarkko,
On 9/1/2022 3:22 PM, Jarkko Sakkinen wrote:
On Wed, Aug 31, 2022 at 01:07:35PM -0700, Reinette Chatre wrote:
On 8/31/2022 10:38 AM, Jarkko Sakkinen wrote:
...
tools/testing/selftests/sgx/load.c | 5 +- tools/testing/selftests/sgx/main.c | 143 +++++++++++++++++++++++++---- tools/testing/selftests/sgx/main.h | 3 +-
Is this test passing on your system? This version is missing the change to mrenclave_ecreate() that causes SGX_IOC_ENCLAVE_INIT to fail when I try it out.
I *did* get a pass in my test machine. Hmm... I'll check if the kernel tree was out-of-sync, which could be the reason.
I do not compile kernel on that machine but have the kernel tree for running selftests. So there is a possiblity for a human error. Thanks for pointing this out.
On my system I encounter the failure below (V1 of this series did not have this problem):
[SNIP] ok 11 enclave.augment_via_eaccept # RUN enclave.augment_via_eaccept_long ... SGX_IOC_ENCLAVE_INIT failed: Operation not permitted # main.c:236:augment_via_eaccept_long:0x0000000000000000 0x0000000000002000 0x03 # main.c:236:augment_via_eaccept_long:0x0000000000002000 0x0000000000001000 0x05 # main.c:236:augment_via_eaccept_long:0x0000000000003000 0x0000000000006000 0x03 # main.c:236:augment_via_eaccept_long:0x0000000000009000 0x0000000000001000 0x03 # main.c:251:augment_via_eaccept_long:Failed to initialize the test enclave. # main.c:1260:augment_via_eaccept_long:Expected 0 (0) != setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata, EDMM_SIZE_LONG) (0) # augment_via_eaccept_long: Test terminated by assertion # FAIL enclave.augment_via_eaccept_long not ok 12 enclave.augment_via_eaccept_long [SNIP]
...
static const uint64_t MAGIC = 0x1122334455667788ULL; static const uint64_t MAGIC2 = 0x8877665544332211ULL; +/* Message-ID: DM8PR11MB55912A7F47A84EC9913A6352F6999@DM8PR11MB5591.namprd11.prod.outlook.com */ +static const uint64_t EDMM_SIZE_LONG = 8L * 1024L * 1024L * 1024L; +static const uint64_t TIMEOUT_LONG = 900; /* seconds */
Apologies if my feedback was vague - I actually think that the comments in V1 added valuable information, it was just the variation in formatting that was distracting.
IMHO message ID is pretty good reference. Can you propose how would you redo it to minimize the number of iterations in the series?
The message ID is a good reference but it points to an email thread and as used here it is unclear what part of that thread is referred to. What you had in V1 was very helpful so it could be:
/* * The size was chosen based on a bug report: * Message-ID: DM8PR11MB55912A7F47A84EC9913A6352F6999@DM8PR11MB5591.namprd11.prod.outlook.com */
I am not sure about Message-ID vs url. The latter may be more convenient since the user needs to first search which inbox the message-ID belongs to before the message can be accessed. Not a big deal though so I think either works.
Reinette
On Thu, Sep 01, 2022 at 04:12:22PM -0700, Reinette Chatre wrote:
Hi Jarkko,
On 9/1/2022 3:22 PM, Jarkko Sakkinen wrote:
On Wed, Aug 31, 2022 at 01:07:35PM -0700, Reinette Chatre wrote:
On 8/31/2022 10:38 AM, Jarkko Sakkinen wrote:
...
tools/testing/selftests/sgx/load.c | 5 +- tools/testing/selftests/sgx/main.c | 143 +++++++++++++++++++++++++---- tools/testing/selftests/sgx/main.h | 3 +-
Is this test passing on your system? This version is missing the change to mrenclave_ecreate() that causes SGX_IOC_ENCLAVE_INIT to fail when I try it out.
I *did* get a pass in my test machine. Hmm... I'll check if the kernel tree was out-of-sync, which could be the reason.
I do not compile kernel on that machine but have the kernel tree for running selftests. So there is a possiblity for a human error. Thanks for pointing this out.
On my system I encounter the failure below (V1 of this series did not have this problem):
[SNIP] ok 11 enclave.augment_via_eaccept # RUN enclave.augment_via_eaccept_long ... SGX_IOC_ENCLAVE_INIT failed: Operation not permitted # main.c:236:augment_via_eaccept_long:0x0000000000000000 0x0000000000002000 0x03 # main.c:236:augment_via_eaccept_long:0x0000000000002000 0x0000000000001000 0x05 # main.c:236:augment_via_eaccept_long:0x0000000000003000 0x0000000000006000 0x03 # main.c:236:augment_via_eaccept_long:0x0000000000009000 0x0000000000001000 0x03 # main.c:251:augment_via_eaccept_long:Failed to initialize the test enclave. # main.c:1260:augment_via_eaccept_long:Expected 0 (0) != setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata, EDMM_SIZE_LONG) (0) # augment_via_eaccept_long: Test terminated by assertion # FAIL enclave.augment_via_eaccept_long not ok 12 enclave.augment_via_eaccept_long [SNIP]
...
static const uint64_t MAGIC = 0x1122334455667788ULL; static const uint64_t MAGIC2 = 0x8877665544332211ULL; +/* Message-ID: DM8PR11MB55912A7F47A84EC9913A6352F6999@DM8PR11MB5591.namprd11.prod.outlook.com */ +static const uint64_t EDMM_SIZE_LONG = 8L * 1024L * 1024L * 1024L; +static const uint64_t TIMEOUT_LONG = 900; /* seconds */
Apologies if my feedback was vague - I actually think that the comments in V1 added valuable information, it was just the variation in formatting that was distracting.
IMHO message ID is pretty good reference. Can you propose how would you redo it to minimize the number of iterations in the series?
The message ID is a good reference but it points to an email thread and as used here it is unclear what part of that thread is referred to. What you had in V1 was very helpful so it could be:
/*
- The size was chosen based on a bug report:
- Message-ID: DM8PR11MB55912A7F47A84EC9913A6352F6999@DM8PR11MB5591.namprd11.prod.outlook.com
*/
I am not sure about Message-ID vs url. The latter may be more convenient since the user needs to first search which inbox the message-ID belongs to before the message can be accessed. Not a big deal though so I think either works.
This is definitely better, I'll use it. Thanks.
Reinette
BR, Jarkko
On Fri, Sep 02, 2022 at 01:22:59AM +0300, Jarkko Sakkinen wrote:
Is this test passing on your system? This version is missing the change to mrenclave_ecreate() that causes SGX_IOC_ENCLAVE_INIT to fail when I try it out.
I *did* get a pass in my test machine. Hmm... I'll check if the kernel tree was out-of-sync, which could be the reason.
I do not compile kernel on that machine but have the kernel tree for running selftests. So there is a possiblity for a human error. Thanks for pointing this out.
Apparently, v1 and v2 break the encl->src_size calculation: the dynamic heap size is not added.
So, in order to revert sigstruct change:
diff --git a/tools/testing/selftests/sgx/load.c b/tools/testing/selftests/sgx/load.c index 47b2786d6a77..0e4e12e1e3eb 100644 --- a/tools/testing/selftests/sgx/load.c +++ b/tools/testing/selftests/sgx/load.c @@ -172,7 +172,7 @@ uint64_t encl_get_entry(struct encl *encl, const char *symbol) }
bool encl_load(const char *path, struct encl *encl, unsigned long heap_size, - unsigned long edmm_size) + unsigned long dynamic_heap_size) { const char device_path[] = "/dev/sgx_enclave"; struct encl_segment *seg; @@ -299,9 +299,9 @@ bool encl_load(const char *path, struct encl *encl, unsigned long heap_size, if (seg->src == MAP_FAILED) goto err;
- encl->src_size = encl->segment_tbl[j].offset + encl->segment_tbl[j].size; + encl->src_size = encl->segment_tbl[j].offset + encl->segment_tbl[j].size + dynamic_heap_size;
- for (encl->encl_size = 4096; encl->encl_size < encl->src_size + edmm_size;) + for (encl->encl_size = 4096; encl->encl_size < encl->src_size;) encl->encl_size <<= 1;
return true;
BR, Jarkko
On Sun, Sep 04, 2022 at 07:02:08AM +0300, Jarkko Sakkinen wrote:
On Fri, Sep 02, 2022 at 01:22:59AM +0300, Jarkko Sakkinen wrote:
Is this test passing on your system? This version is missing the change to mrenclave_ecreate() that causes SGX_IOC_ENCLAVE_INIT to fail when I try it out.
I *did* get a pass in my test machine. Hmm... I'll check if the kernel tree was out-of-sync, which could be the reason.
I do not compile kernel on that machine but have the kernel tree for running selftests. So there is a possiblity for a human error. Thanks for pointing this out.
Apparently, v1 and v2 break the encl->src_size calculation: the dynamic heap size is not added.
So, in order to revert sigstruct change:
diff --git a/tools/testing/selftests/sgx/load.c b/tools/testing/selftests/sgx/load.c index 47b2786d6a77..0e4e12e1e3eb 100644 --- a/tools/testing/selftests/sgx/load.c +++ b/tools/testing/selftests/sgx/load.c @@ -172,7 +172,7 @@ uint64_t encl_get_entry(struct encl *encl, const char *symbol) }
bool encl_load(const char *path, struct encl *encl, unsigned long heap_size,
unsigned long edmm_size)
unsigned long dynamic_heap_size){ const char device_path[] = "/dev/sgx_enclave"; struct encl_segment *seg; @@ -299,9 +299,9 @@ bool encl_load(const char *path, struct encl *encl, unsigned long heap_size, if (seg->src == MAP_FAILED) goto err;
encl->src_size = encl->segment_tbl[j].offset + encl->segment_tbl[j].size;
encl->src_size = encl->segment_tbl[j].offset + encl->segment_tbl[j].size + dynamic_heap_size;
for (encl->encl_size = 4096; encl->encl_size < encl->src_size + edmm_size;)
for (encl->encl_size = 4096; encl->encl_size < encl->src_size;) encl->encl_size <<= 1;
Actually, it is correct after all how Vijay changed it. We should use the final pre-calculated enclave address range in sigstruct.c. It's the re-calculation of that in sigstruct is a reminiscent of it being a separate command-line utility, instead of calculating the sigstruct on-fly. I.e. there has been sane reasons why it has been like that.
BR, Jarkko
linux-kselftest-mirror@lists.linaro.org
-
Jarkko Sakkinen -
Reinette Chatre