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

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

[Sean Christopherson]

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(-)

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

Re: [PATCH for_v22] selftests/x86/sgx: Ensure SECS base (ELRANGE) is naturally aligned

[Jethro Beekman]

[-- Attachment #1: Type: text/plain, Size: 301 bytes --]

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


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 3990 bytes --]

Re: [PATCH for_v22] selftests/x86/sgx: Ensure SECS base (ELRANGE) is naturally aligned

[Jarkko Sakkinen]

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

Re: [PATCH for_v22] selftests/x86/sgx: Ensure SECS base (ELRANGE) is naturally aligned

[Jarkko Sakkinen]

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

Re: [PATCH for_v22] selftests/x86/sgx: Ensure SECS base (ELRANGE) is naturally aligned

[Jarkko Sakkinen]

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