[PATCH 1/5] selftests/sgx: Add PHDRS to encl.lds

[PATCH 1/5] selftests/sgx: Add PHDRS to encl.lds

[Jarkko Sakkinen]

Improve encl.lds to create an ELF image that can be easily loaded without
needing the conversion to a raw binary. This is achieved by adding PHDRS to
encl.lds that describes the different segments.

With a simple Python program it is easy to see that the changes result in a
sane memory layout [1]:

Flags Start              End
rw-   0x0000000000200000 0x0000000000201000
r-x   0x0000000000201000 0x0000000000202000
rw-   0x0000000000202000 0x0000000000205000

These are the start and end positions in the enclave ELF image for
different enclave memory areas. Since all the sections are marked as being
allocated, an ELF enclave loader can be solely based on p_offset, p_memsz
and p_flags fields of struct Elf64_Phdr.

[1]
import sys
from elftools.elf.elffile import ELFFile

PAGE_SIZE = 0x1000

if __name__ == '__main__':
    flags2str = ['---', '--x', '-w-', '-wx', 'r--', 'r-x', 'rw-', 'rwx']

    if len(sys.argv) != 2:
        sys.exit(1)

    with open(sys.argv[1], 'rb') as file:
        file = ELFFile(file)

        print('{:<5} {:<18} {:<18}'.format('Flags', 'Start', 'End'))

        for seg in file.iter_segments():
            if seg['p_type'] != 'PT_LOAD':
                continue

            flags = flags2str[seg['p_flags']]

            start = seg['p_offset'] & ~(PAGE_SIZE - 1)
            end = start +
	          (seg['p_filesz'] + PAGE_SIZE - 1) & ~(PAGE_SIZE - 1)

            print('{:<5} 0x{:0>16x} 0x{:0>16x}'.format(flags, start, end))

Cc: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 tools/testing/selftests/sgx/encl.lds         | 14 ++++++++++----
 tools/testing/selftests/sgx/encl_bootstrap.S |  2 +-
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/sgx/encl.lds b/tools/testing/selftests/sgx/encl.lds
index 9a56d3064104..0fbbda7e665e 100644
--- a/tools/testing/selftests/sgx/encl.lds
+++ b/tools/testing/selftests/sgx/encl.lds
@@ -1,25 +1,31 @@
 OUTPUT_FORMAT(elf64-x86-64)
 
+PHDRS
+{
+	tcs PT_LOAD;
+	text PT_LOAD;
+	data PT_LOAD;
+}
+
 SECTIONS
 {
 	. = 0;
 	.tcs : {
 		*(.tcs*)
-	}
+	} : tcs
 
 	. = ALIGN(4096);
 	.text : {
 		*(.text*)
 		*(.rodata*)
-	}
+	} : text
 
 	. = ALIGN(4096);
 	.data : {
 		*(.data*)
-	}
+	} : data
 
 	/DISCARD/ : {
-		*(.data*)
 		*(.comment*)
 		*(.note*)
 		*(.debug*)
diff --git a/tools/testing/selftests/sgx/encl_bootstrap.S b/tools/testing/selftests/sgx/encl_bootstrap.S
index 3a1479f1cdcf..b9ea6130e422 100644
--- a/tools/testing/selftests/sgx/encl_bootstrap.S
+++ b/tools/testing/selftests/sgx/encl_bootstrap.S
@@ -7,7 +7,7 @@
 	.byte 0x0f, 0x01, 0xd7
 	.endm
 
-	.section ".tcs", "a"
+	.section ".tcs", "aw"
 	.balign	4096
 
 	.fill	1, 8, 0			# STATE (set by CPU)
-- 
2.25.1

[PATCH 2/5] selftests/sgx: Manage encl_fd in the main function

[Jarkko Sakkinen]

In order to consolidate the enclave resource management to a single place,
consolidate the enclave management to the main function.  Introduce a
struct context to track the resources that are allocated by the test
program.

Cc: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 tools/testing/selftests/sgx/main.c | 116 ++++++++++++++++++-----------
 1 file changed, 72 insertions(+), 44 deletions(-)

diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c
index af16dd6f4b92..f39b783c8def 100644
--- a/tools/testing/selftests/sgx/main.c
+++ b/tools/testing/selftests/sgx/main.c
@@ -194,39 +194,29 @@ static bool encl_add_pages(int dev_fd, unsigned long offset, void *data,
 #define SGX_REG_PAGE_FLAGS \
 	(SGX_SECINFO_REG | SGX_SECINFO_R | SGX_SECINFO_W | SGX_SECINFO_X)
 
-static bool encl_build(struct sgx_secs *secs, void *bin,
+static bool encl_build(int encl_fd, struct sgx_secs *secs, void *bin,
 		       unsigned long bin_size, struct sgx_sigstruct *sigstruct)
 {
 	struct sgx_enclave_init ioc;
 	void *addr;
-	int dev_fd;
 	int rc;
 
-	dev_fd = open("/dev/sgx/enclave", O_RDWR);
-	if (dev_fd < 0) {
-		fprintf(stderr, "Unable to open /dev/sgx\n");
+	if (!encl_add_pages(encl_fd, 0, bin, PAGE_SIZE, SGX_SECINFO_TCS))
 		return false;
-	}
-
-	if (!encl_create(dev_fd, bin_size, secs))
-		goto out_dev_fd;
 
-	if (!encl_add_pages(dev_fd, 0, bin, PAGE_SIZE, SGX_SECINFO_TCS))
-		goto out_dev_fd;
-
-	if (!encl_add_pages(dev_fd, PAGE_SIZE, bin + PAGE_SIZE,
+	if (!encl_add_pages(encl_fd, PAGE_SIZE, bin + PAGE_SIZE,
 			    bin_size - PAGE_SIZE, SGX_REG_PAGE_FLAGS))
-		goto out_dev_fd;
+		return false;
 
 	ioc.sigstruct = (uint64_t)sigstruct;
-	rc = ioctl(dev_fd, SGX_IOC_ENCLAVE_INIT, &ioc);
+	rc = ioctl(encl_fd, SGX_IOC_ENCLAVE_INIT, &ioc);
 	if (rc) {
-		printf("EINIT failed rc=%d\n", rc);
-		goto out_map;
+		fprintf(stderr, "EINIT failed rc=%d\n", rc);
+		return false;
 	}
 
 	addr = mmap((void *)secs->base, PAGE_SIZE, PROT_READ | PROT_WRITE,
-		    MAP_SHARED | MAP_FIXED, dev_fd, 0);
+		    MAP_SHARED | MAP_FIXED, encl_fd, 0);
 	if (addr == MAP_FAILED) {
 		fprintf(stderr, "mmap() failed on TCS, errno=%d.\n", errno);
 		return false;
@@ -234,19 +224,13 @@ static bool encl_build(struct sgx_secs *secs, void *bin,
 
 	addr = mmap((void *)(secs->base + PAGE_SIZE), bin_size - PAGE_SIZE,
 		    PROT_READ | PROT_WRITE | PROT_EXEC,
-		    MAP_SHARED | MAP_FIXED, dev_fd, 0);
+		    MAP_SHARED | MAP_FIXED, encl_fd, 0);
 	if (addr == MAP_FAILED) {
 		fprintf(stderr, "mmap() failed, errno=%d.\n", errno);
 		return false;
 	}
 
-	close(dev_fd);
 	return true;
-out_map:
-	munmap((void *)secs->base, secs->size);
-out_dev_fd:
-	close(dev_fd);
-	return false;
 }
 
 bool get_file_size(const char *path, off_t *bin_size)
@@ -271,6 +255,7 @@ bool get_file_size(const char *path, off_t *bin_size)
 
 bool encl_data_map(const char *path, void **bin, off_t *bin_size)
 {
+	off_t tmp_bin_size;
 	int fd;
 
 	fd = open(path, O_RDONLY);
@@ -279,15 +264,17 @@ bool encl_data_map(const char *path, void **bin, off_t *bin_size)
 		return false;
 	}
 
-	if (!get_file_size(path, bin_size))
+	if (!get_file_size(path, &tmp_bin_size))
 		goto err_out;
 
-	*bin = mmap(NULL, *bin_size, PROT_READ, MAP_PRIVATE, fd, 0);
+	*bin = mmap(NULL, tmp_bin_size, PROT_READ, MAP_PRIVATE, fd, 0);
 	if (*bin == MAP_FAILED) {
 		fprintf(stderr, "mmap() %s failed, errno=%d.\n", path, errno);
 		goto err_out;
 	}
 
+	*bin_size = tmp_bin_size;
+
 	close(fd);
 	return true;
 
@@ -296,48 +283,89 @@ bool encl_data_map(const char *path, void **bin, off_t *bin_size)
 	return false;
 }
 
+struct context {
+	void *bin;
+	off_t bin_size;
+	int encl_fd;
+	struct sgx_secs secs;
+};
+
+static void context_init(struct context *ctx)
+{
+	memset(&ctx, 0, sizeof(ctx));
+}
+
+static void context_delete(struct context *ctx)
+{
+	if (ctx->secs.base)
+		munmap((void *)ctx->secs.base, ctx->secs.size);
+
+	if (ctx->bin)
+		munmap(ctx->bin, ctx->bin_size);
+
+	if (ctx->encl_fd)
+		close(ctx->encl_fd);
+}
+
 int main(int argc, char *argv[], char *envp[])
 {
 	struct sgx_enclave_exception exception;
 	struct sgx_sigstruct sigstruct;
 	struct vdso_symtab symtab;
 	Elf64_Sym *eenter_sym;
-	struct sgx_secs secs;
 	uint64_t result = 0;
-	off_t bin_size;
+	struct context ctx;
 	void *addr;
-	void *bin;
 
-	if (!encl_data_map("encl.bin", &bin, &bin_size))
-		exit(1);
+	context_init(&ctx);
 
-	if (!encl_create_sigstruct(bin, bin_size, &sigstruct))
-		exit(1);
+	ctx.encl_fd = open("/dev/sgx/enclave", O_RDWR);
+	if (ctx.encl_fd < 0) {
+		fprintf(stderr, "Unable to open /dev/sgx\n");
+		goto err;
+	}
 
-	if (!encl_build(&secs, bin, bin_size, &sigstruct))
-		exit(1);
+	if (!encl_data_map("encl.bin", &ctx.bin, &ctx.bin_size))
+		goto err;
+
+	if (!encl_create_sigstruct(ctx.bin, ctx.bin_size, &sigstruct))
+		goto err;
+
+	if (!encl_create(ctx.encl_fd, ctx.bin_size, &ctx.secs))
+		goto err;
+
+	if (!encl_build(ctx.encl_fd, &ctx.secs, ctx.bin, ctx.bin_size,
+			&sigstruct))
+		goto err;
 
 	memset(&exception, 0, sizeof(exception));
 
 	addr = vdso_get_base_addr(envp);
 	if (!addr)
-		exit(1);
+		goto err;
 
 	if (!vdso_get_symtab(addr, &symtab))
-		exit(1);
+		goto err;
 
 	eenter_sym = vdso_symtab_get(&symtab, "__vdso_sgx_enter_enclave");
 	if (!eenter_sym)
-		exit(1);
+		goto err;
+
 	eenter = addr + eenter_sym->st_value;
 
 	sgx_call_vdso((void *)&MAGIC, &result, 0, NULL, NULL, NULL,
-		      (void *)secs.base, &exception, NULL);
-	if (result != MAGIC) {
-		fprintf(stderr, "FAILURE\n");
-		exit(1);
-	}
+		      (void *)ctx.secs.base, &exception, NULL);
+	if (result != MAGIC)
+		goto err;
 
 	printf("SUCCESS\n");
+
+	context_delete(&ctx);
 	exit(0);
+
+err:
+	printf("FAILURE\n");
+
+	context_delete(&ctx);
+	exit(1);
 }
-- 
2.25.1

[PATCH 3/5] selftests/sgx: Move EINIT out of encl_build()

[Jarkko Sakkinen]

Take EINIT out of encl_build() and move SIGSTRUCT generation and EINIT
right after the encl_build() call. No reason to intertie them as they are
fully independent tasks (i.e. could be even done in parallel).

Cc: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 tools/testing/selftests/sgx/main.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c
index f39b783c8def..995423565c83 100644
--- a/tools/testing/selftests/sgx/main.c
+++ b/tools/testing/selftests/sgx/main.c
@@ -195,11 +195,9 @@ static bool encl_add_pages(int dev_fd, unsigned long offset, void *data,
 	(SGX_SECINFO_REG | SGX_SECINFO_R | SGX_SECINFO_W | SGX_SECINFO_X)
 
 static bool encl_build(int encl_fd, struct sgx_secs *secs, void *bin,
-		       unsigned long bin_size, struct sgx_sigstruct *sigstruct)
+		       unsigned long bin_size)
 {
-	struct sgx_enclave_init ioc;
 	void *addr;
-	int rc;
 
 	if (!encl_add_pages(encl_fd, 0, bin, PAGE_SIZE, SGX_SECINFO_TCS))
 		return false;
@@ -208,13 +206,6 @@ static bool encl_build(int encl_fd, struct sgx_secs *secs, void *bin,
 			    bin_size - PAGE_SIZE, SGX_REG_PAGE_FLAGS))
 		return false;
 
-	ioc.sigstruct = (uint64_t)sigstruct;
-	rc = ioctl(encl_fd, SGX_IOC_ENCLAVE_INIT, &ioc);
-	if (rc) {
-		fprintf(stderr, "EINIT failed rc=%d\n", rc);
-		return false;
-	}
-
 	addr = mmap((void *)secs->base, PAGE_SIZE, PROT_READ | PROT_WRITE,
 		    MAP_SHARED | MAP_FIXED, encl_fd, 0);
 	if (addr == MAP_FAILED) {
@@ -311,11 +302,13 @@ int main(int argc, char *argv[], char *envp[])
 {
 	struct sgx_enclave_exception exception;
 	struct sgx_sigstruct sigstruct;
+	struct sgx_enclave_init ioc;
 	struct vdso_symtab symtab;
 	Elf64_Sym *eenter_sym;
 	uint64_t result = 0;
 	struct context ctx;
 	void *addr;
+	int ret;
 
 	context_init(&ctx);
 
@@ -328,15 +321,21 @@ int main(int argc, char *argv[], char *envp[])
 	if (!encl_data_map("encl.bin", &ctx.bin, &ctx.bin_size))
 		goto err;
 
-	if (!encl_create_sigstruct(ctx.bin, ctx.bin_size, &sigstruct))
+	if (!encl_create(ctx.encl_fd, ctx.bin_size, &ctx.secs))
 		goto err;
 
-	if (!encl_create(ctx.encl_fd, ctx.bin_size, &ctx.secs))
+	if (!encl_build(ctx.encl_fd, &ctx.secs, ctx.bin, ctx.bin_size))
+		goto err;
+
+	if (!encl_create_sigstruct(ctx.bin, ctx.bin_size, &sigstruct))
 		goto err;
 
-	if (!encl_build(ctx.encl_fd, &ctx.secs, ctx.bin, ctx.bin_size,
-			&sigstruct))
+	ioc.sigstruct = (uint64_t)&sigstruct;
+	ret = ioctl(ctx.encl_fd, SGX_IOC_ENCLAVE_INIT, &ioc);
+	if (ret) {
+		fprintf(stderr, "EINIT failed ret=%d, errno=%d\n", ret, errno);
 		goto err;
+	}
 
 	memset(&exception, 0, sizeof(exception));
 
-- 
2.25.1

[PATCH 4/5] selftest/sgx: Replace encl_build() with encl_build_segment()

[Jarkko Sakkinen]

Remove encl_build() and introduce encl_build_segment(), which builds and
maps one segment of the enclave with given flags and permissions. This
enables to load segments directly from an ELF files.

Cc: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 tools/testing/selftests/sgx/main.c | 35 ++++++++++++++----------------
 1 file changed, 16 insertions(+), 19 deletions(-)

diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c
index 995423565c83..a78e64159313 100644
--- a/tools/testing/selftests/sgx/main.c
+++ b/tools/testing/selftests/sgx/main.c
@@ -191,30 +191,18 @@ static bool encl_add_pages(int dev_fd, unsigned long offset, void *data,
 	return true;
 }
 
-#define SGX_REG_PAGE_FLAGS \
-	(SGX_SECINFO_REG | SGX_SECINFO_R | SGX_SECINFO_W | SGX_SECINFO_X)
 
-static bool encl_build(int encl_fd, struct sgx_secs *secs, void *bin,
-		       unsigned long bin_size)
+static bool encl_build_segment(int encl_fd, struct sgx_secs *secs, void *bin,
+			       unsigned long seg_offset, unsigned long seg_size,
+			       uint64_t flags, int prot)
 {
 	void *addr;
 
-	if (!encl_add_pages(encl_fd, 0, bin, PAGE_SIZE, SGX_SECINFO_TCS))
+	if (!encl_add_pages(encl_fd, seg_offset, bin + seg_offset, seg_size,
+			    flags))
 		return false;
 
-	if (!encl_add_pages(encl_fd, PAGE_SIZE, bin + PAGE_SIZE,
-			    bin_size - PAGE_SIZE, SGX_REG_PAGE_FLAGS))
-		return false;
-
-	addr = mmap((void *)secs->base, PAGE_SIZE, PROT_READ | PROT_WRITE,
-		    MAP_SHARED | MAP_FIXED, encl_fd, 0);
-	if (addr == MAP_FAILED) {
-		fprintf(stderr, "mmap() failed on TCS, errno=%d.\n", errno);
-		return false;
-	}
-
-	addr = mmap((void *)(secs->base + PAGE_SIZE), bin_size - PAGE_SIZE,
-		    PROT_READ | PROT_WRITE | PROT_EXEC,
+	addr = mmap((void *)secs->base + seg_offset, seg_size, prot,
 		    MAP_SHARED | MAP_FIXED, encl_fd, 0);
 	if (addr == MAP_FAILED) {
 		fprintf(stderr, "mmap() failed, errno=%d.\n", errno);
@@ -324,7 +312,16 @@ int main(int argc, char *argv[], char *envp[])
 	if (!encl_create(ctx.encl_fd, ctx.bin_size, &ctx.secs))
 		goto err;
 
-	if (!encl_build(ctx.encl_fd, &ctx.secs, ctx.bin, ctx.bin_size))
+	/* TCS */
+	if (!encl_build_segment(ctx.encl_fd, &ctx.secs, ctx.bin, 0, PAGE_SIZE,
+				SGX_SECINFO_TCS, PROT_READ | PROT_WRITE))
+		goto err;
+
+	if (!encl_build_segment(ctx.encl_fd, &ctx.secs, ctx.bin, PAGE_SIZE,
+				ctx.bin_size - PAGE_SIZE,
+				SGX_SECINFO_REG | SGX_SECINFO_R |
+				SGX_SECINFO_W | SGX_SECINFO_X,
+				PROT_READ | PROT_WRITE | PROT_EXEC))
 		goto err;
 
 	if (!encl_create_sigstruct(ctx.bin, ctx.bin_size, &sigstruct))
-- 
2.25.1

[PATCH 5/5] selftests/sgx: Load encl.elf directly in the test program

[Jarkko Sakkinen]

To make test program more realistic and robust, load the test enclave
directly from encl.elf.

Cc: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 tools/testing/selftests/sgx/Makefile  | 11 +++---
 tools/testing/selftests/sgx/defines.h |  1 +
 tools/testing/selftests/sgx/main.c    | 48 ++++++++++++++++++++-------
 3 files changed, 41 insertions(+), 19 deletions(-)

diff --git a/tools/testing/selftests/sgx/Makefile b/tools/testing/selftests/sgx/Makefile
index d9c3b3a1983b..48a2cda6c34d 100644
--- a/tools/testing/selftests/sgx/Makefile
+++ b/tools/testing/selftests/sgx/Makefile
@@ -16,7 +16,7 @@ HOST_CFLAGS := -Wall -Werror -g $(INCLUDES) -fPIC -z noexecstack
 ENCL_CFLAGS := -Wall -Werror -static -nostdlib -nostartfiles -fPIC \
 	       -fno-stack-protector -mrdrnd $(INCLUDES)
 
-TEST_CUSTOM_PROGS := $(OUTPUT)/test_sgx $(OUTPUT)/encl.bin
+TEST_CUSTOM_PROGS := $(OUTPUT)/test_sgx $(OUTPUT)/encl.elf
 
 ifeq ($(CAN_BUILD_X86_64), 1)
 all: $(TEST_CUSTOM_PROGS)
@@ -34,16 +34,13 @@ $(OUTPUT)/sign.o: sign.c
 $(OUTPUT)/call.o: call.S
 	$(CC) $(HOST_CFLAGS) -c $< -o $@
 
-$(OUTPUT)/encl.bin: $(OUTPUT)/encl.elf
-	$(OBJCOPY) -O binary $< $@
-
 $(OUTPUT)/encl.elf: encl.lds encl.c encl_bootstrap.S
 	$(CC) $(ENCL_CFLAGS) -T $^ -o $@
 
 EXTRA_CLEAN := \
-	$(OUTPUT)/encl.bin \
 	$(OUTPUT)/encl.elf \
-	$(OUTPUT)/sgx_call.o \
+	$(OUTPUT)/call.o \
+	$(OUTPUT)/main.o \
+	$(OUTPUT)/sign.o \
 	$(OUTPUT)/test_sgx \
 	$(OUTPUT)/test_sgx.o \
-
diff --git a/tools/testing/selftests/sgx/defines.h b/tools/testing/selftests/sgx/defines.h
index 8f4d17cf8cee..1802cace7527 100644
--- a/tools/testing/selftests/sgx/defines.h
+++ b/tools/testing/selftests/sgx/defines.h
@@ -9,6 +9,7 @@
 #include <stdint.h>
 
 #define PAGE_SIZE 4096
+#define PAGE_MASK (~(PAGE_SIZE - 1))
 
 #define __aligned(x) __attribute__((__aligned__(x)))
 #define __packed __attribute__((packed))
diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c
index a78e64159313..a0a37d85714b 100644
--- a/tools/testing/selftests/sgx/main.c
+++ b/tools/testing/selftests/sgx/main.c
@@ -223,11 +223,6 @@ bool get_file_size(const char *path, off_t *bin_size)
 		return false;
 	}
 
-	if (!sb.st_size || sb.st_size & 0xfff) {
-		fprintf(stderr, "Invalid blob size %lu\n", sb.st_size);
-		return false;
-	}
-
 	*bin_size = sb.st_size;
 	return true;
 }
@@ -291,12 +286,17 @@ int main(int argc, char *argv[], char *envp[])
 	struct sgx_enclave_exception exception;
 	struct sgx_sigstruct sigstruct;
 	struct sgx_enclave_init ioc;
+	Elf64_Phdr *phdr, *phdr_tbl;
+	unsigned long start_offset;
 	struct vdso_symtab symtab;
+	unsigned long encl_size;
 	Elf64_Sym *eenter_sym;
 	uint64_t result = 0;
 	struct context ctx;
+	Elf64_Ehdr *ehdr;
 	void *addr;
 	int ret;
+	int i;
 
 	context_init(&ctx);
 
@@ -306,25 +306,49 @@ int main(int argc, char *argv[], char *envp[])
 		goto err;
 	}
 
-	if (!encl_data_map("encl.bin", &ctx.bin, &ctx.bin_size))
+	if (!encl_data_map("encl.elf", &ctx.bin, &ctx.bin_size))
 		goto err;
 
-	if (!encl_create(ctx.encl_fd, ctx.bin_size, &ctx.secs))
+	ehdr = ctx.bin;
+	phdr_tbl = ctx.bin + ehdr->e_phoff;
+	start_offset = 0;
+	encl_size = 0;
+
+	for (i = 0; i < ehdr->e_phnum; i++) {
+		unsigned long offset, size;
+
+		phdr = &phdr_tbl[i];
+		if (phdr->p_type != PT_LOAD)
+			continue;
+
+		offset = phdr->p_offset & PAGE_MASK;
+		if (!start_offset)
+			start_offset = offset;
+
+		size = (offset - start_offset + phdr->p_filesz +
+			PAGE_SIZE - 1) & PAGE_MASK;
+		if (size > encl_size)
+			encl_size = size;
+	}
+
+	if (!encl_create(ctx.encl_fd, encl_size, &ctx.secs))
 		goto err;
 
 	/* TCS */
-	if (!encl_build_segment(ctx.encl_fd, &ctx.secs, ctx.bin, 0, PAGE_SIZE,
-				SGX_SECINFO_TCS, PROT_READ | PROT_WRITE))
+	if (!encl_build_segment(ctx.encl_fd, &ctx.secs, ctx.bin + start_offset,
+				0, PAGE_SIZE, SGX_SECINFO_TCS,
+				PROT_READ | PROT_WRITE))
 		goto err;
 
-	if (!encl_build_segment(ctx.encl_fd, &ctx.secs, ctx.bin, PAGE_SIZE,
-				ctx.bin_size - PAGE_SIZE,
+	if (!encl_build_segment(ctx.encl_fd, &ctx.secs, ctx.bin + start_offset,
+				PAGE_SIZE, encl_size - PAGE_SIZE,
 				SGX_SECINFO_REG | SGX_SECINFO_R |
 				SGX_SECINFO_W | SGX_SECINFO_X,
 				PROT_READ | PROT_WRITE | PROT_EXEC))
 		goto err;
 
-	if (!encl_create_sigstruct(ctx.bin, ctx.bin_size, &sigstruct))
+	if (!encl_create_sigstruct(ctx.bin + start_offset, encl_size,
+				   &sigstruct))
 		goto err;
 
 	ioc.sigstruct = (uint64_t)&sigstruct;
-- 
2.25.1

Re: [PATCH 1/5] selftests/sgx: Add PHDRS to encl.lds

[Jarkko Sakkinen]

On Mon, Mar 23, 2020 at 05:46:30AM +0200, Jarkko Sakkinen wrote:
> Improve encl.lds to create an ELF image that can be easily loaded without
> needing the conversion to a raw binary. This is achieved by adding PHDRS to
> encl.lds that describes the different segments.
> 
> With a simple Python program it is easy to see that the changes result in a
> sane memory layout [1]:
> 
> Flags Start              End
> rw-   0x0000000000200000 0x0000000000201000
> r-x   0x0000000000201000 0x0000000000202000
> rw-   0x0000000000202000 0x0000000000205000
> 
> These are the start and end positions in the enclave ELF image for
> different enclave memory areas. Since all the sections are marked as being
> allocated, an ELF enclave loader can be solely based on p_offset, p_memsz
> and p_flags fields of struct Elf64_Phdr.
> 
> [1]
> import sys
> from elftools.elf.elffile import ELFFile
> 
> PAGE_SIZE = 0x1000
> 
> if __name__ == '__main__':
>     flags2str = ['---', '--x', '-w-', '-wx', 'r--', 'r-x', 'rw-', 'rwx']
> 
>     if len(sys.argv) != 2:
>         sys.exit(1)
> 
>     with open(sys.argv[1], 'rb') as file:
>         file = ELFFile(file)
> 
>         print('{:<5} {:<18} {:<18}'.format('Flags', 'Start', 'End'))
> 
>         for seg in file.iter_segments():
>             if seg['p_type'] != 'PT_LOAD':
>                 continue
> 
>             flags = flags2str[seg['p_flags']]
> 
>             start = seg['p_offset'] & ~(PAGE_SIZE - 1)
>             end = start +
> 	          (seg['p_filesz'] + PAGE_SIZE - 1) & ~(PAGE_SIZE - 1)
> 
>             print('{:<5} 0x{:0>16x} 0x{:0>16x}'.format(flags, start, end))
> 
> Cc: Sean Christopherson <sean.j.christopherson@intel.com>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>  tools/testing/selftests/sgx/encl.lds         | 14 ++++++++++----
>  tools/testing/selftests/sgx/encl_bootstrap.S |  2 +-
>  2 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/testing/selftests/sgx/encl.lds b/tools/testing/selftests/sgx/encl.lds
> index 9a56d3064104..0fbbda7e665e 100644
> --- a/tools/testing/selftests/sgx/encl.lds
> +++ b/tools/testing/selftests/sgx/encl.lds
> @@ -1,25 +1,31 @@
>  OUTPUT_FORMAT(elf64-x86-64)
>  
> +PHDRS
> +{
> +	tcs PT_LOAD;
> +	text PT_LOAD;
> +	data PT_LOAD;
> +}
> +
>  SECTIONS
>  {
>  	. = 0;
>  	.tcs : {
>  		*(.tcs*)
> -	}
> +	} : tcs
>  
>  	. = ALIGN(4096);
>  	.text : {
>  		*(.text*)
>  		*(.rodata*)
> -	}
> +	} : text
>  
>  	. = ALIGN(4096);
>  	.data : {
>  		*(.data*)
> -	}
> +	} : data
>  
>  	/DISCARD/ : {
> -		*(.data*)
>  		*(.comment*)
>  		*(.note*)
>  		*(.debug*)
> diff --git a/tools/testing/selftests/sgx/encl_bootstrap.S b/tools/testing/selftests/sgx/encl_bootstrap.S
> index 3a1479f1cdcf..b9ea6130e422 100644
> --- a/tools/testing/selftests/sgx/encl_bootstrap.S
> +++ b/tools/testing/selftests/sgx/encl_bootstrap.S
> @@ -7,7 +7,7 @@
>  	.byte 0x0f, 0x01, 0xd7
>  	.endm
>  
> -	.section ".tcs", "a"
> +	.section ".tcs", "aw"
>  	.balign	4096
>  
>  	.fill	1, 8, 0			# STATE (set by CPU)
> -- 
> 2.25.1
> 

These changes have been squashed to my tree. Please provide patches
if something feels not right.

The changes were live coded on a Geminilake NUC that I brought home
last week and are tested quite extensively.

The place for improvement would be to call sgx_encl_build_segment()
based on segments in the program header table so that the permissions
would be assigned dynamically.

/Jarkko