From: Jarkko Sakkinen <jarkko@kernel.org>
To: Reinette Chatre <reinette.chatre@intel.com>,
linux-sgx@vger.kernel.org, shuah@kernel.org
Cc: seanjc@google.com, bp@alien8.de, dave.hansen@linux.intel.com,
linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 13/14] selftests/sgx: Enable multiple thread support
Date: Thu, 16 Sep 2021 18:23:26 +0300 [thread overview]
Message-ID: <fe7dc7cc1b923ac07cfe8a05b5e1303c49a80a9f.camel@kernel.org> (raw)
In-Reply-To: <7b413966289d22f043762b3d20e30cb6ded936e3.1631731214.git.reinette.chatre@intel.com>
On Wed, 2021-09-15 at 13:31 -0700, Reinette Chatre wrote:
> 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.
>
> Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
> ---
> .../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
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Thanks for the throughout explanation!
/Jarkko
next prev parent reply other threads:[~2021-09-16 15:23 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-15 20:30 [PATCH 00/14] selftests/sgx: Oversubscription, page permission, thread entry Reinette Chatre
2021-09-15 20:30 ` [PATCH 01/14] selftests/x86/sgx: Fix a benign linker warning Reinette Chatre
2021-09-16 14:38 ` Jarkko Sakkinen
2021-09-16 14:39 ` Jarkko Sakkinen
2021-09-16 17:31 ` Reinette Chatre
2021-09-15 20:30 ` [PATCH 02/14] x86/sgx: Add /sys/kernel/debug/x86/sgx_total_mem Reinette Chatre
2021-09-16 14:09 ` Jarkko Sakkinen
2021-09-16 15:35 ` Reinette Chatre
2021-09-15 20:30 ` [PATCH 03/14] selftests/sgx: Assign source for each segment Reinette Chatre
2021-09-15 20:30 ` [PATCH 04/14] selftests/sgx: Make data measurement for an enclave segment optional Reinette Chatre
2021-09-15 20:30 ` [PATCH 05/14] selftests/sgx: Create a heap for the test enclave Reinette Chatre
2021-09-15 20:30 ` [PATCH 06/14] selftests/sgx: Dump segments and /proc/self/maps only on failure Reinette Chatre
2021-09-15 20:30 ` [PATCH 07/14] selftests/sgx: Encpsulate the test enclave creation Reinette Chatre
2021-09-15 20:30 ` [PATCH 08/14] selftests/sgx: Move setup_test_encl() to each TEST_F() Reinette Chatre
2021-09-15 20:30 ` [PATCH 09/14] selftests/sgx: Add a new kselftest: unclobbered_vdso_oversubscribed Reinette Chatre
2021-09-15 20:31 ` [PATCH 10/14] selftests/sgx: Provide per-op parameter structs for the test enclave Reinette Chatre
2021-09-15 20:31 ` [PATCH 11/14] selftests/sgx: Rename test properties in preparation for more enclave tests Reinette Chatre
2021-09-16 15:20 ` Jarkko Sakkinen
2021-09-15 20:31 ` [PATCH 12/14] selftests/sgx: Add page permission and exception test Reinette Chatre
2021-09-16 15:21 ` Jarkko Sakkinen
2021-09-16 15:37 ` Reinette Chatre
2021-09-16 15:30 ` Dave Hansen
2021-09-16 15:50 ` Reinette Chatre
2021-09-15 20:31 ` [PATCH 13/14] selftests/sgx: Enable multiple thread support Reinette Chatre
2021-09-16 15:23 ` Jarkko Sakkinen [this message]
2021-09-15 20:31 ` [PATCH 14/14] selftests/sgx: Add test for multiple TCS entry Reinette Chatre
2021-09-16 15:24 ` Jarkko Sakkinen
2021-09-16 14:13 ` [PATCH 00/14] selftests/sgx: Oversubscription, page permission, thread entry Jarkko Sakkinen
2021-09-16 15:37 ` Dave Hansen
2021-09-16 16:14 ` Reinette Chatre
2021-09-16 16:15 ` Jarkko Sakkinen
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=fe7dc7cc1b923ac07cfe8a05b5e1303c49a80a9f.camel@kernel.org \
--to=jarkko@kernel.org \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-sgx@vger.kernel.org \
--cc=reinette.chatre@intel.com \
--cc=seanjc@google.com \
--cc=shuah@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).