On Fri, Jan 28, 2022 at 10:23:56AM -0800, Reinette Chatre wrote:
A segfault is encountered if there happens to be an early failure of any of the SGX tests. One way to reproduce this is to remove the enclave binary "test_encl.elf" that will trigger early enclave loading failure followed by a segfault.
The segfault occurs within encl_delete() that cleans up after an enclave by umapping its mapped regions and closing the file descriptor to the SGX driver. As integrated with the kselftest harness encl_delete() is called upon exit from every test, irrespective of test success. encl_delete() is also called to clean up if an error is encountered during enclave loading.
encl_delete() is thus responsible for cleaning any amount of enclave state - including state that has already been cleaned.
encl_delete() starts by accessing encl->segment_tbl that may not have been created yet due to a very early failure or may already be cleaned up because of a failure encountered after encl->segment_tbl was created.
Ensure encl->segment_tbl is valid before attempting to access memory offset from it. The offset with which it is accessed, encl->nr_segments, is initialized after encl->segment_tbl and thus considered valid to use after the encl->segment_tbl check succeeds.
Nit: textwidth=75
Not something I would NAK but just saying.
Fixes: 3200505d4de6 ("selftests/sgx: Create a heap for the test enclave") Signed-off-by: Reinette Chatre reinette.chatre@intel.com
tools/testing/selftests/sgx/load.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/sgx/load.c b/tools/testing/selftests/sgx/load.c index 9d4322c946e2..006b464c8fc9 100644 --- a/tools/testing/selftests/sgx/load.c +++ b/tools/testing/selftests/sgx/load.c @@ -21,7 +21,7 @@ void encl_delete(struct encl *encl) {
- struct encl_segment *heap_seg = &encl->segment_tbl[encl->nr_segments - 1];
- struct encl_segment *heap_seg;
if (encl->encl_base) munmap((void *)encl->encl_base, encl->encl_size); @@ -32,10 +32,11 @@ void encl_delete(struct encl *encl) if (encl->fd) close(encl->fd);
- munmap(heap_seg->src, heap_seg->size);
- if (encl->segment_tbl)
- if (encl->segment_tbl) {
heap_seg = &encl->segment_tbl[encl->nr_segments - 1];munmap(heap_seg->src, heap_seg->size);- }
memset(encl, 0, sizeof(*encl)); } -- 2.25.1
Reviewed-by: Jarkko Sakkinen jarkko@kernel.org Tested-by: Jarkko Sakkinen jarkko@kernel.org
I tested this by a minor code mod to trigger to failure case.
/Jarkko