[for_v22] selftests/x86/sgx: Ensure SECS base (ELRANGE) is naturally aligned
diff mbox series

Message ID 20190810003051.15712-1-sean.j.christopherson@intel.com
State New
Headers show
Series
  • [for_v22] selftests/x86/sgx: Ensure SECS base (ELRANGE) is naturally aligned
Related show

Commit Message

Sean Christopherson Aug. 10, 2019, 12:30 a.m. UTC
Manually align ELRANGE now that the kernel doesn't automatically do so
for SGX mappings.  To guarantee mmap() returns a region that can be
naturally aligned, temporarily map 2x the enclave size and do fancy
arithmetic to naturally align the base.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 tools/testing/selftests/x86/sgx/main.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

Comments

Jethro Beekman Aug. 10, 2019, 12:39 a.m. UTC | #1
On 2019-08-09 17:30, Sean Christopherson wrote:
> Manually align ELRANGE now that the kernel doesn't automatically do so
> for SGX mappings.

What was the rationale for moving this out of the kernel? Now every user 
of this API needs to implement it manually.

--
Jethro Beekman | Fortanix
Jarkko Sakkinen Aug. 10, 2019, 11:22 a.m. UTC | #2
On Fri, Aug 09, 2019 at 05:30:51PM -0700, Sean Christopherson wrote:
> Manually align ELRANGE now that the kernel doesn't automatically do so
> for SGX mappings.  To guarantee mmap() returns a region that can be
> naturally aligned, temporarily map 2x the enclave size and do fancy
> arithmetic to naturally align the base.
> 
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>

Thank you.

I fine-tuned the calculations a bit:

area = mmap(NULL, secs->size * 2, PROT_NONE, MAP_SHARED, dev_fd, 0);
if (area == MAP_FAILED) {
	perror("mmap");
	return false;
}

secs->base = ((uint64_t)area + secs->size - 1) & ~(secs->size - 1);

munmap(area, secs->base - (uint64_t)area);
munmap((void *)(secs->base + secs->size),
       (uint64_t)area + secs->size - secs->base);

/Jarkko
Jarkko Sakkinen Aug. 10, 2019, 11:23 a.m. UTC | #3
On Sat, Aug 10, 2019 at 12:39:54AM +0000, Jethro Beekman wrote:
> On 2019-08-09 17:30, Sean Christopherson wrote:
> > Manually align ELRANGE now that the kernel doesn't automatically do so
> > for SGX mappings.
> 
> What was the rationale for moving this out of the kernel? Now every user of
> this API needs to implement it manually.

VMAs are viewports to the enclave, not the enclave. You can even fully
create and initialize an enclave without creating VMAs. Thus, binding
must be a loose one.

/Jarkko
Jarkko Sakkinen Aug. 15, 2019, 12:26 p.m. UTC | #4
On Sat, Aug 10, 2019 at 12:39:54AM +0000, Jethro Beekman wrote:
> On 2019-08-09 17:30, Sean Christopherson wrote:
> > Manually align ELRANGE now that the kernel doesn't automatically do so
> > for SGX mappings.
> 
> What was the rationale for moving this out of the kernel? Now every user of
> this API needs to implement it manually.

The binding is now loose between VMAs and enclave e.g. you can
even create and initialize enclave without even a single mmap()
call.

/Jarkko

Patch
diff mbox series

diff --git a/tools/testing/selftests/x86/sgx/main.c b/tools/testing/selftests/x86/sgx/main.c
index effcdb3380ad..5bbc60cf7f89 100644
--- a/tools/testing/selftests/x86/sgx/main.c
+++ b/tools/testing/selftests/x86/sgx/main.c
@@ -130,7 +130,8 @@  static bool encl_create(int dev_fd, unsigned long bin_size,
 			struct sgx_secs *secs)
 {
 	struct sgx_enclave_create ioc;
-	void *base;
+	uint64_t base;
+	void *basep;
 	int rc;
 
 	memset(secs, 0, sizeof(*secs));
@@ -141,19 +142,27 @@  static bool encl_create(int dev_fd, unsigned long bin_size,
 	for (secs->size = 4096; secs->size < bin_size; )
 		secs->size <<= 1;
 
-	base = mmap(NULL, secs->size, PROT_NONE, MAP_SHARED, dev_fd, 0);
-	if (base == MAP_FAILED) {
+	basep = mmap(NULL, secs->size * 2, PROT_NONE, MAP_SHARED, dev_fd, 0);
+	if (basep == MAP_FAILED) {
 		perror("mmap");
 		return false;
 	}
+	base = (uint64_t)basep;
 
-	secs->base = (uint64_t)base;
+	secs->base = base + secs->size - (base % secs->size);
+
+	if (secs->base != base)
+		munmap(basep, secs->base - base);
+
+	if (secs->base - secs->size != base)
+		munmap((void *)(secs->base + secs->size),
+		       base + secs->size - secs->base);
 
 	ioc.src = (unsigned long)secs;
 	rc = ioctl(dev_fd, SGX_IOC_ENCLAVE_CREATE, &ioc);
 	if (rc) {
 		fprintf(stderr, "ECREATE failed rc=%d, err=%d.\n", rc, errno);
-		munmap(base, secs->size);
+		munmap((void *)secs->base, secs->size);
 		return false;
 	}