All of lore.kernel.org
 help / color / mirror / Atom feed
From: Reinette Chatre <reinette.chatre@intel.com>
To: dave.hansen@linux.intel.com, jarkko@kernel.org,
	tglx@linutronix.de, bp@alien8.de, luto@kernel.org,
	mingo@redhat.com, linux-sgx@vger.kernel.org, x86@kernel.org
Cc: seanjc@google.com, kai.huang@intel.com, cathy.zhang@intel.com,
	cedric.xing@intel.com, haitao.huang@intel.com,
	mark.shanahan@intel.com, hpa@zytor.com,
	linux-kernel@vger.kernel.org
Subject: [PATCH V3 27/30] selftests/sgx: Test faulty enclave behavior
Date: Mon,  4 Apr 2022 09:49:35 -0700	[thread overview]
Message-ID: <2532238125e82343e26a6610241c7926e620527f.1648847675.git.reinette.chatre@intel.com> (raw)
In-Reply-To: <cover.1648847675.git.reinette.chatre@intel.com>

Removing a page from an initialized enclave involves three steps:
first the user requests changing the page type to SGX_PAGE_TYPE_TRIM
via an ioctl(), on success the ENCLU[EACCEPT] instruction needs to be
run from within the enclave to accept the page removal, finally the
user requests page removal to be completed via an ioctl(). Only after
acceptance (ENCLU[EACCEPT]) from within the enclave can the kernel
remove the page from a running enclave.

Test the behavior when the user's request to change the page type
succeeds, but the ENCLU[EACCEPT] instruction is not run before the
ioctl() requesting page removal is run. This should not be permitted.

Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
---
Changes since V2:
- Rename struct sgx_enclave_modt -> struct sgx_enclave_modify_type

Changes since V1:
- Update to support ioctl() name change (SGX_IOC_PAGE_MODT ->
  SGX_IOC_ENCLAVE_MODIFY_TYPE) and provide secinfo as parameter instead
  of just page type (Jarkko).
- Update test to reflect page removal ioctl() and struct name change:
  SGX_IOC_PAGE_REMOVE->SGX_IOC_ENCLAVE_REMOVE_PAGES,
  struct sgx_page_remove -> struct sgx_enclave_remove_pages (Jarkko).
- Use ioctl() instead of ioctl in text (Dave).

 tools/testing/selftests/sgx/main.c | 116 +++++++++++++++++++++++++++++
 1 file changed, 116 insertions(+)

diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c
index 17ade940425b..f6a8e2dd4a23 100644
--- a/tools/testing/selftests/sgx/main.c
+++ b/tools/testing/selftests/sgx/main.c
@@ -1444,4 +1444,120 @@ TEST_F(enclave, tcs_create)
 	munmap(addr, 3 * PAGE_SIZE);
 }
 
+/*
+ * Ensure sane behavior if user requests page removal, does not run
+ * EACCEPT from within enclave but still attempts to finalize page removal
+ * with the SGX_IOC_ENCLAVE_REMOVE_PAGES ioctl(). The latter should fail
+ * because the removal was not EACCEPTed from within the enclave.
+ */
+TEST_F(enclave, remove_added_page_no_eaccept)
+{
+	struct sgx_enclave_remove_pages remove_ioc;
+	struct encl_op_get_from_addr get_addr_op;
+	struct sgx_enclave_modify_type modt_ioc;
+	struct encl_op_put_to_addr put_addr_op;
+	struct sgx_secinfo secinfo;
+	unsigned long data_start;
+	int ret, errno_save;
+
+	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;
+
+	/*
+	 * Hardware (SGX2) and kernel support is needed for this test. Start
+	 * with check that test has a chance of succeeding.
+	 */
+	memset(&modt_ioc, 0, sizeof(modt_ioc));
+	ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_MODIFY_TYPE, &modt_ioc);
+
+	if (ret == -1) {
+		if (errno == ENOTTY)
+			SKIP(return, "Kernel does not support SGX_IOC_ENCLAVE_MODIFY_TYPE ioctl()");
+		else if (errno == ENODEV)
+			SKIP(return, "System does not support SGX2");
+	}
+
+	/*
+	 * Invalid parameters were provided during sanity check,
+	 * expect command to fail.
+	 */
+	EXPECT_EQ(ret, -1);
+
+	/*
+	 * Page that will be removed is the second data page in the .data
+	 * segment. This forms part of the local encl_buffer within the
+	 * enclave.
+	 */
+	data_start = self->encl.encl_base +
+		     encl_get_data_offset(&self->encl) + PAGE_SIZE;
+
+	/*
+	 * Sanity check that page at @data_start is writable before
+	 * removing it.
+	 *
+	 * Start by writing MAGIC to test 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 data
+	 * previously written (MAGIC) is present.
+	 */
+	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);
+
+	/* Start page removal by requesting change of page type to PT_TRIM */
+	memset(&modt_ioc, 0, sizeof(modt_ioc));
+	memset(&secinfo, 0, sizeof(secinfo));
+
+	secinfo.flags = SGX_PAGE_TYPE_TRIM << 8;
+	modt_ioc.offset = encl_get_data_offset(&self->encl) + PAGE_SIZE;
+	modt_ioc.length = PAGE_SIZE;
+	modt_ioc.secinfo = (unsigned long)&secinfo;
+
+	ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_MODIFY_TYPE, &modt_ioc);
+	errno_save = ret == -1 ? errno : 0;
+
+	EXPECT_EQ(ret, 0);
+	EXPECT_EQ(errno_save, 0);
+	EXPECT_EQ(modt_ioc.result, 0);
+	EXPECT_EQ(modt_ioc.count, 4096);
+
+	/* Skip EACCEPT */
+
+	/* Send final ioctl() to complete page removal */
+	memset(&remove_ioc, 0, sizeof(remove_ioc));
+
+	remove_ioc.offset = encl_get_data_offset(&self->encl) + PAGE_SIZE;
+	remove_ioc.length = PAGE_SIZE;
+
+	ret = ioctl(self->encl.fd, SGX_IOC_ENCLAVE_REMOVE_PAGES, &remove_ioc);
+	errno_save = ret == -1 ? errno : 0;
+
+	/* Operation not permitted since EACCEPT was omitted. */
+	EXPECT_EQ(ret, -1);
+	EXPECT_EQ(errno_save, EPERM);
+	EXPECT_EQ(remove_ioc.count, 0);
+}
+
 TEST_HARNESS_MAIN
-- 
2.25.1


  parent reply	other threads:[~2022-04-04 21:18 UTC|newest]

Thread overview: 79+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-04 16:49 [PATCH V3 00/30] x86/sgx and selftests/sgx: Support SGX2 Reinette Chatre
2022-04-04 16:49 ` [PATCH V3 01/30] x86/sgx: Add short descriptions to ENCLS wrappers Reinette Chatre
2022-04-05  6:52   ` Jarkko Sakkinen
2022-04-04 16:49 ` [PATCH V3 02/30] x86/sgx: Add wrapper for SGX2 EMODPR function Reinette Chatre
2022-04-05  6:53   ` Jarkko Sakkinen
2022-04-04 16:49 ` [PATCH V3 03/30] x86/sgx: Add wrapper for SGX2 EMODT function Reinette Chatre
2022-04-05  6:53   ` Jarkko Sakkinen
2022-04-04 16:49 ` [PATCH V3 04/30] x86/sgx: Add wrapper for SGX2 EAUG function Reinette Chatre
2022-04-05  6:54   ` Jarkko Sakkinen
2022-04-04 16:49 ` [PATCH V3 05/30] x86/sgx: Support loading enclave page without VMA permissions check Reinette Chatre
2022-04-05  6:56   ` Jarkko Sakkinen
2022-04-04 16:49 ` [PATCH V3 06/30] x86/sgx: Export sgx_encl_ewb_cpumask() Reinette Chatre
2022-04-05  6:56   ` Jarkko Sakkinen
2022-04-04 16:49 ` [PATCH V3 07/30] x86/sgx: Rename sgx_encl_ewb_cpumask() as sgx_encl_cpumask() Reinette Chatre
2022-04-05  6:57   ` Jarkko Sakkinen
2022-04-04 16:49 ` [PATCH V3 08/30] x86/sgx: Move PTE zap code to new sgx_zap_enclave_ptes() Reinette Chatre
2022-04-05  6:59   ` Jarkko Sakkinen
2022-04-04 16:49 ` [PATCH V3 09/30] x86/sgx: Make sgx_ipi_cb() available internally Reinette Chatre
2022-04-05  6:59   ` Jarkko Sakkinen
2022-04-04 16:49 ` [PATCH V3 10/30] x86/sgx: Create utility to validate user provided offset and length Reinette Chatre
2022-04-05  7:00   ` Jarkko Sakkinen
2022-04-04 16:49 ` [PATCH V3 11/30] x86/sgx: Keep record of SGX page type Reinette Chatre
2022-04-05  7:00   ` Jarkko Sakkinen
2022-04-04 16:49 ` [PATCH V3 12/30] x86/sgx: Export sgx_encl_{grow,shrink}() Reinette Chatre
2022-04-05  7:04   ` Jarkko Sakkinen
2022-04-04 16:49 ` [PATCH V3 13/30] x86/sgx: Export sgx_encl_page_alloc() Reinette Chatre
2022-04-04 16:49 ` [PATCH V3 14/30] x86/sgx: Support restricting of enclave page permissions Reinette Chatre
2022-04-05  5:03   ` Jarkko Sakkinen
2022-04-05  5:07     ` Jarkko Sakkinen
2022-04-05 13:40       ` Jarkko Sakkinen
2022-04-05 14:19         ` Jarkko Sakkinen
2022-04-05 14:27           ` Jarkko Sakkinen
2022-04-05 14:52             ` Jarkko Sakkinen
2022-04-05 16:49               ` Reinette Chatre
2022-04-05 18:39                 ` Jarkko Sakkinen
2022-04-05 18:59                   ` Reinette Chatre
2022-04-06  7:30                     ` Jarkko Sakkinen
2022-04-06 17:51                       ` Reinette Chatre
2022-04-05 16:40             ` Reinette Chatre
2022-04-04 16:49 ` [PATCH V3 15/30] x86/sgx: Support adding of pages to an initialized enclave Reinette Chatre
2022-04-05  5:05   ` Jarkko Sakkinen
2022-04-05 10:03     ` Jarkko Sakkinen
2022-04-06  7:37       ` Jarkko Sakkinen
2022-04-06 22:42         ` Reinette Chatre
2022-04-04 16:49 ` [PATCH V3 16/30] x86/sgx: Tighten accessible memory range after enclave initialization Reinette Chatre
2022-04-05  7:05   ` Jarkko Sakkinen
2022-04-04 16:49 ` [PATCH V3 17/30] x86/sgx: Support modifying SGX page type Reinette Chatre
2022-04-05  7:06   ` Jarkko Sakkinen
2022-04-05 15:34     ` Jarkko Sakkinen
2022-04-05 17:05       ` Reinette Chatre
2022-04-05 18:41         ` Jarkko Sakkinen
2022-04-05 18:59           ` Reinette Chatre
2022-04-06  7:32             ` Jarkko Sakkinen
2022-04-06 17:50               ` Reinette Chatre
2022-04-04 16:49 ` [PATCH V3 18/30] x86/sgx: Support complete page removal Reinette Chatre
2022-04-05  7:08   ` Jarkko Sakkinen
2022-04-04 16:49 ` [PATCH V3 19/30] x86/sgx: Free up EPC pages directly to support large page ranges Reinette Chatre
2022-04-05  7:11   ` Jarkko Sakkinen
2022-04-05 17:13     ` Reinette Chatre
2022-04-05 17:25       ` Dave Hansen
2022-04-06  6:35         ` Jarkko Sakkinen
2022-04-06 17:50           ` Reinette Chatre
2022-04-05 18:42       ` Jarkko Sakkinen
2022-04-05 19:56         ` Reinette Chatre
2022-04-04 16:49 ` [PATCH V3 20/30] Documentation/x86: Introduce enclave runtime management section Reinette Chatre
2022-04-04 16:49 ` [PATCH V3 21/30] selftests/sgx: Add test for EPCM permission changes Reinette Chatre
2022-04-05  7:02   ` Jarkko Sakkinen
2022-04-05  7:03     ` Jarkko Sakkinen
2022-04-05 17:28     ` Reinette Chatre
2022-04-05 18:43       ` Jarkko Sakkinen
2022-04-04 16:49 ` [PATCH V3 22/30] selftests/sgx: Add test for TCS page " Reinette Chatre
2022-04-04 16:49 ` [PATCH V3 23/30] selftests/sgx: Test two different SGX2 EAUG flows Reinette Chatre
2022-04-04 16:49 ` [PATCH V3 24/30] selftests/sgx: Introduce dynamic entry point Reinette Chatre
2022-04-04 16:49 ` [PATCH V3 25/30] selftests/sgx: Introduce TCS initialization enclave operation Reinette Chatre
2022-04-04 16:49 ` [PATCH V3 26/30] selftests/sgx: Test complete changing of page type flow Reinette Chatre
2022-04-04 16:49 ` Reinette Chatre [this message]
2022-04-04 16:49 ` [PATCH V3 28/30] selftests/sgx: Test invalid access to removed enclave page Reinette Chatre
2022-04-04 16:49 ` [PATCH V3 29/30] selftests/sgx: Test reclaiming of untouched page Reinette Chatre
2022-04-04 16:49 ` [PATCH V3 30/30] selftests/sgx: Page removal stress test Reinette Chatre

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2532238125e82343e26a6610241c7926e620527f.1648847675.git.reinette.chatre@intel.com \
    --to=reinette.chatre@intel.com \
    --cc=bp@alien8.de \
    --cc=cathy.zhang@intel.com \
    --cc=cedric.xing@intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=haitao.huang@intel.com \
    --cc=hpa@zytor.com \
    --cc=jarkko@kernel.org \
    --cc=kai.huang@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sgx@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mark.shanahan@intel.com \
    --cc=mingo@redhat.com \
    --cc=seanjc@google.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.