* [PATCH v2 1/6] selftests/sgx: Ignore OpenSSL 3.0 deprecated functions warning
[not found] <20220831173829.126661-1-jarkko@kernel.org>
@ 2022-08-31 17:38 ` Jarkko Sakkinen
2022-08-31 17:38 ` [PATCH v2 4/6] selftests/sgx: Add SGX selftest augment_via_eaccept_long Jarkko Sakkinen
` (2 subsequent siblings)
3 siblings, 0 replies; 13+ messages in thread
From: Jarkko Sakkinen @ 2022-08-31 17:38 UTC (permalink / raw)
To: linux-sgx
Cc: Haitao Huang, Vijay Dhanraj, Reinette Chatre, Dave Hansen,
Paul Menzel, Kristen Carlson Accardi, Jarkko Sakkinen, Shuah Khan,
open list:KERNEL SELFTEST FRAMEWORK, open list
From: Kristen Carlson Accardi <kristen@linux.intel.com>
OpenSSL 3.0 deprecates some of the functions used in the SGX
selftests, causing build errors on new distros. For now ignore
the warnings until support for the functions is no longer
available and mark FIXME so that it can be clear this should
be removed at some point.
Signed-off-by: Kristen Carlson Accardi <kristen@linux.intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v2:
- Kept because does not exist in tip/x86/sgx, which is the
maintainer branch for SGX, and is required for selftests.
---
tools/testing/selftests/sgx/sigstruct.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/tools/testing/selftests/sgx/sigstruct.c b/tools/testing/selftests/sgx/sigstruct.c
index 50c5ab1aa6fa..a07896a46364 100644
--- a/tools/testing/selftests/sgx/sigstruct.c
+++ b/tools/testing/selftests/sgx/sigstruct.c
@@ -17,6 +17,12 @@
#include "defines.h"
#include "main.h"
+/*
+ * FIXME: OpenSSL 3.0 has deprecated some functions. For now just ignore
+ * the warnings.
+ */
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+
struct q1q2_ctx {
BN_CTX *bn_ctx;
BIGNUM *m;
--
2.37.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 4/6] selftests/sgx: Add SGX selftest augment_via_eaccept_long
[not found] <20220831173829.126661-1-jarkko@kernel.org>
2022-08-31 17:38 ` [PATCH v2 1/6] selftests/sgx: Ignore OpenSSL 3.0 deprecated functions warning Jarkko Sakkinen
@ 2022-08-31 17:38 ` Jarkko Sakkinen
2022-08-31 20:07 ` Reinette Chatre
2022-08-31 17:38 ` [PATCH v2 5/6] selftests/sgx: retry the ioctls returned with EAGAIN Jarkko Sakkinen
2022-08-31 17:38 ` [PATCH v2 6/6] selftests/sgx: Add a bpftrace script for tracking allocation errors Jarkko Sakkinen
3 siblings, 1 reply; 13+ messages in thread
From: Jarkko Sakkinen @ 2022-08-31 17:38 UTC (permalink / raw)
To: linux-sgx
Cc: Haitao Huang, Vijay Dhanraj, Reinette Chatre, Dave Hansen,
Paul Menzel, Jarkko Sakkinen, Shuah Khan,
open list:KERNEL SELFTEST FRAMEWORK, open list
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.com/
v2:
- Addressed Reinette's feedback:
https://lore.kernel.org/linux-sgx/24bd8e42-ff4e-0090-d9e1-cd81e4807f21@intel.com/
---
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);
--
2.37.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 5/6] selftests/sgx: retry the ioctls returned with EAGAIN
[not found] <20220831173829.126661-1-jarkko@kernel.org>
2022-08-31 17:38 ` [PATCH v2 1/6] selftests/sgx: Ignore OpenSSL 3.0 deprecated functions warning Jarkko Sakkinen
2022-08-31 17:38 ` [PATCH v2 4/6] selftests/sgx: Add SGX selftest augment_via_eaccept_long Jarkko Sakkinen
@ 2022-08-31 17:38 ` Jarkko Sakkinen
2022-08-31 20:08 ` Reinette Chatre
2022-08-31 17:38 ` [PATCH v2 6/6] selftests/sgx: Add a bpftrace script for tracking allocation errors Jarkko Sakkinen
3 siblings, 1 reply; 13+ messages in thread
From: Jarkko Sakkinen @ 2022-08-31 17:38 UTC (permalink / raw)
To: linux-sgx
Cc: Haitao Huang, Vijay Dhanraj, Reinette Chatre, Dave Hansen,
Paul Menzel, Jarkko Sakkinen, Shuah Khan,
open list:KERNEL SELFTEST FRAMEWORK, open list
From: Haitao Huang <haitao.huang@linux.intel.com>
For EMODT and EREMOVE ioctls with a large range, kernel
may not finish in one shot and return EAGAIN error code
and count of bytes of EPC pages on that operations are
finished successfully.
Change the unclobbered_vdso_oversubscribed_remove test
to rerun the ioctls in a loop, updating offset and length
using the byte count returned in each iteration.
Signed-off-by: Haitao Huang <haitao.huang@linux.intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v2:
* Changed branching in EAGAIN condition so that else branch
is not required.
* Addressed Reinette's feedback:
https://lore.kernel.org/linux-sgx/5d19be91-3aef-5cbe-6063-3ff3dbd5572b@intel.com/
---
tools/testing/selftests/sgx/main.c | 42 ++++++++++++++++++++++++------
1 file changed, 34 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c
index c5aa9f323745..f42941da3bbe 100644
--- a/tools/testing/selftests/sgx/main.c
+++ b/tools/testing/selftests/sgx/main.c
@@ -395,6 +395,7 @@ TEST_F_TIMEOUT(enclave, unclobbered_vdso_oversubscribed_remove, 900)
struct encl_segment *heap;
unsigned long total_mem;
int ret, errno_save;
+ unsigned long count;
unsigned long addr;
unsigned long i;
@@ -458,16 +459,30 @@ TEST_F_TIMEOUT(enclave, unclobbered_vdso_oversubscribed_remove, 900)
modt_ioc.offset = heap->offset;
modt_ioc.length = heap->size;
modt_ioc.page_type = SGX_PAGE_TYPE_TRIM;
-
+ count = 0;
TH_LOG("Changing type of %zd bytes to trimmed may take a while ...",
heap->size);
- ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_MODIFY_TYPES, &modt_ioc);
- errno_save = ret == -1 ? errno : 0;
+ do {
+ ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_MODIFY_TYPES, &modt_ioc);
+
+ errno_save = ret == -1 ? errno : 0;
+ if (errno_save != EAGAIN)
+ break;
+
+ EXPECT_EQ(modt_ioc.result, 0);
+
+ count += modt_ioc.count;
+ modt_ioc.offset += modt_ioc.count;
+ modt_ioc.length -= modt_ioc.count;
+ modt_ioc.result = 0;
+ modt_ioc.count = 0;
+ } while (modt_ioc.length != 0);
EXPECT_EQ(ret, 0);
EXPECT_EQ(errno_save, 0);
EXPECT_EQ(modt_ioc.result, 0);
- EXPECT_EQ(modt_ioc.count, heap->size);
+ count += modt_ioc.count;
+ EXPECT_EQ(count, heap->size);
/* EACCEPT all removed pages. */
addr = self->encl.encl_base + heap->offset;
@@ -495,15 +510,26 @@ TEST_F_TIMEOUT(enclave, unclobbered_vdso_oversubscribed_remove, 900)
remove_ioc.offset = heap->offset;
remove_ioc.length = heap->size;
-
+ count = 0;
TH_LOG("Removing %zd bytes from enclave may take a while ...",
heap->size);
- ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_REMOVE_PAGES, &remove_ioc);
- errno_save = ret == -1 ? errno : 0;
+ do {
+ ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_REMOVE_PAGES, &remove_ioc);
+
+ errno_save = ret == -1 ? errno : 0;
+ if (errno_save != EAGAIN)
+ break;
+
+ count += remove_ioc.count;
+ remove_ioc.offset += remove_ioc.count;
+ remove_ioc.length -= remove_ioc.count;
+ remove_ioc.count = 0;
+ } while (remove_ioc.length != 0);
EXPECT_EQ(ret, 0);
EXPECT_EQ(errno_save, 0);
- EXPECT_EQ(remove_ioc.count, heap->size);
+ count += remove_ioc.count;
+ EXPECT_EQ(count, heap->size);
}
TEST_F(enclave, clobbered_vdso)
--
2.37.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 6/6] selftests/sgx: Add a bpftrace script for tracking allocation errors
[not found] <20220831173829.126661-1-jarkko@kernel.org>
` (2 preceding siblings ...)
2022-08-31 17:38 ` [PATCH v2 5/6] selftests/sgx: retry the ioctls returned with EAGAIN Jarkko Sakkinen
@ 2022-08-31 17:38 ` Jarkko Sakkinen
2022-08-31 20:09 ` Reinette Chatre
3 siblings, 1 reply; 13+ messages in thread
From: Jarkko Sakkinen @ 2022-08-31 17:38 UTC (permalink / raw)
To: linux-sgx
Cc: Haitao Huang, Vijay Dhanraj, Reinette Chatre, Dave Hansen,
Paul Menzel, Jarkko Sakkinen, Shuah Khan, open list,
open list:KERNEL SELFTEST FRAMEWORK
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v2:
* Added comments.
---
tools/testing/selftests/sgx/alloc-error.bt | 9 +++++++++
1 file changed, 9 insertions(+)
create mode 100644 tools/testing/selftests/sgx/alloc-error.bt
diff --git a/tools/testing/selftests/sgx/alloc-error.bt b/tools/testing/selftests/sgx/alloc-error.bt
new file mode 100644
index 000000000000..0cc8b2e41852
--- /dev/null
+++ b/tools/testing/selftests/sgx/alloc-error.bt
@@ -0,0 +1,9 @@
+/* EPC allocation */
+kr:sgx_alloc_epc_page /(uint64)retval >= (uint64)(-4095)/ {
+ printf("sgx_alloc_epc_page: retval=%d\n", (int64)retval);
+}
+
+/* kzalloc for struct sgx_encl_page */
+kr:sgx_encl_page_alloc /(uint64)retval >= (uint64)(-4095)/ {
+ printf("sgx_encl_page_alloc: retval=%d\n", (int64)retval);
+}
--
2.37.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/6] selftests/sgx: Add SGX selftest augment_via_eaccept_long
2022-08-31 17:38 ` [PATCH v2 4/6] selftests/sgx: Add SGX selftest augment_via_eaccept_long Jarkko Sakkinen
@ 2022-08-31 20:07 ` Reinette Chatre
2022-09-01 22:22 ` Jarkko Sakkinen
0 siblings, 1 reply; 13+ messages in thread
From: Reinette Chatre @ 2022-08-31 20:07 UTC (permalink / raw)
To: Jarkko Sakkinen, linux-sgx
Cc: Haitao Huang, Vijay Dhanraj, Dave Hansen, Paul Menzel, Shuah Khan,
open list:KERNEL SELFTEST FRAMEWORK, open list
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.com/
> v2:
> - Addressed Reinette's feedback:
> https://lore.kernel.org/linux-sgx/24bd8e42-ff4e-0090-d9e1-cd81e4807f21@intel.com/
> ---
> 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 5/6] selftests/sgx: retry the ioctls returned with EAGAIN
2022-08-31 17:38 ` [PATCH v2 5/6] selftests/sgx: retry the ioctls returned with EAGAIN Jarkko Sakkinen
@ 2022-08-31 20:08 ` Reinette Chatre
0 siblings, 0 replies; 13+ messages in thread
From: Reinette Chatre @ 2022-08-31 20:08 UTC (permalink / raw)
To: Jarkko Sakkinen, linux-sgx
Cc: Haitao Huang, Vijay Dhanraj, Dave Hansen, Paul Menzel, Shuah Khan,
open list:KERNEL SELFTEST FRAMEWORK, open list
Hi Jarkko,
On 8/31/2022 10:38 AM, Jarkko Sakkinen wrote:
> @@ -458,16 +459,30 @@ TEST_F_TIMEOUT(enclave, unclobbered_vdso_oversubscribed_remove, 900)
> modt_ioc.offset = heap->offset;
> modt_ioc.length = heap->size;
> modt_ioc.page_type = SGX_PAGE_TYPE_TRIM;
> -
> + count = 0;
> TH_LOG("Changing type of %zd bytes to trimmed may take a while ...",
> heap->size);
> - ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_MODIFY_TYPES, &modt_ioc);
> - errno_save = ret == -1 ? errno : 0;
> + do {
> + ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_MODIFY_TYPES, &modt_ioc);
> +
> + errno_save = ret == -1 ? errno : 0;
> + if (errno_save != EAGAIN)
> + break;
> +
> + EXPECT_EQ(modt_ioc.result, 0);
> +
> + count += modt_ioc.count;
> + modt_ioc.offset += modt_ioc.count;
> + modt_ioc.length -= modt_ioc.count;
> + modt_ioc.result = 0;
From the discussion in V1 it seemed that you were planning to add a check
on the result value instead of just overwriting it. Are you still planning to
do this?
Reinette
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 6/6] selftests/sgx: Add a bpftrace script for tracking allocation errors
2022-08-31 17:38 ` [PATCH v2 6/6] selftests/sgx: Add a bpftrace script for tracking allocation errors Jarkko Sakkinen
@ 2022-08-31 20:09 ` Reinette Chatre
2022-09-01 22:24 ` Jarkko Sakkinen
0 siblings, 1 reply; 13+ messages in thread
From: Reinette Chatre @ 2022-08-31 20:09 UTC (permalink / raw)
To: Jarkko Sakkinen, linux-sgx
Cc: Haitao Huang, Vijay Dhanraj, Dave Hansen, Paul Menzel, Shuah Khan,
open list, open list:KERNEL SELFTEST FRAMEWORK
Hi Jarkko,
On 8/31/2022 10:38 AM, Jarkko Sakkinen wrote:
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> v2:
> * Added comments.
> ---
> tools/testing/selftests/sgx/alloc-error.bt | 9 +++++++++
> 1 file changed, 9 insertions(+)
> create mode 100644 tools/testing/selftests/sgx/alloc-error.bt
>
> diff --git a/tools/testing/selftests/sgx/alloc-error.bt b/tools/testing/selftests/sgx/alloc-error.bt
> new file mode 100644
> index 000000000000..0cc8b2e41852
> --- /dev/null
> +++ b/tools/testing/selftests/sgx/alloc-error.bt
> @@ -0,0 +1,9 @@
> +/* EPC allocation */
> +kr:sgx_alloc_epc_page /(uint64)retval >= (uint64)(-4095)/ {
> + printf("sgx_alloc_epc_page: retval=%d\n", (int64)retval);
> +}
> +
> +/* kzalloc for struct sgx_encl_page */
> +kr:sgx_encl_page_alloc /(uint64)retval >= (uint64)(-4095)/ {
> + printf("sgx_encl_page_alloc: retval=%d\n", (int64)retval);
> +}
I did see you response in [1]. I continue to find this
addition very cryptic in that it assumes global familiarity
with BPF scripting which I do not think can be assumed. I
still think this script can benefit from a comment to describe
what the script does and how to use it.
Reinette
[1] https://lore.kernel.org/linux-sgx/Yw+nCBVYfueEXVZK@kernel.org/
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/6] selftests/sgx: Add SGX selftest augment_via_eaccept_long
2022-08-31 20:07 ` Reinette Chatre
@ 2022-09-01 22:22 ` Jarkko Sakkinen
2022-09-01 23:12 ` Reinette Chatre
2022-09-04 4:02 ` Jarkko Sakkinen
0 siblings, 2 replies; 13+ messages in thread
From: Jarkko Sakkinen @ 2022-09-01 22:22 UTC (permalink / raw)
To: Reinette Chatre
Cc: linux-sgx, Haitao Huang, Vijay Dhanraj, Dave Hansen, Paul Menzel,
Shuah Khan, open list:KERNEL SELFTEST FRAMEWORK, open list
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.com/
> > v2:
> > - Addressed Reinette's feedback:
> > https://lore.kernel.org/linux-sgx/24bd8e42-ff4e-0090-d9e1-cd81e4807f21@intel.com/
> > ---
> > 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 6/6] selftests/sgx: Add a bpftrace script for tracking allocation errors
2022-08-31 20:09 ` Reinette Chatre
@ 2022-09-01 22:24 ` Jarkko Sakkinen
0 siblings, 0 replies; 13+ messages in thread
From: Jarkko Sakkinen @ 2022-09-01 22:24 UTC (permalink / raw)
To: Reinette Chatre
Cc: linux-sgx, Haitao Huang, Vijay Dhanraj, Dave Hansen, Paul Menzel,
Shuah Khan, open list, open list:KERNEL SELFTEST FRAMEWORK
On Wed, Aug 31, 2022 at 01:09:21PM -0700, Reinette Chatre wrote:
> Hi Jarkko,
>
> On 8/31/2022 10:38 AM, Jarkko Sakkinen wrote:
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > ---
> > v2:
> > * Added comments.
> > ---
> > tools/testing/selftests/sgx/alloc-error.bt | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> > create mode 100644 tools/testing/selftests/sgx/alloc-error.bt
> >
> > diff --git a/tools/testing/selftests/sgx/alloc-error.bt b/tools/testing/selftests/sgx/alloc-error.bt
> > new file mode 100644
> > index 000000000000..0cc8b2e41852
> > --- /dev/null
> > +++ b/tools/testing/selftests/sgx/alloc-error.bt
> > @@ -0,0 +1,9 @@
> > +/* EPC allocation */
> > +kr:sgx_alloc_epc_page /(uint64)retval >= (uint64)(-4095)/ {
> > + printf("sgx_alloc_epc_page: retval=%d\n", (int64)retval);
> > +}
> > +
> > +/* kzalloc for struct sgx_encl_page */
> > +kr:sgx_encl_page_alloc /(uint64)retval >= (uint64)(-4095)/ {
> > + printf("sgx_encl_page_alloc: retval=%d\n", (int64)retval);
> > +}
>
> I did see you response in [1]. I continue to find this
> addition very cryptic in that it assumes global familiarity
> with BPF scripting which I do not think can be assumed. I
> still think this script can benefit from a comment to describe
> what the script does and how to use it.
I think I follow Dave's advice to use perf here. I'll
make a header which explains what the script does.
BR, Jarkko
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/6] selftests/sgx: Add SGX selftest augment_via_eaccept_long
2022-09-01 22:22 ` Jarkko Sakkinen
@ 2022-09-01 23:12 ` Reinette Chatre
2022-09-02 0:03 ` Jarkko Sakkinen
2022-09-04 4:02 ` Jarkko Sakkinen
1 sibling, 1 reply; 13+ messages in thread
From: Reinette Chatre @ 2022-09-01 23:12 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-sgx, Haitao Huang, Vijay Dhanraj, Dave Hansen, Paul Menzel,
Shuah Khan, open list:KERNEL SELFTEST FRAMEWORK, open list
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/6] selftests/sgx: Add SGX selftest augment_via_eaccept_long
2022-09-01 23:12 ` Reinette Chatre
@ 2022-09-02 0:03 ` Jarkko Sakkinen
0 siblings, 0 replies; 13+ messages in thread
From: Jarkko Sakkinen @ 2022-09-02 0:03 UTC (permalink / raw)
To: Reinette Chatre
Cc: linux-sgx, Haitao Huang, Vijay Dhanraj, Dave Hansen, Paul Menzel,
Shuah Khan, open list:KERNEL SELFTEST FRAMEWORK, open list
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/6] selftests/sgx: Add SGX selftest augment_via_eaccept_long
2022-09-01 22:22 ` Jarkko Sakkinen
2022-09-01 23:12 ` Reinette Chatre
@ 2022-09-04 4:02 ` Jarkko Sakkinen
2022-09-04 4:21 ` Jarkko Sakkinen
1 sibling, 1 reply; 13+ messages in thread
From: Jarkko Sakkinen @ 2022-09-04 4:02 UTC (permalink / raw)
To: Reinette Chatre
Cc: linux-sgx, Haitao Huang, Vijay Dhanraj, Dave Hansen, Paul Menzel,
Shuah Khan, open list:KERNEL SELFTEST FRAMEWORK, open list
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/6] selftests/sgx: Add SGX selftest augment_via_eaccept_long
2022-09-04 4:02 ` Jarkko Sakkinen
@ 2022-09-04 4:21 ` Jarkko Sakkinen
0 siblings, 0 replies; 13+ messages in thread
From: Jarkko Sakkinen @ 2022-09-04 4:21 UTC (permalink / raw)
To: Reinette Chatre
Cc: linux-sgx, Haitao Huang, Vijay Dhanraj, Dave Hansen, Paul Menzel,
Shuah Khan, open list:KERNEL SELFTEST FRAMEWORK, open list
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
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2022-09-04 4:21 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20220831173829.126661-1-jarkko@kernel.org>
2022-08-31 17:38 ` [PATCH v2 1/6] selftests/sgx: Ignore OpenSSL 3.0 deprecated functions warning Jarkko Sakkinen
2022-08-31 17:38 ` [PATCH v2 4/6] selftests/sgx: Add SGX selftest augment_via_eaccept_long Jarkko Sakkinen
2022-08-31 20:07 ` Reinette Chatre
2022-09-01 22:22 ` Jarkko Sakkinen
2022-09-01 23:12 ` Reinette Chatre
2022-09-02 0:03 ` Jarkko Sakkinen
2022-09-04 4:02 ` Jarkko Sakkinen
2022-09-04 4:21 ` Jarkko Sakkinen
2022-08-31 17:38 ` [PATCH v2 5/6] selftests/sgx: retry the ioctls returned with EAGAIN Jarkko Sakkinen
2022-08-31 20:08 ` Reinette Chatre
2022-08-31 17:38 ` [PATCH v2 6/6] selftests/sgx: Add a bpftrace script for tracking allocation errors Jarkko Sakkinen
2022-08-31 20:09 ` Reinette Chatre
2022-09-01 22:24 ` Jarkko Sakkinen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox