V1 available at: https://lore.kernel.org/lkml/cover.1631731214.git.reinette.chatre@intel.com/
Changes since V1:
- Biggest change: The non-kselftest placeholder patches included in this series that the kselftest work depends on are still being discussed elsewhere (link below) but has changed significantly since the first submission, warranting an update to the kselftest patches that depend on it. Jarkko: I made significant modifications to your "selftests/sgx: Add a new kselftest: unclobbered_vdso_oversubscribed" that you may want to look at. - Improve cover letter and changelogs (Dave). - Add Jarkko and Dave's signatures where obtained (Jarkko and Dave). - Fix Cedric's signature in patch 1 (Jarkko and Cedric). - Improve the loop locating the data segment (Jarkko). - Update placeholder patches that makes the amount of SGX memory available to latest version (v8). Previously this dependency consisted out of one patch, now it spans two.
Hi Everybody,
This series consists out of outstanding SGX selftests changes, rebased and gathered in a single series that can easily be merged for testing and development, and a few more changes added to expand the existing tests.
The outstanding SGX selftest changes included in this series that have already been submitted separately are:
* A more than two year old patch fixing a benign linker warning that is still present today: https://lore.kernel.org/linux-sgx/20191017030340.18301-2-sean.j.christophers... The original patch is added intact and not all email addresses within are valid.
* Latest (v4) of Jarkko Sakkinen's series to add an oversubscription test: https://lore.kernel.org/linux-sgx/20210809093127.76264-1-jarkko@kernel.org/
* Latest (v2) of Jarkko Sakkinen's patch that provides per-op parameter structs for the test enclave: https://lore.kernel.org/linux-sgx/20210812224645.90280-1-jarkko@kernel.org/
The reason why most of these patches are outstanding is that they depend on a kernel change that is still under discussion. Decision to wait in: https://lore.kernel.org/linux-sgx/f8674dac5579a8a424de1565f7ffa2b5bf2f8e36.c... The latest patches (v8) for this dependency is included in this series as a placeholder until the ongoing discussions are concluded: https://lore.kernel.org/lkml/20211018135744.45527-1-jarkko@kernel.org/ https://lore.kernel.org/lkml/20211018135744.45527-2-jarkko@kernel.org/
The new changes introduced in this series builds on Jarkko's outstanding SGX selftest changes and adds new tests for page permissions, exception handling, and thread entry.
Building and running enclaves is painful and traditionally requires a big software stack. This adds features like threads to the SGX selftests which are traditionally implemented in that big software stack. This helps test SGX kernel support with only code from the kernel tree.
Reinette
Jarkko Sakkinen (10): x86/sgx: Rename fallback labels in sgx_init() x86/sgx: Add an attribute for the amount of SGX memory in a NUMA node selftests/sgx: Assign source for each segment selftests/sgx: Make data measurement for an enclave segment optional selftests/sgx: Create a heap for the test enclave selftests/sgx: Dump segments and /proc/self/maps only on failure selftests/sgx: Encpsulate the test enclave creation selftests/sgx: Move setup_test_encl() to each TEST_F() selftests/sgx: Add a new kselftest: unclobbered_vdso_oversubscribed selftests/sgx: Provide per-op parameter structs for the test enclave
Reinette Chatre (4): selftests/sgx: Rename test properties in preparation for more enclave tests selftests/sgx: Add page permission and exception test selftests/sgx: Enable multiple thread support selftests/sgx: Add test for multiple TCS entry
Sean Christopherson (1): selftests/x86/sgx: Fix a benign linker warning
Documentation/ABI/stable/sysfs-devices-node | 7 + arch/x86/kernel/cpu/sgx/main.c | 97 ++++- arch/x86/kernel/cpu/sgx/sgx.h | 2 + tools/testing/selftests/sgx/Makefile | 2 +- tools/testing/selftests/sgx/defines.h | 33 +- tools/testing/selftests/sgx/load.c | 40 +- tools/testing/selftests/sgx/main.c | 396 ++++++++++++++++-- tools/testing/selftests/sgx/main.h | 7 +- tools/testing/selftests/sgx/sigstruct.c | 12 +- tools/testing/selftests/sgx/test_encl.c | 60 ++- .../selftests/sgx/test_encl_bootstrap.S | 21 +- 11 files changed, 585 insertions(+), 92 deletions(-)
base-commit: 7d2a07b769330c34b4deabeed939325c77a7ec2f
From: Sean Christopherson sean.j.christopherson@intel.com
Pass a build id of "none" to the linker to suppress a warning about the build id being ignored:
/usr/bin/ld: warning: .note.gnu.build-id section discarded, --build-id ignored.
Link: https://lore.kernel.org/linux-sgx/20191017030340.18301-2-sean.j.christophers... Suggested-by: Cedric Xing cedric.xing@intel.com Signed-off-by: Sean Christopherson sean.j.christopherson@intel.com Reviewed-by: Jarkko Sakkinen jarkko@kernel.org Acked-by: Dave Hansen dave.hansen@linux.intel.com Signed-off-by: Reinette Chatre reinette.chatre@intel.com --- Changes since V1: - Change Cedric's signature to a "Suggested-by" (Jarkko and Cedric). - Add signatures from Jarkko and Dave.
tools/testing/selftests/sgx/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/sgx/Makefile b/tools/testing/selftests/sgx/Makefile index 7f12d55b97f8..2956584e1e37 100644 --- a/tools/testing/selftests/sgx/Makefile +++ b/tools/testing/selftests/sgx/Makefile @@ -45,7 +45,7 @@ $(OUTPUT)/sign_key.o: sign_key.S $(CC) $(HOST_CFLAGS) -c $< -o $@
$(OUTPUT)/test_encl.elf: test_encl.lds test_encl.c test_encl_bootstrap.S - $(CC) $(ENCL_CFLAGS) -T $^ -o $@ + $(CC) $(ENCL_CFLAGS) -T $^ -o $@ -Wl,--build-id=none
EXTRA_CLEAN := \ $(OUTPUT)/test_encl.elf \
On 10/28/21 1:37 PM, Reinette Chatre wrote:
From: Sean Christopherson sean.j.christopherson@intel.com
Pass a build id of "none" to the linker to suppress a warning about the build id being ignored:
/usr/bin/ld: warning: .note.gnu.build-id section discarded, --build-id ignored.
Do we have a good grasp on why this is producing a warning in the first place? This seems like something that could get merged quickly with one more sentence in the changelog.
On Thu, Oct 28, 2021, Dave Hansen wrote:
On 10/28/21 1:37 PM, Reinette Chatre wrote:
From: Sean Christopherson sean.j.christopherson@intel.com
Pass a build id of "none" to the linker to suppress a warning about the build id being ignored:
/usr/bin/ld: warning: .note.gnu.build-id section discarded, --build-id ignored.
Do we have a good grasp on why this is producing a warning in the first place? This seems like something that could get merged quickly with one more sentence in the changelog.
The SGX selftests use a custom linker script, tools/testing/selftests/sgx/test_encl.lds, to configure the resulting enclave binary so that it's loadable as an enclave more or less as-is. One of the things the script does is drop sections the selftests doesn't want, .note* sections being in that category. I don't recall exactly why the script drops sections; I assume it's to simply the loading process. Anyways, .note.gnu.build-id is collateral damage and the linker complains.
Hi Dave,
On 10/28/2021 5:26 PM, Dave Hansen wrote:
On 10/28/21 1:37 PM, Reinette Chatre wrote:
From: Sean Christopherson sean.j.christopherson@intel.com
Pass a build id of "none" to the linker to suppress a warning about the build id being ignored:
/usr/bin/ld: warning: .note.gnu.build-id section discarded, --build-id ignored.
Do we have a good grasp on why this is producing a warning in the first place? This seems like something that could get merged quickly with one more sentence in the changelog.
How about a new changelog as below:
The enclave binary (test_encl.elf) is built with only three sections (tcs, text, and data) as controlled by its custom linker script.
If gcc is built with "--enable-linker-build-id" (this appears to be a common configuration even if it is by default off) then gcc will pass "--build-id" to the linker that will prompt it (the linker) to to write unique bits identifying the linked file to a ".note.gnu.build-id" section.
The section ".note.gnu.build-id" does not exist in the test enclave resulting in the following warning emitted by the linker:
/usr/bin/ld: warning: .note.gnu.build-id section discarded, --build-id ignored
The test enclave does not use the build id within the binary so fix the warning by passing a build id of "none" to the linker that will disable the setting from any earlier "--build-id" options and thus disable the attempt to write the build id to a ".note.gnu.build-id" section that does not exist.
Reinette
On 10/29/21 10:09 AM, Reinette Chatre wrote:
On 10/28/2021 5:26 PM, Dave Hansen wrote:
On 10/28/21 1:37 PM, Reinette Chatre wrote:
From: Sean Christopherson sean.j.christopherson@intel.com
Pass a build id of "none" to the linker to suppress a warning about the build id being ignored:
/usr/bin/ld: warning: .note.gnu.build-id section discarded, --build-id ignored.
Do we have a good grasp on why this is producing a warning in the first place? This seems like something that could get merged quickly with one more sentence in the changelog.
How about a new changelog as below:
The enclave binary (test_encl.elf) is built with only three sections (tcs, text, and data) as controlled by its custom linker script.
If gcc is built with "--enable-linker-build-id" (this appears to be a common configuration even if it is by default off) then gcc will pass "--build-id" to the linker that will prompt it (the linker) to to write unique bits identifying the linked file to a ".note.gnu.build-id" section.
The section ".note.gnu.build-id" does not exist in the test enclave resulting in the following warning emitted by the linker:
/usr/bin/ld: warning: .note.gnu.build-id section discarded, --build-id ignored
The test enclave does not use the build id within the binary so fix the warning by passing a build id of "none" to the linker that will disable the setting from any earlier "--build-id" options and thus disable the attempt to write the build id to a ".note.gnu.build-id" section that does not exist.
Looks great, thanks for putting that together!
From: Jarkko Sakkinen jarkko@kernel.org
It's hard to add new content to this function because it is time consuming to match fallback and its cause. Rename labels in a way that the name of error label refers to the site where failure happened. This way it is easier to keep on track what is going on.
Signed-off-by: Jarkko Sakkinen jarkko@kernel.org [reinette: Placeholder patch - submitted separately upstream https://lore.kernel.org/lkml/20211018135744.45527-1-jarkko@kernel.org/ ] --- arch/x86/kernel/cpu/sgx/main.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c index 63d3de02bbcc..a6e313f1a82d 100644 --- a/arch/x86/kernel/cpu/sgx/main.c +++ b/arch/x86/kernel/cpu/sgx/main.c @@ -803,12 +803,12 @@ static int __init sgx_init(void)
if (!sgx_page_reclaimer_init()) { ret = -ENOMEM; - goto err_page_cache; + goto err_reclaimer; }
ret = misc_register(&sgx_dev_provision); if (ret) - goto err_kthread; + goto err_provision;
/* * Always try to initialize the native *and* KVM drivers. @@ -821,17 +821,17 @@ static int __init sgx_init(void) ret = sgx_drv_init();
if (sgx_vepc_init() && ret) - goto err_provision; + goto err_driver;
return 0;
-err_provision: +err_driver: misc_deregister(&sgx_dev_provision);
-err_kthread: +err_provision: kthread_stop(ksgxd_tsk);
-err_page_cache: +err_reclaimer: for (i = 0; i < sgx_nr_epc_sections; i++) { vfree(sgx_epc_sections[i].pages); memunmap(sgx_epc_sections[i].virt_addr);
On 10/28/21 1:37 PM, Reinette Chatre wrote:
-err_provision: +err_driver: misc_deregister(&sgx_dev_provision); -err_kthread: +err_provision: kthread_stop(ksgxd_tsk); -err_page_cache: +err_reclaimer: for (i = 0; i < sgx_nr_epc_sections; i++) { vfree(sgx_epc_sections[i].pages); memunmap(sgx_epc_sections[i].virt_addr);
This isn't the normal pattern we use in the kernel.
Usually, we say what is being *DONE* at the label, almost like we are calling a function. Here's a random example from one of the most prolific users of gotos in arch/x86:
ret = kernfs_get_tree(fc); if (ret < 0) goto out_psl; ... out_psl: rdt_pseudo_lock_release();
See? The "psl" is doing a "pseudo lock release".
I don't particularly like the labels as-is, but this patch makes them less clear, IMNHO. Let's just drop this patch for now, please.
From: Jarkko Sakkinen jarkko@kernel.org
The amount of SGX memory on the system is determined by the BIOS and it varies wildly between systems. It can be from dozens of MB's on desktops or VM's, up to many GB's on servers. Just like for regular memory, it is sometimes useful to know the amount of usable SGX memory in the system.
Add an attribute for the amount of SGX memory in bytes to each NUMA node. The path is /sys/devices/system/node/node[0-9]*/sgx/size. Calculate these values by summing up EPC section sizes for each node during the driver initalization.
Signed-off-by: Jarkko Sakkinen jarkko@kernel.org [reinette: Placeholder patch - submitted separately upstream https://lore.kernel.org/lkml/20211018135744.45527-2-jarkko@kernel.org/ ] --- Documentation/ABI/stable/sysfs-devices-node | 7 ++ arch/x86/kernel/cpu/sgx/main.c | 85 +++++++++++++++++++++ arch/x86/kernel/cpu/sgx/sgx.h | 2 + 3 files changed, 94 insertions(+)
diff --git a/Documentation/ABI/stable/sysfs-devices-node b/Documentation/ABI/stable/sysfs-devices-node index 484fc04bcc25..12dc2149e8e0 100644 --- a/Documentation/ABI/stable/sysfs-devices-node +++ b/Documentation/ABI/stable/sysfs-devices-node @@ -176,3 +176,10 @@ Contact: Keith Busch keith.busch@intel.com Description: The cache write policy: 0 for write-back, 1 for write-through, other or unknown. + +What: /sys/devices/system/node/nodeX/sgx/size +Date: October 2021 +Contact: Jarkko Sakkinen jarkko@kernel.org +Description: + Total available physical SGX memory, also known as Enclave Page + Cache (EPC), in bytes. diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c index a6e313f1a82d..dc1d46c51323 100644 --- a/arch/x86/kernel/cpu/sgx/main.c +++ b/arch/x86/kernel/cpu/sgx/main.c @@ -714,9 +714,11 @@ static bool __init sgx_page_cache_init(void) spin_lock_init(&sgx_numa_nodes[nid].lock); INIT_LIST_HEAD(&sgx_numa_nodes[nid].free_page_list); node_set(nid, sgx_numa_mask); + sgx_numa_nodes[nid].size = 0; }
sgx_epc_sections[i].node = &sgx_numa_nodes[nid]; + sgx_numa_nodes[nid].size += size;
sgx_nr_epc_sections++; } @@ -790,6 +792,81 @@ int sgx_set_attribute(unsigned long *allowed_attributes, } EXPORT_SYMBOL_GPL(sgx_set_attribute);
+#ifdef CONFIG_NUMA +static ssize_t size_show(struct device *dev, struct device_attribute *attr, char *buf) +{ + unsigned long size = 0; + int nid; + + for (nid = 0; nid < num_possible_nodes(); nid++) { + if (dev == sgx_numa_nodes[nid].dev) { + size = sgx_numa_nodes[nid].size; + break; + } + } + + return sysfs_emit(buf, "%lu\n", size); +} +DEVICE_ATTR_RO(size); + +static struct attribute *sgx_node_attrs[] = { + &dev_attr_size.attr, + NULL, +}; + +static const struct attribute_group sgx_node_attr_group = { + .name = "sgx", + .attrs = sgx_node_attrs, +}; + +static void sgx_numa_exit(void) +{ + struct device *dev; + int nid; + + for (nid = 0; nid < num_possible_nodes(); nid++) { + dev = &node_devices[nid]->dev; + if (dev) + sysfs_remove_group(&dev->kobj, &sgx_node_attr_group); + } +} + +static bool sgx_numa_init(void) +{ + struct sgx_numa_node *node; + struct device *dev; + int nid; + int ret; + + for (nid = 0; nid < num_possible_nodes(); nid++) { + if (!sgx_numa_nodes[nid].size) + continue; + + node = &sgx_numa_nodes[nid]; + dev = &node_devices[nid]->dev; + + ret = sysfs_create_group(&dev->kobj, &sgx_node_attr_group); + if (ret) { + sgx_numa_exit(); + return false; + } + + node->dev = dev; + } + + return true; +} +#else +static inline void sgx_numa_exit(void) +{ +} + +static inline bool sgx_numa_init(void) +{ + return true; +} +#endif /* CONFIG_NUMA */ + static int __init sgx_init(void) { int ret; @@ -806,6 +883,11 @@ static int __init sgx_init(void) goto err_reclaimer; }
+ if (!sgx_numa_init()) { + ret = -ENOMEM; + goto err_numa_nodes; + } + ret = misc_register(&sgx_dev_provision); if (ret) goto err_provision; @@ -829,6 +911,9 @@ static int __init sgx_init(void) misc_deregister(&sgx_dev_provision);
err_provision: + sgx_numa_exit(); + +err_numa_nodes: kthread_stop(ksgxd_tsk);
err_reclaimer: diff --git a/arch/x86/kernel/cpu/sgx/sgx.h b/arch/x86/kernel/cpu/sgx/sgx.h index 4628acec0009..1de8c627a286 100644 --- a/arch/x86/kernel/cpu/sgx/sgx.h +++ b/arch/x86/kernel/cpu/sgx/sgx.h @@ -39,6 +39,8 @@ struct sgx_epc_page { */ struct sgx_numa_node { struct list_head free_page_list; + struct device *dev; + unsigned long size; spinlock_t lock; };
On 10/28/21 1:37 PM, Reinette Chatre wrote:
The amount of SGX memory on the system is determined by the BIOS and it varies wildly between systems. It can be from dozens of MB's on desktops or VM's, up to many GB's on servers. Just like for regular memory, it is sometimes useful to know the amount of usable SGX memory in the system.
Add an attribute for the amount of SGX memory in bytes to each NUMA node. The path is /sys/devices/system/node/node[0-9]*/sgx/size. Calculate these values by summing up EPC section sizes for each node during the driver initalization.
For now, can we just make the selftests read the SGX CPUID section leaves? It's not as precise as knowing how much the kernel actually decided to use, but it's good enough for a selftest. It also means we can merge something without having to worry about long-term ABI.
This is also why I once suggested that we first make the selftests depend on some debugfs file that would be short-lived. But, if we use CPUID, we don't even need to mess with debugfs.
You can even just steal the code from sgx_page_cache_init() to do it.
Would that work, or am I missing something?
Hi Dave,
On 10/29/2021 11:06 AM, Dave Hansen wrote:
On 10/28/21 1:37 PM, Reinette Chatre wrote:
The amount of SGX memory on the system is determined by the BIOS and it varies wildly between systems. It can be from dozens of MB's on desktops or VM's, up to many GB's on servers. Just like for regular memory, it is sometimes useful to know the amount of usable SGX memory in the system.
Add an attribute for the amount of SGX memory in bytes to each NUMA node. The path is /sys/devices/system/node/node[0-9]*/sgx/size. Calculate these values by summing up EPC section sizes for each node during the driver initalization.
For now, can we just make the selftests read the SGX CPUID section leaves? It's not as precise as knowing how much the kernel actually decided to use, but it's good enough for a selftest. It also means we can merge something without having to worry about long-term ABI.
Yes, we can do that.
This is also why I once suggested that we first make the selftests depend on some debugfs file that would be short-lived. But, if we use CPUID, we don't even need to mess with debugfs.
My apologies, this was not intended to avoid your suggestion. V1 did use the debugfs solution as you suggested as placeholder but after the debufs solution evolved the tests were adapted to follow those changes instead of sticking with the debugfs solution as proposed in https://lore.kernel.org/lkml/6f3cc681e10877e639b882eaabf1a5e21bd2fc94.camel@...
You can even just steal the code from sgx_page_cache_init() to do it.
Would that work, or am I missing something?
I do think that will work. The selftests are only interested in the total SGX memory (as opposed to memory per numa node as exposed with the current interface) and that can be obtained via CPUID. I will adapt the oversubscription test case to obtain its needed info via CPUID.
Thank you very much
Reinette
From: Jarkko Sakkinen jarkko@kernel.org
Define source per segment so that enclave pages can be added from different sources, e.g. anonymous VMA for zero pages. In other words, add 'src' field to struct encl_segment, and assign it to 'encl->src' for pages inherited from the enclave binary.
Signed-off-by: Jarkko Sakkinen jarkko@kernel.org Acked-by: Dave Hansen dave.hansen@linux.intel.com Signed-off-by: Reinette Chatre reinette.chatre@intel.com --- Changes since V1: - Add signature from Dave.
tools/testing/selftests/sgx/load.c | 5 +++-- tools/testing/selftests/sgx/main.h | 1 + tools/testing/selftests/sgx/sigstruct.c | 8 ++++---- 3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/sgx/load.c b/tools/testing/selftests/sgx/load.c index 3ebe5d1fe337..5605474aab73 100644 --- a/tools/testing/selftests/sgx/load.c +++ b/tools/testing/selftests/sgx/load.c @@ -107,7 +107,7 @@ static bool encl_ioc_add_pages(struct encl *encl, struct encl_segment *seg) memset(&secinfo, 0, sizeof(secinfo)); secinfo.flags = seg->flags;
- ioc.src = (uint64_t)encl->src + seg->offset; + ioc.src = (uint64_t)seg->src; ioc.offset = seg->offset; ioc.length = seg->size; ioc.secinfo = (unsigned long)&secinfo; @@ -216,6 +216,7 @@ bool encl_load(const char *path, struct encl *encl)
if (j == 0) { src_offset = phdr->p_offset & PAGE_MASK; + encl->src = encl->bin + src_offset;
seg->prot = PROT_READ | PROT_WRITE; seg->flags = SGX_PAGE_TYPE_TCS << 8; @@ -228,13 +229,13 @@ bool encl_load(const char *path, struct encl *encl)
seg->offset = (phdr->p_offset & PAGE_MASK) - src_offset; seg->size = (phdr->p_filesz + PAGE_SIZE - 1) & PAGE_MASK; + seg->src = encl->src + seg->offset;
j++; }
assert(j == encl->nr_segments);
- encl->src = encl->bin + src_offset; encl->src_size = encl->segment_tbl[j - 1].offset + encl->segment_tbl[j - 1].size;
diff --git a/tools/testing/selftests/sgx/main.h b/tools/testing/selftests/sgx/main.h index 68672fd86cf9..452d11dc4889 100644 --- a/tools/testing/selftests/sgx/main.h +++ b/tools/testing/selftests/sgx/main.h @@ -7,6 +7,7 @@ #define MAIN_H
struct encl_segment { + void *src; off_t offset; size_t size; unsigned int prot; diff --git a/tools/testing/selftests/sgx/sigstruct.c b/tools/testing/selftests/sgx/sigstruct.c index 92bbc5a15c39..202a96fd81bf 100644 --- a/tools/testing/selftests/sgx/sigstruct.c +++ b/tools/testing/selftests/sgx/sigstruct.c @@ -289,14 +289,14 @@ static bool mrenclave_eextend(EVP_MD_CTX *ctx, uint64_t offset, static bool mrenclave_segment(EVP_MD_CTX *ctx, struct encl *encl, struct encl_segment *seg) { - uint64_t end = seg->offset + seg->size; + uint64_t end = seg->size; uint64_t offset;
- for (offset = seg->offset; offset < end; offset += PAGE_SIZE) { - if (!mrenclave_eadd(ctx, offset, seg->flags)) + for (offset = 0; offset < end; offset += PAGE_SIZE) { + if (!mrenclave_eadd(ctx, seg->offset + offset, seg->flags)) return false;
- if (!mrenclave_eextend(ctx, offset, encl->src + offset)) + if (!mrenclave_eextend(ctx, seg->offset + offset, seg->src + offset)) return false; }
From: Jarkko Sakkinen jarkko@kernel.org
For a heap makes sense to leave its contents "unmeasured" in the SGX enclave build process, meaning that they won't contribute to the cryptographic signature (a RSA-3072 signed SHA56 hash) of the enclave.
Enclaves are signed blobs where the signature is calculated both from page data and also from "structural properties" of the pages. For instance a page offset of *every* page added to the enclave is hashed.
For data, this is optional, not least because hashing a page has a significant contribution to the enclave load time. Thus, where there is no reason to hash, do not. The SGX ioctl interface supports this with SGX_PAGE_MEASURE flag. Only when the flag is *set*, data is measured.
Add seg->measure boolean flag to struct encl_segment. Only when the flag is set, include the segment data to the signature (represented by SIGSTRUCT architectural structure).
Signed-off-by: Jarkko Sakkinen jarkko@kernel.org Acked-by: Dave Hansen dave.hansen@linux.intel.com Signed-off-by: Reinette Chatre reinette.chatre@intel.com --- Changes since V1: - Add signature from Dave.
tools/testing/selftests/sgx/load.c | 6 +++++- tools/testing/selftests/sgx/main.h | 1 + tools/testing/selftests/sgx/sigstruct.c | 6 ++++-- 3 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/sgx/load.c b/tools/testing/selftests/sgx/load.c index 5605474aab73..f1be78984c50 100644 --- a/tools/testing/selftests/sgx/load.c +++ b/tools/testing/selftests/sgx/load.c @@ -111,7 +111,10 @@ static bool encl_ioc_add_pages(struct encl *encl, struct encl_segment *seg) ioc.offset = seg->offset; ioc.length = seg->size; ioc.secinfo = (unsigned long)&secinfo; - ioc.flags = SGX_PAGE_MEASURE; + if (seg->measure) + ioc.flags = SGX_PAGE_MEASURE; + else + ioc.flags = 0;
rc = ioctl(encl->fd, SGX_IOC_ENCLAVE_ADD_PAGES, &ioc); if (rc < 0) { @@ -230,6 +233,7 @@ bool encl_load(const char *path, struct encl *encl) seg->offset = (phdr->p_offset & PAGE_MASK) - src_offset; seg->size = (phdr->p_filesz + PAGE_SIZE - 1) & PAGE_MASK; seg->src = encl->src + seg->offset; + seg->measure = true;
j++; } diff --git a/tools/testing/selftests/sgx/main.h b/tools/testing/selftests/sgx/main.h index 452d11dc4889..aebc69e7cdc8 100644 --- a/tools/testing/selftests/sgx/main.h +++ b/tools/testing/selftests/sgx/main.h @@ -12,6 +12,7 @@ struct encl_segment { size_t size; unsigned int prot; unsigned int flags; + bool measure; };
struct encl { diff --git a/tools/testing/selftests/sgx/sigstruct.c b/tools/testing/selftests/sgx/sigstruct.c index 202a96fd81bf..50c5ab1aa6fa 100644 --- a/tools/testing/selftests/sgx/sigstruct.c +++ b/tools/testing/selftests/sgx/sigstruct.c @@ -296,8 +296,10 @@ static bool mrenclave_segment(EVP_MD_CTX *ctx, struct encl *encl, if (!mrenclave_eadd(ctx, seg->offset + offset, seg->flags)) return false;
- if (!mrenclave_eextend(ctx, seg->offset + offset, seg->src + offset)) - return false; + if (seg->measure) { + if (!mrenclave_eextend(ctx, seg->offset + offset, seg->src + offset)) + return false; + } }
return true;
From: Jarkko Sakkinen jarkko@kernel.org
Create a heap for the test enclave, which is allocated from /dev/null, and left unmeasured. This is beneficial by its own because it verifies that an enclave built from multiple choices, works properly. If LSM hooks are added for SGX some day, a multi source enclave has higher probability to trigger bugs on access control checks.
The immediate need comes from the need to implement page reclaim tests. In order to trigger the page reclaimer, one can just set the size of the heap to high enough.
Signed-off-by: Jarkko Sakkinen jarkko@kernel.org Acked-by: Dave Hansen dave.hansen@linux.intel.com Signed-off-by: Reinette Chatre reinette.chatre@intel.com --- Changes since V1: - Add signature from Dave.
tools/testing/selftests/sgx/load.c | 29 ++++++++++++++++++++++------- tools/testing/selftests/sgx/main.c | 2 +- tools/testing/selftests/sgx/main.h | 4 +++- 3 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/sgx/load.c b/tools/testing/selftests/sgx/load.c index f1be78984c50..9d4322c946e2 100644 --- a/tools/testing/selftests/sgx/load.c +++ b/tools/testing/selftests/sgx/load.c @@ -21,6 +21,8 @@
void encl_delete(struct encl *encl) { + struct encl_segment *heap_seg = &encl->segment_tbl[encl->nr_segments - 1]; + if (encl->encl_base) munmap((void *)encl->encl_base, encl->encl_size);
@@ -30,6 +32,8 @@ void encl_delete(struct encl *encl) if (encl->fd) close(encl->fd);
+ munmap(heap_seg->src, heap_seg->size); + if (encl->segment_tbl) free(encl->segment_tbl);
@@ -125,11 +129,10 @@ static bool encl_ioc_add_pages(struct encl *encl, struct encl_segment *seg) return true; }
- - -bool encl_load(const char *path, struct encl *encl) +bool encl_load(const char *path, struct encl *encl, unsigned long heap_size) { const char device_path[] = "/dev/sgx_enclave"; + struct encl_segment *seg; Elf64_Phdr *phdr_tbl; off_t src_offset; Elf64_Ehdr *ehdr; @@ -181,6 +184,8 @@ bool encl_load(const char *path, struct encl *encl) ehdr = encl->bin; phdr_tbl = encl->bin + ehdr->e_phoff;
+ encl->nr_segments = 1; /* one for the heap */ + for (i = 0; i < ehdr->e_phnum; i++) { Elf64_Phdr *phdr = &phdr_tbl[i];
@@ -196,7 +201,6 @@ bool encl_load(const char *path, struct encl *encl) for (i = 0, j = 0; i < ehdr->e_phnum; i++) { Elf64_Phdr *phdr = &phdr_tbl[i]; unsigned int flags = phdr->p_flags; - struct encl_segment *seg;
if (phdr->p_type != PT_LOAD) continue; @@ -238,10 +242,21 @@ bool encl_load(const char *path, struct encl *encl) j++; }
- assert(j == encl->nr_segments); + assert(j == encl->nr_segments - 1); + + seg = &encl->segment_tbl[j]; + seg->offset = encl->segment_tbl[j - 1].offset + encl->segment_tbl[j - 1].size; + seg->size = heap_size; + seg->src = mmap(NULL, heap_size, PROT_READ | PROT_WRITE, + MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); + seg->prot = PROT_READ | PROT_WRITE; + seg->flags = (SGX_PAGE_TYPE_REG << 8) | seg->prot; + seg->measure = false; + + if (seg->src == MAP_FAILED) + goto err;
- encl->src_size = encl->segment_tbl[j - 1].offset + - encl->segment_tbl[j - 1].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; ) encl->encl_size <<= 1; diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c index e252015e0c15..6858a35fed20 100644 --- a/tools/testing/selftests/sgx/main.c +++ b/tools/testing/selftests/sgx/main.c @@ -122,7 +122,7 @@ FIXTURE_SETUP(enclave) unsigned int i; void *addr;
- if (!encl_load("test_encl.elf", &self->encl)) { + if (!encl_load("test_encl.elf", &self->encl, ENCL_HEAP_SIZE_DEFAULT)) { encl_delete(&self->encl); ksft_exit_skip("cannot load enclaves\n"); } diff --git a/tools/testing/selftests/sgx/main.h b/tools/testing/selftests/sgx/main.h index aebc69e7cdc8..b45c52ec7ab3 100644 --- a/tools/testing/selftests/sgx/main.h +++ b/tools/testing/selftests/sgx/main.h @@ -6,6 +6,8 @@ #ifndef MAIN_H #define MAIN_H
+#define ENCL_HEAP_SIZE_DEFAULT 4096 + struct encl_segment { void *src; off_t offset; @@ -33,7 +35,7 @@ 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); +bool encl_load(const char *path, struct encl *encl, unsigned long heap_size); bool encl_measure(struct encl *encl); bool encl_build(struct encl *encl);
From: Jarkko Sakkinen jarkko@kernel.org
Logging is always a compromise between clarity and detail. The main use case for dumping VMA's is when FIXTURE_SETUP() fails, and is less important for enclaves that do initialize correctly. Therefore, print the segments and /proc/self/maps only in the error case.
Finally, if a single test ever creates multiple enclaves, the amount of log lines would become enormous.
Signed-off-by: Jarkko Sakkinen jarkko@kernel.org Acked-by: Dave Hansen dave.hansen@linux.intel.com Signed-off-by: Reinette Chatre reinette.chatre@intel.com --- Changes since V1: - Add signature from Dave.
tools/testing/selftests/sgx/main.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c index 6858a35fed20..deab02f2f3ce 100644 --- a/tools/testing/selftests/sgx/main.c +++ b/tools/testing/selftests/sgx/main.c @@ -127,12 +127,6 @@ FIXTURE_SETUP(enclave) ksft_exit_skip("cannot load enclaves\n"); }
- for (i = 0; i < self->encl.nr_segments; i++) { - seg = &self->encl.segment_tbl[i]; - - TH_LOG("0x%016lx 0x%016lx 0x%02x", seg->offset, seg->size, seg->prot); - } - if (!encl_measure(&self->encl)) goto err;
@@ -169,6 +163,17 @@ FIXTURE_SETUP(enclave) memset(&self->run, 0, sizeof(self->run)); self->run.tcs = self->encl.encl_base;
+ return; + +err: + encl_delete(&self->encl); + + for (i = 0; i < self->encl.nr_segments; i++) { + seg = &self->encl.segment_tbl[i]; + + TH_LOG("0x%016lx 0x%016lx 0x%02x", seg->offset, seg->size, seg->prot); + } + maps_file = fopen("/proc/self/maps", "r"); if (maps_file != NULL) { while (fgets(maps_line, sizeof(maps_line), maps_file) != NULL) { @@ -181,11 +186,7 @@ FIXTURE_SETUP(enclave) fclose(maps_file); }
-err: - if (!sgx_enter_enclave_sym) - encl_delete(&self->encl); - - ASSERT_NE(sgx_enter_enclave_sym, NULL); + ASSERT_TRUE(false); }
FIXTURE_TEARDOWN(enclave)
From: Jarkko Sakkinen jarkko@kernel.org
Introduce setup_test_encl() so that the enclave creation can be moved to TEST_F()'s. This is required for a reclaimer test where the heap size needs to be set large enough to triger the page reclaimer.
Signed-off-by: Jarkko Sakkinen jarkko@kernel.org Acked-by: Dave Hansen dave.hansen@linux.intel.com Signed-off-by: Reinette Chatre reinette.chatre@intel.com --- Changes since V1: - Add signature from Dave.
tools/testing/selftests/sgx/main.c | 44 ++++++++++++++++++------------ 1 file changed, 26 insertions(+), 18 deletions(-)
diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c index deab02f2f3ce..5b3e49a36344 100644 --- a/tools/testing/selftests/sgx/main.c +++ b/tools/testing/selftests/sgx/main.c @@ -112,7 +112,8 @@ FIXTURE(enclave) { struct sgx_enclave_run run; };
-FIXTURE_SETUP(enclave) +static bool setup_test_encl(unsigned long heap_size, struct encl *encl, + struct __test_metadata *_metadata) { Elf64_Sym *sgx_enter_enclave_sym = NULL; struct vdso_symtab symtab; @@ -122,25 +123,25 @@ FIXTURE_SETUP(enclave) unsigned int i; void *addr;
- if (!encl_load("test_encl.elf", &self->encl, ENCL_HEAP_SIZE_DEFAULT)) { - encl_delete(&self->encl); - ksft_exit_skip("cannot load enclaves\n"); + if (!encl_load("test_encl.elf", encl, heap_size)) { + encl_delete(encl); + TH_LOG("Failed to load the test enclave.\n"); }
- if (!encl_measure(&self->encl)) + if (!encl_measure(encl)) goto err;
- if (!encl_build(&self->encl)) + if (!encl_build(encl)) goto err;
/* * An enclave consumer only must do this. */ - for (i = 0; i < self->encl.nr_segments; i++) { - struct encl_segment *seg = &self->encl.segment_tbl[i]; + for (i = 0; i < encl->nr_segments; i++) { + struct encl_segment *seg = &encl->segment_tbl[i];
- addr = mmap((void *)self->encl.encl_base + seg->offset, seg->size, - seg->prot, MAP_SHARED | MAP_FIXED, self->encl.fd, 0); + addr = mmap((void *)encl->encl_base + seg->offset, seg->size, + seg->prot, MAP_SHARED | MAP_FIXED, encl->fd, 0); EXPECT_NE(addr, MAP_FAILED); if (addr == MAP_FAILED) goto err; @@ -160,16 +161,13 @@ FIXTURE_SETUP(enclave)
vdso_sgx_enter_enclave = addr + sgx_enter_enclave_sym->st_value;
- memset(&self->run, 0, sizeof(self->run)); - self->run.tcs = self->encl.encl_base; - - return; + return true;
err: - encl_delete(&self->encl); + encl_delete(encl);
- for (i = 0; i < self->encl.nr_segments; i++) { - seg = &self->encl.segment_tbl[i]; + for (i = 0; i < encl->nr_segments; i++) { + seg = &encl->segment_tbl[i];
TH_LOG("0x%016lx 0x%016lx 0x%02x", seg->offset, seg->size, seg->prot); } @@ -186,7 +184,17 @@ FIXTURE_SETUP(enclave) fclose(maps_file); }
- ASSERT_TRUE(false); + TH_LOG("Failed to initialize the test enclave.\n"); + + return false; +} + +FIXTURE_SETUP(enclave) +{ + ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata)); + + memset(&self->run, 0, sizeof(self->run)); + self->run.tcs = self->encl.encl_base; }
FIXTURE_TEARDOWN(enclave)
From: Jarkko Sakkinen jarkko@kernel.org
Create the test enclave inside each TEST_F(), instead of FIXTURE_SETUP(), so that the heap size can be defined per test.
Signed-off-by: Jarkko Sakkinen jarkko@kernel.org Acked-by: Dave Hansen dave.hansen@linux.intel.com Signed-off-by: Reinette Chatre reinette.chatre@intel.com --- Changes since V1: - Add signature from Dave.
tools/testing/selftests/sgx/main.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c index 5b3e49a36344..f41fba919d06 100644 --- a/tools/testing/selftests/sgx/main.c +++ b/tools/testing/selftests/sgx/main.c @@ -191,10 +191,6 @@ static bool setup_test_encl(unsigned long heap_size, struct encl *encl,
FIXTURE_SETUP(enclave) { - ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata)); - - memset(&self->run, 0, sizeof(self->run)); - self->run.tcs = self->encl.encl_base; }
FIXTURE_TEARDOWN(enclave) @@ -226,6 +222,11 @@ TEST_F(enclave, unclobbered_vdso) { struct encl_op op;
+ ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata)); + + memset(&self->run, 0, sizeof(self->run)); + self->run.tcs = self->encl.encl_base; + op.type = ENCL_OP_PUT; op.buffer = MAGIC;
@@ -248,6 +249,11 @@ TEST_F(enclave, clobbered_vdso) { struct encl_op op;
+ ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata)); + + memset(&self->run, 0, sizeof(self->run)); + self->run.tcs = self->encl.encl_base; + op.type = ENCL_OP_PUT; op.buffer = MAGIC;
@@ -278,6 +284,11 @@ TEST_F(enclave, clobbered_vdso_and_user_function) { struct encl_op op;
+ ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata)); + + memset(&self->run, 0, sizeof(self->run)); + self->run.tcs = self->encl.encl_base; + self->run.user_handler = (__u64)test_handler; self->run.user_data = 0xdeadbeef;
From: Jarkko Sakkinen jarkko@kernel.org
Add a variation of the unclobbered_vdso test.
In the new test, create a heap for the test enclave, which has the same size as all available Enclave Page Cache (EPC) pages in the system. This will guarantee that all test_encl.elf pages *and* SGX Enclave Control Structure (SECS) have been swapped out by the page reclaimer during the load time.
This test will trigger both the page reclaimer and the page fault handler. The page reclaimer triggered, while the heap is being created during the load time. The page fault handler is triggered for all the required pages, while the test case is executing.
Signed-off-by: Jarkko Sakkinen jarkko@kernel.org Acked-by: Dave Hansen dave.hansen@linux.intel.com [reinette: rework code obtaining SGX physical memory based on most recent upstream solution] Signed-off-by: Reinette Chatre reinette.chatre@intel.com --- Changes since V1: - Add signature from Dave. - Add support to obtain SGX physical memory from v8 of Jarkko's patches.
tools/testing/selftests/sgx/main.c | 114 +++++++++++++++++++++++++++++ tools/testing/selftests/sgx/main.h | 1 + 2 files changed, 115 insertions(+)
diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c index f41fba919d06..541862c15901 100644 --- a/tools/testing/selftests/sgx/main.c +++ b/tools/testing/selftests/sgx/main.c @@ -1,6 +1,8 @@ // SPDX-License-Identifier: GPL-2.0 /* Copyright(c) 2016-20 Intel Corporation. */
+#define _GNU_SOURCE +#include <dirent.h> #include <elf.h> #include <errno.h> #include <fcntl.h> @@ -245,6 +247,118 @@ TEST_F(enclave, unclobbered_vdso) EXPECT_EQ(self->run.user_data, 0); }
+static bool sysfs_get_ulong(const char *path, unsigned long *value) +{ + struct stat sbuf; + char buf[128]; + ssize_t ret; + int fd; + + ret = stat(path, &sbuf); + if (ret) + return false; + + fd = open(path, O_RDONLY); + if (fd < 0) + return false; + + ret = read(fd, buf, sizeof(buf)); + if (ret < 0) { + close(fd); + return false; + } + + errno = 0; + *value = strtoul(buf, NULL, 0); + + close(fd); + + return errno ? false : true; +} + +/* + * Sum total available physical SGX memory across all NUMA nodes + * + * Return: total available physical SGX memory available on system or 0 if a + * failure is encountered. + */ +static unsigned long get_total_epc_mem(void) +{ + char *node, *path = NULL; + unsigned long total = 0; + unsigned long size = 0; + struct dirent *entry; + struct stat statbuf; + DIR *dp; + int ret; + + dp = opendir(SGX_TOTAL_MEM_PATH); + if (!dp) + return 0; + + while ((entry = readdir(dp))) { + node = strstr(entry->d_name, "node"); + if (!node) + continue; + + ret = asprintf(&path, "%s/%s/sgx/size", + SGX_TOTAL_MEM_PATH, entry->d_name); + if (ret == -1) { + total = 0; + goto out; + } + + ret = stat(path, &statbuf); + if (ret == -1) { + free(path); + continue; + } + + if (S_ISREG(statbuf.st_mode) && statbuf.st_size > 0) { + if (sysfs_get_ulong(path, &size)) + total += size; + } + + free(path); + } + +out: + closedir(dp); + + return total; +} + +TEST_F(enclave, unclobbered_vdso_oversubscribed) +{ + unsigned long total_mem; + struct encl_op op; + + total_mem = get_total_epc_mem(); + ASSERT_NE(total_mem, 0); + ASSERT_TRUE(setup_test_encl(total_mem, &self->encl, _metadata)); + + memset(&self->run, 0, sizeof(self->run)); + self->run.tcs = self->encl.encl_base; + + op.type = ENCL_OP_PUT; + op.buffer = MAGIC; + + EXPECT_EQ(ENCL_CALL(&op, &self->run, false), 0); + + EXPECT_EEXIT(&self->run); + EXPECT_EQ(self->run.user_data, 0); + + op.type = ENCL_OP_GET; + op.buffer = 0; + + EXPECT_EQ(ENCL_CALL(&op, &self->run, false), 0); + + EXPECT_EQ(op.buffer, MAGIC); + EXPECT_EEXIT(&self->run); + EXPECT_EQ(self->run.user_data, 0); + +} + TEST_F(enclave, clobbered_vdso) { struct encl_op op; diff --git a/tools/testing/selftests/sgx/main.h b/tools/testing/selftests/sgx/main.h index b45c52ec7ab3..59027df32a6a 100644 --- a/tools/testing/selftests/sgx/main.h +++ b/tools/testing/selftests/sgx/main.h @@ -7,6 +7,7 @@ #define MAIN_H
#define ENCL_HEAP_SIZE_DEFAULT 4096 +#define SGX_TOTAL_MEM_PATH "/sys/devices/system/node"
struct encl_segment { void *src;
From: Jarkko Sakkinen jarkko@kernel.org
To add more operations to the test enclave, the protocol needs to allow to have operations with varying parameters. Create a separate parameter struct for each existing operation, with the shared parameters in struct encl_op_header.
Signed-off-by: Jarkko Sakkinen jarkko@kernel.org Acked-by: Dave Hansen dave.hansen@linux.intel.com [reinette: rebased to apply on top of oversubscription test series] Signed-off-by: Reinette Chatre reinette.chatre@intel.com --- Changes since V1: - Add signature from Dave.
tools/testing/selftests/sgx/defines.h | 14 ++++- tools/testing/selftests/sgx/main.c | 68 +++++++++++++------------ tools/testing/selftests/sgx/test_encl.c | 33 +++++++----- 3 files changed, 69 insertions(+), 46 deletions(-)
diff --git a/tools/testing/selftests/sgx/defines.h b/tools/testing/selftests/sgx/defines.h index f88562afcaa0..6ff95a766287 100644 --- a/tools/testing/selftests/sgx/defines.h +++ b/tools/testing/selftests/sgx/defines.h @@ -21,11 +21,21 @@ enum encl_op_type { ENCL_OP_PUT, ENCL_OP_GET, + ENCL_OP_MAX, };
-struct encl_op { +struct encl_op_header { uint64_t type; - uint64_t buffer; +}; + +struct encl_op_put { + struct encl_op_header header; + uint64_t value; +}; + +struct encl_op_get { + struct encl_op_header header; + uint64_t value; };
#endif /* DEFINES_H */ diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c index 541862c15901..2ac5c3300df2 100644 --- a/tools/testing/selftests/sgx/main.c +++ b/tools/testing/selftests/sgx/main.c @@ -222,27 +222,28 @@ FIXTURE_TEARDOWN(enclave)
TEST_F(enclave, unclobbered_vdso) { - struct encl_op op; + struct encl_op_put put_op; + struct encl_op_get get_op;
ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata));
memset(&self->run, 0, sizeof(self->run)); self->run.tcs = self->encl.encl_base;
- op.type = ENCL_OP_PUT; - op.buffer = MAGIC; + put_op.header.type = ENCL_OP_PUT; + put_op.value = MAGIC;
- EXPECT_EQ(ENCL_CALL(&op, &self->run, false), 0); + EXPECT_EQ(ENCL_CALL(&put_op, &self->run, false), 0);
EXPECT_EEXIT(&self->run); EXPECT_EQ(self->run.user_data, 0);
- op.type = ENCL_OP_GET; - op.buffer = 0; + get_op.header.type = ENCL_OP_GET; + get_op.value = 0;
- EXPECT_EQ(ENCL_CALL(&op, &self->run, false), 0); + EXPECT_EQ(ENCL_CALL(&get_op, &self->run, false), 0);
- EXPECT_EQ(op.buffer, MAGIC); + EXPECT_EQ(get_op.value, MAGIC); EXPECT_EEXIT(&self->run); EXPECT_EQ(self->run.user_data, 0); } @@ -331,7 +332,8 @@ static unsigned long get_total_epc_mem(void) TEST_F(enclave, unclobbered_vdso_oversubscribed) { unsigned long total_mem; - struct encl_op op; + struct encl_op_put put_op; + struct encl_op_get get_op;
total_mem = get_total_epc_mem(); ASSERT_NE(total_mem, 0); @@ -340,20 +342,20 @@ TEST_F(enclave, unclobbered_vdso_oversubscribed) memset(&self->run, 0, sizeof(self->run)); self->run.tcs = self->encl.encl_base;
- op.type = ENCL_OP_PUT; - op.buffer = MAGIC; + put_op.header.type = ENCL_OP_PUT; + put_op.value = MAGIC;
- EXPECT_EQ(ENCL_CALL(&op, &self->run, false), 0); + EXPECT_EQ(ENCL_CALL(&put_op, &self->run, false), 0);
EXPECT_EEXIT(&self->run); EXPECT_EQ(self->run.user_data, 0);
- op.type = ENCL_OP_GET; - op.buffer = 0; + get_op.header.type = ENCL_OP_GET; + get_op.value = 0;
- EXPECT_EQ(ENCL_CALL(&op, &self->run, false), 0); + EXPECT_EQ(ENCL_CALL(&get_op, &self->run, false), 0);
- EXPECT_EQ(op.buffer, MAGIC); + EXPECT_EQ(get_op.value, MAGIC); EXPECT_EEXIT(&self->run); EXPECT_EQ(self->run.user_data, 0);
@@ -361,27 +363,28 @@ TEST_F(enclave, unclobbered_vdso_oversubscribed)
TEST_F(enclave, clobbered_vdso) { - struct encl_op op; + struct encl_op_put put_op; + struct encl_op_get get_op;
ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata));
memset(&self->run, 0, sizeof(self->run)); self->run.tcs = self->encl.encl_base;
- op.type = ENCL_OP_PUT; - op.buffer = MAGIC; + put_op.header.type = ENCL_OP_PUT; + put_op.value = MAGIC;
- EXPECT_EQ(ENCL_CALL(&op, &self->run, true), 0); + EXPECT_EQ(ENCL_CALL(&put_op, &self->run, true), 0);
EXPECT_EEXIT(&self->run); EXPECT_EQ(self->run.user_data, 0);
- op.type = ENCL_OP_GET; - op.buffer = 0; + get_op.header.type = ENCL_OP_GET; + get_op.value = 0;
- EXPECT_EQ(ENCL_CALL(&op, &self->run, true), 0); + EXPECT_EQ(ENCL_CALL(&get_op, &self->run, true), 0);
- EXPECT_EQ(op.buffer, MAGIC); + EXPECT_EQ(get_op.value, MAGIC); EXPECT_EEXIT(&self->run); EXPECT_EQ(self->run.user_data, 0); } @@ -396,7 +399,8 @@ static int test_handler(long rdi, long rsi, long rdx, long ursp, long r8, long r
TEST_F(enclave, clobbered_vdso_and_user_function) { - struct encl_op op; + struct encl_op_put put_op; + struct encl_op_get get_op;
ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata));
@@ -406,20 +410,20 @@ TEST_F(enclave, clobbered_vdso_and_user_function) self->run.user_handler = (__u64)test_handler; self->run.user_data = 0xdeadbeef;
- op.type = ENCL_OP_PUT; - op.buffer = MAGIC; + put_op.header.type = ENCL_OP_PUT; + put_op.value = MAGIC;
- EXPECT_EQ(ENCL_CALL(&op, &self->run, true), 0); + EXPECT_EQ(ENCL_CALL(&put_op, &self->run, true), 0);
EXPECT_EEXIT(&self->run); EXPECT_EQ(self->run.user_data, 0);
- op.type = ENCL_OP_GET; - op.buffer = 0; + get_op.header.type = ENCL_OP_GET; + get_op.value = 0;
- EXPECT_EQ(ENCL_CALL(&op, &self->run, true), 0); + EXPECT_EQ(ENCL_CALL(&get_op, &self->run, true), 0);
- EXPECT_EQ(op.buffer, MAGIC); + EXPECT_EQ(get_op.value, MAGIC); EXPECT_EEXIT(&self->run); EXPECT_EQ(self->run.user_data, 0); } diff --git a/tools/testing/selftests/sgx/test_encl.c b/tools/testing/selftests/sgx/test_encl.c index 734ea52f9924..f11eb8315704 100644 --- a/tools/testing/selftests/sgx/test_encl.c +++ b/tools/testing/selftests/sgx/test_encl.c @@ -16,20 +16,29 @@ static void *memcpy(void *dest, const void *src, size_t n) return dest; }
-void encl_body(void *rdi, void *rsi) +static void do_encl_op_put(void *op) +{ + struct encl_op_put *op2 = op; + + memcpy(&encl_buffer[0], &op2->value, 8); +} + +static void do_encl_op_get(void *op) { - struct encl_op *op = (struct encl_op *)rdi; + struct encl_op_get *op2 = op;
- switch (op->type) { - case ENCL_OP_PUT: - memcpy(&encl_buffer[0], &op->buffer, 8); - break; + memcpy(&op2->value, &encl_buffer[0], 8); +} + +void encl_body(void *rdi, void *rsi) +{ + const void (*encl_op_array[ENCL_OP_MAX])(void *) = { + do_encl_op_put, + do_encl_op_get, + };
- case ENCL_OP_GET: - memcpy(&op->buffer, &encl_buffer[0], 8); - break; + struct encl_op_header *op = (struct encl_op_header *)rdi;
- default: - break; - } + if (op->type < ENCL_OP_MAX) + (*encl_op_array[op->type])(op); }
SGX selftests prepares a data structure outside of the enclave with the type of and data for the operation that needs to be run within the enclave. At this time only two complementary operations are supported by the enclave: copying a value from outside the enclave into a default buffer within the enclave and reading a value from the enclave's default buffer into a variable accessible outside the enclave.
In preparation for more operations supported by the enclave the names of the current enclave operations are changed to more accurately reflect the operations and more easily distinguish it from future operations:
* The enums ENCL_OP_PUT and ENCL_OP_GET are renamed to ENCL_OP_PUT_TO_BUFFER and ENCL_OP_GET_FROM_BUFFER respectively. * The structs encl_op_put and encl_op_get are renamed to encl_op_put_to_buf and encl_op_get_from_buf respectively. * The enclave functions do_encl_op_put and do_encl_op_get are renamed to do_encl_op_put_to_buf and do_encl_op_get_from_buf respectively.
No functional changes.
Suggested-by: Jarkko Sakkinen jarkko@kernel.org Acked-by: Jarkko Sakkinen jarkko@kernel.org Acked-by: Dave Hansen dave.hansen@linux.intel.com Signed-off-by: Reinette Chatre reinette.chatre@intel.com --- Changes since V1: - Add Jarkko and Dave's signatures.
tools/testing/selftests/sgx/defines.h | 8 +++---- tools/testing/selftests/sgx/main.c | 32 ++++++++++++------------- tools/testing/selftests/sgx/test_encl.c | 12 +++++----- 3 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/tools/testing/selftests/sgx/defines.h b/tools/testing/selftests/sgx/defines.h index 6ff95a766287..9ea0c7882dfb 100644 --- a/tools/testing/selftests/sgx/defines.h +++ b/tools/testing/selftests/sgx/defines.h @@ -19,8 +19,8 @@ #include "../../../../arch/x86/include/uapi/asm/sgx.h"
enum encl_op_type { - ENCL_OP_PUT, - ENCL_OP_GET, + ENCL_OP_PUT_TO_BUFFER, + ENCL_OP_GET_FROM_BUFFER, ENCL_OP_MAX, };
@@ -28,12 +28,12 @@ struct encl_op_header { uint64_t type; };
-struct encl_op_put { +struct encl_op_put_to_buf { struct encl_op_header header; uint64_t value; };
-struct encl_op_get { +struct encl_op_get_from_buf { struct encl_op_header header; uint64_t value; }; diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c index 2ac5c3300df2..f1802faed78e 100644 --- a/tools/testing/selftests/sgx/main.c +++ b/tools/testing/selftests/sgx/main.c @@ -222,15 +222,15 @@ FIXTURE_TEARDOWN(enclave)
TEST_F(enclave, unclobbered_vdso) { - struct encl_op_put put_op; - struct encl_op_get get_op; + 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));
memset(&self->run, 0, sizeof(self->run)); self->run.tcs = self->encl.encl_base;
- put_op.header.type = ENCL_OP_PUT; + put_op.header.type = ENCL_OP_PUT_TO_BUFFER; put_op.value = MAGIC;
EXPECT_EQ(ENCL_CALL(&put_op, &self->run, false), 0); @@ -238,7 +238,7 @@ TEST_F(enclave, unclobbered_vdso) EXPECT_EEXIT(&self->run); EXPECT_EQ(self->run.user_data, 0);
- get_op.header.type = ENCL_OP_GET; + get_op.header.type = ENCL_OP_GET_FROM_BUFFER; get_op.value = 0;
EXPECT_EQ(ENCL_CALL(&get_op, &self->run, false), 0); @@ -331,9 +331,9 @@ static unsigned long get_total_epc_mem(void)
TEST_F(enclave, unclobbered_vdso_oversubscribed) { + struct encl_op_get_from_buf get_op; + struct encl_op_put_to_buf put_op; unsigned long total_mem; - struct encl_op_put put_op; - struct encl_op_get get_op;
total_mem = get_total_epc_mem(); ASSERT_NE(total_mem, 0); @@ -342,7 +342,7 @@ TEST_F(enclave, unclobbered_vdso_oversubscribed) memset(&self->run, 0, sizeof(self->run)); self->run.tcs = self->encl.encl_base;
- put_op.header.type = ENCL_OP_PUT; + put_op.header.type = ENCL_OP_PUT_TO_BUFFER; put_op.value = MAGIC;
EXPECT_EQ(ENCL_CALL(&put_op, &self->run, false), 0); @@ -350,7 +350,7 @@ TEST_F(enclave, unclobbered_vdso_oversubscribed) EXPECT_EEXIT(&self->run); EXPECT_EQ(self->run.user_data, 0);
- get_op.header.type = ENCL_OP_GET; + get_op.header.type = ENCL_OP_GET_FROM_BUFFER; get_op.value = 0;
EXPECT_EQ(ENCL_CALL(&get_op, &self->run, false), 0); @@ -363,15 +363,15 @@ TEST_F(enclave, unclobbered_vdso_oversubscribed)
TEST_F(enclave, clobbered_vdso) { - struct encl_op_put put_op; - struct encl_op_get get_op; + 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));
memset(&self->run, 0, sizeof(self->run)); self->run.tcs = self->encl.encl_base;
- put_op.header.type = ENCL_OP_PUT; + put_op.header.type = ENCL_OP_PUT_TO_BUFFER; put_op.value = MAGIC;
EXPECT_EQ(ENCL_CALL(&put_op, &self->run, true), 0); @@ -379,7 +379,7 @@ TEST_F(enclave, clobbered_vdso) EXPECT_EEXIT(&self->run); EXPECT_EQ(self->run.user_data, 0);
- get_op.header.type = ENCL_OP_GET; + get_op.header.type = ENCL_OP_GET_FROM_BUFFER; get_op.value = 0;
EXPECT_EQ(ENCL_CALL(&get_op, &self->run, true), 0); @@ -399,8 +399,8 @@ static int test_handler(long rdi, long rsi, long rdx, long ursp, long r8, long r
TEST_F(enclave, clobbered_vdso_and_user_function) { - struct encl_op_put put_op; - struct encl_op_get get_op; + 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));
@@ -410,7 +410,7 @@ TEST_F(enclave, clobbered_vdso_and_user_function) self->run.user_handler = (__u64)test_handler; self->run.user_data = 0xdeadbeef;
- put_op.header.type = ENCL_OP_PUT; + put_op.header.type = ENCL_OP_PUT_TO_BUFFER; put_op.value = MAGIC;
EXPECT_EQ(ENCL_CALL(&put_op, &self->run, true), 0); @@ -418,7 +418,7 @@ TEST_F(enclave, clobbered_vdso_and_user_function) EXPECT_EEXIT(&self->run); EXPECT_EQ(self->run.user_data, 0);
- get_op.header.type = ENCL_OP_GET; + get_op.header.type = ENCL_OP_GET_FROM_BUFFER; get_op.value = 0;
EXPECT_EQ(ENCL_CALL(&get_op, &self->run, true), 0); diff --git a/tools/testing/selftests/sgx/test_encl.c b/tools/testing/selftests/sgx/test_encl.c index f11eb8315704..4e8da738173f 100644 --- a/tools/testing/selftests/sgx/test_encl.c +++ b/tools/testing/selftests/sgx/test_encl.c @@ -16,16 +16,16 @@ static void *memcpy(void *dest, const void *src, size_t n) return dest; }
-static void do_encl_op_put(void *op) +static void do_encl_op_put_to_buf(void *op) { - struct encl_op_put *op2 = op; + struct encl_op_put_to_buf *op2 = op;
memcpy(&encl_buffer[0], &op2->value, 8); }
-static void do_encl_op_get(void *op) +static void do_encl_op_get_from_buf(void *op) { - struct encl_op_get *op2 = op; + struct encl_op_get_from_buf *op2 = op;
memcpy(&op2->value, &encl_buffer[0], 8); } @@ -33,8 +33,8 @@ static void do_encl_op_get(void *op) void encl_body(void *rdi, void *rsi) { const void (*encl_op_array[ENCL_OP_MAX])(void *) = { - do_encl_op_put, - do_encl_op_get, + do_encl_op_put_to_buf, + do_encl_op_get_from_buf, };
struct encl_op_header *op = (struct encl_op_header *)rdi;
The Enclave Page Cache Map (EPCM) is a secure structure used by the processor to track the contents of the enclave page cache. The EPCM contains permissions with which enclave pages can be accessed. SGX support allows EPCM and PTE page permissions to differ - as long as the PTE permissions do not exceed the EPCM permissions.
Add a test that: (1) Creates an SGX enclave page with writable EPCM permission. (2) Changes the PTE permission on the page to read-only. This should be permitted because the permission does not exceed the EPCM permission. (3) Attempts a write to the page. This should generate a page fault (#PF) because of the read-only PTE even though the EPCM permissions allow the page to be written to.
This introduces the first test of SGX exception handling. In this test the issue that caused the exception (PTE page permissions) can be fixed from outside the enclave and after doing so it is possible to re-enter enclave at original entrypoint with ERESUME.
Acked-by: Dave Hansen dave.hansen@linux.intel.com Signed-off-by: Reinette Chatre reinette.chatre@intel.com --- Changes since V1: - Make changelog more readable (Dave). - Add signature from Dave. - Improve loop locating data segment (Jarkko).
tools/testing/selftests/sgx/defines.h | 14 +++ tools/testing/selftests/sgx/main.c | 134 ++++++++++++++++++++++++ tools/testing/selftests/sgx/test_encl.c | 21 ++++ 3 files changed, 169 insertions(+)
diff --git a/tools/testing/selftests/sgx/defines.h b/tools/testing/selftests/sgx/defines.h index 9ea0c7882dfb..0bbda6f0c7d3 100644 --- a/tools/testing/selftests/sgx/defines.h +++ b/tools/testing/selftests/sgx/defines.h @@ -21,6 +21,8 @@ enum encl_op_type { ENCL_OP_PUT_TO_BUFFER, ENCL_OP_GET_FROM_BUFFER, + ENCL_OP_PUT_TO_ADDRESS, + ENCL_OP_GET_FROM_ADDRESS, ENCL_OP_MAX, };
@@ -38,4 +40,16 @@ struct encl_op_get_from_buf { uint64_t value; };
+struct encl_op_put_to_addr { + struct encl_op_header header; + uint64_t value; + uint64_t addr; +}; + +struct encl_op_get_from_addr { + struct encl_op_header header; + uint64_t value; + uint64_t addr; +}; + #endif /* DEFINES_H */ diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c index f1802faed78e..9efb619e0393 100644 --- a/tools/testing/selftests/sgx/main.c +++ b/tools/testing/selftests/sgx/main.c @@ -23,6 +23,7 @@ #include "main.h"
static const uint64_t MAGIC = 0x1122334455667788ULL; +static const uint64_t MAGIC2 = 0x8877665544332211ULL; vdso_sgx_enter_enclave_t vdso_sgx_enter_enclave;
struct vdso_symtab { @@ -109,6 +110,25 @@ static Elf64_Sym *vdso_symtab_get(struct vdso_symtab *symtab, const char *name) return NULL; }
+/* + * Return the offset in the enclave where the data segment can be found. + * The first RW segment loaded is the TCS, skip that to get info on the + * data segment. + */ +static off_t encl_get_data_offset(struct encl *encl) +{ + int i; + + for (i = 1; i < encl->nr_segments; i++) { + struct encl_segment *seg = &encl->segment_tbl[i]; + + if (seg->prot == (PROT_READ | PROT_WRITE)) + return seg->offset; + } + + return -1; +} + FIXTURE(enclave) { struct encl encl; struct sgx_enclave_run run; @@ -428,4 +448,118 @@ TEST_F(enclave, clobbered_vdso_and_user_function) EXPECT_EQ(self->run.user_data, 0); }
+/* + * Second page of .data segment is used to test changing PTE permissions. + * This spans the local encl_buffer within the test enclave. + * + * 1) Start with a sanity check: a value is written to the target page within + * the enclave and read back to ensure target page can be written to. + * 2) Change PTE permissions (RW -> RO) of target page within enclave. + * 3) Repeat (1) - this time expecting a regular #PF communicated via the + * vDSO. + * 4) Change PTE permissions of target page within enclave back to be RW. + * 5) Repeat (1) by resuming enclave, now expected to be possible to write to + * and read from target page within enclave. + */ +TEST_F(enclave, pte_permissions) +{ + struct encl_op_get_from_addr get_addr_op; + struct encl_op_put_to_addr put_addr_op; + unsigned long data_start; + int ret; + + ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata)); + + memset(&self->run, 0, sizeof(self->run)); + self->run.tcs = self->encl.encl_base; + + data_start = self->encl.encl_base + + encl_get_data_offset(&self->encl) + + PAGE_SIZE; + + /* + * Sanity check to ensure it is possible to write to page that will + * have its permissions manipulated. + */ + + /* Write MAGIC to page */ + put_addr_op.value = MAGIC; + put_addr_op.addr = data_start; + 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 that was just written to, confirming that it is the + * value previously written (MAGIC). + */ + get_addr_op.value = 0; + get_addr_op.addr = data_start; + 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); + + /* Change PTE permissions of target page within the enclave */ + ret = mprotect((void *)data_start, PAGE_SIZE, PROT_READ); + if (ret) + perror("mprotect"); + + /* + * PTE permissions of target page changed to read-only, EPCM + * permissions unchanged (EPCM permissions are RW), attempt to + * write to the page, expecting a regular #PF. + */ + + put_addr_op.value = MAGIC2; + + EXPECT_EQ(ENCL_CALL(&put_addr_op, &self->run, true), 0); + + EXPECT_EQ(self->run.exception_vector, 14); + EXPECT_EQ(self->run.exception_error_code, 0x7); + EXPECT_EQ(self->run.exception_addr, data_start); + + self->run.exception_vector = 0; + self->run.exception_error_code = 0; + self->run.exception_addr = 0; + + /* + * Change PTE permissions back to enable enclave to write to the + * target page and resume enclave - do not expect any exceptions this + * time. + */ + ret = mprotect((void *)data_start, PAGE_SIZE, PROT_READ | PROT_WRITE); + if (ret) + perror("mprotect"); + + EXPECT_EQ(vdso_sgx_enter_enclave((unsigned long)&put_addr_op, 0, + 0, ERESUME, 0, 0, &self->run), + 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); + + get_addr_op.value = 0; + + EXPECT_EQ(ENCL_CALL(&get_addr_op, &self->run, true), 0); + + EXPECT_EQ(get_addr_op.value, MAGIC2); + 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); +} + TEST_HARNESS_MAIN diff --git a/tools/testing/selftests/sgx/test_encl.c b/tools/testing/selftests/sgx/test_encl.c index 4e8da738173f..5d86e3e6456a 100644 --- a/tools/testing/selftests/sgx/test_encl.c +++ b/tools/testing/selftests/sgx/test_encl.c @@ -4,6 +4,11 @@ #include <stddef.h> #include "defines.h"
+/* + * Data buffer spanning two pages that will be placed first in .data + * segment. Even if not used internally the second page is needed by + * external test manipulating page permissions. + */ static uint8_t encl_buffer[8192] = { 1 };
static void *memcpy(void *dest, const void *src, size_t n) @@ -30,11 +35,27 @@ static void do_encl_op_get_from_buf(void *op) memcpy(&op2->value, &encl_buffer[0], 8); }
+static void do_encl_op_put_to_addr(void *_op) +{ + struct encl_op_put_to_addr *op = _op; + + memcpy((void *)op->addr, &op->value, 8); +} + +static void do_encl_op_get_from_addr(void *_op) +{ + struct encl_op_get_from_addr *op = _op; + + memcpy(&op->value, (void *)op->addr, 8); +} + void encl_body(void *rdi, void *rsi) { const void (*encl_op_array[ENCL_OP_MAX])(void *) = { do_encl_op_put_to_buf, do_encl_op_get_from_buf, + do_encl_op_put_to_addr, + do_encl_op_get_from_addr, };
struct encl_op_header *op = (struct encl_op_header *)rdi;
On Thu, Oct 28, 2021 at 01:37:38PM -0700, Reinette Chatre wrote:
The Enclave Page Cache Map (EPCM) is a secure structure used by the processor to track the contents of the enclave page cache. The EPCM contains permissions with which enclave pages can be accessed. SGX support allows EPCM and PTE page permissions to differ - as long as the PTE permissions do not exceed the EPCM permissions.
Add a test that: (1) Creates an SGX enclave page with writable EPCM permission. (2) Changes the PTE permission on the page to read-only. This should be permitted because the permission does not exceed the EPCM permission. (3) Attempts a write to the page. This should generate a page fault (#PF) because of the read-only PTE even though the EPCM permissions allow the page to be written to.
This introduces the first test of SGX exception handling. In this test the issue that caused the exception (PTE page permissions) can be fixed from outside the enclave and after doing so it is possible to re-enter enclave at original entrypoint with ERESUME.
Acked-by: Dave Hansen dave.hansen@linux.intel.com Signed-off-by: Reinette Chatre reinette.chatre@intel.com
Reviewed-by: Jarkko Sakkinen jarkko@kernel.org
/Jarkko
Each thread executing in an enclave is associated with a Thread Control Structure (TCS). The test enclave contains two hardcoded TCS. Each TCS contains meta-data used by the hardware to save and restore thread specific information when entering/exiting the enclave.
The two TCS structures within the test enclave share their SSA (State Save Area) resulting in the threads clobbering each other's data. Fix this by providing each TCS their own SSA area.
Additionally, there is an 8K stack space and its address is computed from the enclave entry point which is correctly done for TCS #1 that starts on the first address inside the enclave but results in out of bounds memory when entering as TCS #2. Split 8K stack space into two separate pages with offset symbol between to ensure the current enclave entry calculation can continue to be used for both threads.
While using the enclave with multiple threads requires these fixes the impact is not apparent because every test up to this point enters the enclave from the first TCS.
More detail about the stack fix: ------------------------------- Before this change the test enclave (test_encl) looks as follows:
.tcs (2 pages): (page 1) TCS #1 (page 2) TCS #2
.text (1 page) One page of code
.data (5 pages) (page 1) encl_buffer (page 2) encl_buffer (page 3) SSA (page 4 and 5) STACK encl_stack:
As shown above there is a symbol, encl_stack, that points to the end of the .data segment (pointing to the end of page 5 in .data) which is also the end of the enclave.
The enclave entry code computes the stack address by adding encl_stack to the pointer to the TCS that entered the enclave. When entering at TCS #1 the stack is computed correctly but when entering at TCS #2 the stack pointer would point to one page beyond the end of the enclave and a #PF would result when TCS #2 attempts to enter the enclave.
The fix involves moving the encl_stack symbol between the two stack pages. Doing so enables the stack address computation in the entry code to compute the correct stack address for each TCS.
Reviewed-by: Jarkko Sakkinen jarkko@kernel.org Acked-by: Dave Hansen dave.hansen@linux.intel.com Signed-off-by: Reinette Chatre reinette.chatre@intel.com --- Changes since V1: - Add Jarkko and Dave's signatures.
.../selftests/sgx/test_encl_bootstrap.S | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/sgx/test_encl_bootstrap.S b/tools/testing/selftests/sgx/test_encl_bootstrap.S index 5d5680d4ea39..82fb0dfcbd23 100644 --- a/tools/testing/selftests/sgx/test_encl_bootstrap.S +++ b/tools/testing/selftests/sgx/test_encl_bootstrap.S @@ -12,7 +12,7 @@
.fill 1, 8, 0 # STATE (set by CPU) .fill 1, 8, 0 # FLAGS - .quad encl_ssa # OSSA + .quad encl_ssa_tcs1 # OSSA .fill 1, 4, 0 # CSSA (set by CPU) .fill 1, 4, 1 # NSSA .quad encl_entry # OENTRY @@ -23,10 +23,10 @@ .fill 1, 4, 0xFFFFFFFF # GSLIMIT .fill 4024, 1, 0 # Reserved
- # Identical to the previous TCS. + # TCS2 .fill 1, 8, 0 # STATE (set by CPU) .fill 1, 8, 0 # FLAGS - .quad encl_ssa # OSSA + .quad encl_ssa_tcs2 # OSSA .fill 1, 4, 0 # CSSA (set by CPU) .fill 1, 4, 1 # NSSA .quad encl_entry # OENTRY @@ -40,8 +40,9 @@ .text
encl_entry: - # RBX contains the base address for TCS, which is also the first address - # inside the enclave. By adding the value of le_stack_end to it, we get + # RBX contains the base address for TCS, which is the first address + # inside the enclave for TCS #1 and one page into the enclave for + # TCS #2. By adding the value of encl_stack to it, we get # the absolute address for the stack. lea (encl_stack)(%rbx), %rax xchg %rsp, %rax @@ -81,9 +82,15 @@ encl_entry:
.section ".data", "aw"
-encl_ssa: +encl_ssa_tcs1: + .space 4096 +encl_ssa_tcs2: .space 4096
.balign 4096 - .space 8192 + # Stack of TCS #1 + .space 4096 encl_stack: + .balign 4096 + # Stack of TCS #2 + .space 4096
Each thread executing in an enclave is associated with a Thread Control Structure (TCS). The SGX test enclave contains two hardcoded TCS, thus supporting two threads in the enclave.
Add a test to ensure it is possible to enter enclave at both entrypoints.
Reviewed-by: Jarkko Sakkinen jarkko@kernel.org Acked-by: Dave Hansen dave.hansen@linux.intel.com Signed-off-by: Reinette Chatre reinette.chatre@intel.com --- Changes since V1: - Add Jarkko and Dave's signatures.
tools/testing/selftests/sgx/defines.h | 1 + tools/testing/selftests/sgx/main.c | 32 +++++++++++++++++++++++++ tools/testing/selftests/sgx/test_encl.c | 6 +++++ 3 files changed, 39 insertions(+)
diff --git a/tools/testing/selftests/sgx/defines.h b/tools/testing/selftests/sgx/defines.h index 0bbda6f0c7d3..02d775789ea7 100644 --- a/tools/testing/selftests/sgx/defines.h +++ b/tools/testing/selftests/sgx/defines.h @@ -23,6 +23,7 @@ enum encl_op_type { ENCL_OP_GET_FROM_BUFFER, ENCL_OP_PUT_TO_ADDRESS, ENCL_OP_GET_FROM_ADDRESS, + ENCL_OP_NOP, ENCL_OP_MAX, };
diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c index 9efb619e0393..a86796d70a7b 100644 --- a/tools/testing/selftests/sgx/main.c +++ b/tools/testing/selftests/sgx/main.c @@ -448,6 +448,38 @@ TEST_F(enclave, clobbered_vdso_and_user_function) EXPECT_EQ(self->run.user_data, 0); }
+/* + * Sanity check that it is possible to enter either of the two hardcoded TCS + */ +TEST_F(enclave, tcs_entry) +{ + struct encl_op_header op; + + ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata)); + + memset(&self->run, 0, sizeof(self->run)); + self->run.tcs = self->encl.encl_base; + + op.type = ENCL_OP_NOP; + + EXPECT_EQ(ENCL_CALL(&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); + + /* Move to the next TCS. */ + self->run.tcs = self->encl.encl_base + PAGE_SIZE; + + EXPECT_EQ(ENCL_CALL(&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); +} + /* * Second page of .data segment is used to test changing PTE permissions. * This spans the local encl_buffer within the test enclave. diff --git a/tools/testing/selftests/sgx/test_encl.c b/tools/testing/selftests/sgx/test_encl.c index 5d86e3e6456a..4fca01cfd898 100644 --- a/tools/testing/selftests/sgx/test_encl.c +++ b/tools/testing/selftests/sgx/test_encl.c @@ -49,6 +49,11 @@ static void do_encl_op_get_from_addr(void *_op) memcpy(&op->value, (void *)op->addr, 8); }
+static void do_encl_op_nop(void *_op) +{ + +} + void encl_body(void *rdi, void *rsi) { const void (*encl_op_array[ENCL_OP_MAX])(void *) = { @@ -56,6 +61,7 @@ void encl_body(void *rdi, void *rsi) do_encl_op_get_from_buf, do_encl_op_put_to_addr, do_encl_op_get_from_addr, + do_encl_op_nop, };
struct encl_op_header *op = (struct encl_op_header *)rdi;
linux-kselftest-mirror@lists.linaro.org
-
Dave Hansen -
Jarkko Sakkinen -
Reinette Chatre -
Sean Christopherson