From: Jarkko Sakkinen <jarkko@kernel.org>
To: Reinette Chatre <reinette.chatre@intel.com>
Cc: dave.hansen@linux.intel.com, tglx@linutronix.de, bp@alien8.de,
luto@kernel.org, mingo@redhat.com, linux-sgx@vger.kernel.org,
x86@kernel.org, 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: Re: [PATCH 04/25] x86/sgx: Add pfn_mkwrite() handler for present PTEs
Date: Tue, 28 Dec 2021 16:51:06 +0200 [thread overview]
Message-ID: <YcskWr6yH/8AjDPv@iki.fi> (raw)
In-Reply-To: <16bce99a-206e-1e9c-a74d-6116df705df4@intel.com>
On Mon, Dec 13, 2021 at 02:09:30PM -0800, Reinette Chatre wrote:
> Hi Jarkko,
>
> On 12/10/2021 11:37 PM, Jarkko Sakkinen wrote:
> > On Mon, 2021-12-06 at 13:18 -0800, Reinette Chatre wrote:
> > > Hi Jarkko,
> > >
> > > On 12/4/2021 2:43 PM, Jarkko Sakkinen wrote:
> > > > On Wed, Dec 01, 2021 at 11:23:02AM -0800, Reinette Chatre wrote:
> > > > > By default a write page fault on a present PTE inherits the permissions
> > > > > of the VMA. Enclave page permissions maintained in the hardware's
> > > > > Enclave Page Cache Map (EPCM) may change after a VMA accessing the page
> > > > > is created. A VMA's permissions may thus exceed the enclave page
> > > > > permissions even though the VMA was originally created not to exceed
> > > > > the enclave page permissions. Following the default behavior during
> > > > > a page fault on a present PTE while the VMA permissions exceed the
> > > > > enclave page permissions would result in the PTE for an enclave page
> > > > > to be writable even though the page is not writable according to the
> > > > > enclave's permissions.
> > > > >
> > > > > Consider the following scenario:
> > > > > * An enclave page exists with RW EPCM permissions.
> > > > > * A RW VMA maps the range spanning the enclave page.
> > > > > * The enclave page's EPCM permissions are changed to read-only.
> > > >
> > > > How could this happen in the existing mainline code?
> > >
> > > This is a preparatory patch for SGX2 support. Restricting the
> > > permissions of an enclave page would require OS support that is added in
> > > a later patch.
> > >
> > > >
> > > > > * There is no page table entry for the enclave page.
> > > > >
> > > > > Q.
> > > > > What will user space observe when an attempt is made to write to the
> > > > > enclave page from within the enclave?
> > > > >
> > > > > A.
> > > > > Initially the page table entry is not present so the following is
> > > > > observed:
> > > > > 1) Instruction writing to enclave page is run from within the enclave.
> > > > > 2) A page fault with second and third bits set (0x6) is encountered
> > > > > and handled by the SGX handler sgx_vma_fault() that installs a
> > > > > read-only page table entry following previous patch that installs
> > > > > page table entry with permissions that VMA and enclave agree on
> > > > > (read-only in this case).
> > > > > 3) Instruction writing to enclave page is re-attempted.
> > > > > 4) A page fault with first three bits set (0x7) is encountered and
> > > > > transparently (from SGX and user space perspective) handled by the
> > > > > OS with the page table entry made writable because the VMA is
> > > > > writable.
> > > > > 5) Instruction writing to enclave page is re-attempted.
> > > > > 6) Since the EPCM permissions prevents writing to the page a new page
> > > > > fault is encountered, this time with the SGX flag set in the error
> > > > > code (0x8007). No action is taken by OS for this page fault and
> > > > > execution returns to user space.
> > > > > 7) Typically such a fault will be passed on to an application with a
> > > > > signal but if the enclave is entered with the vDSO function provided
> > > > > by the kernel then user space does not receive a signal but instead
> > > > > the vDSO function returns successfully with exception information
> > > > > (vector=14, error code=0x8007, and address) within the exception
> > > > > fields within the vDSO function's struct sgx_enclave_run.
> > > > >
> > > > > As can be observed it is not possible for user space to write to an
> > > > > enclave page if that page's enclave page permissions do not allow so,
> > > > > no matter what the VMA or PTE allows.
> > > > >
> > > > > Even so, the OS should not allow writing to a page if that page is not
> > > > > writable. Thus the page table entry should accurately reflect the
> > > > > enclave page permissions.
> > > > >
> > > > > Do not blindly accept VMA permissions on a page fault due to a write
> > > > > attempt to a present PTE. Install a pfn_mkwrite() handler that ensures
> > > > > that the VMA permissions agree with the enclave permissions in this
> > > > > regard.
> > > > >
> > > > > Considering the same scenario as above after this change results in
> > > > > the following behavior change:
> > > > >
> > > > > Q.
> > > > > What will user space observe when an attempt is made to write to the
> > > > > enclave page from within the enclave?
> > > > >
> > > > > A.
> > > > > Initially the page table entry is not present so the following is
> > > > > observed:
> > > > > 1) Instruction writing to enclave page is run from within the enclave.
> > > > > 2) A page fault with second and third bits set (0x6) is encountered
> > > > > and handled by the SGX handler sgx_vma_fault() that installs a
> > > > > read-only page table entry following previous patch that installs
> > > > > page table entry with permissions that VMA and enclave agree on
> > > > > (read-only in this case).
> > > > > 3) Instruction writing to enclave page is re-attempted.
> > > > > 4) A page fault with first three bits set (0x7) is encountered and
> > > > > passed to the pfn_mkwrite() handler for consideration. The handler
> > > > > determines that the page should not be writable and returns SIGBUS.
> > > > > 5) Typically such a fault will be passed on to an application with a
> > > > > signal but if the enclave is entered with the vDSO function provided
> > > > > by the kernel then user space does not receive a signal but instead
> > > > > the vDSO function returns successfully with exception information
> > > > > (vector=14, error code=0x7, and address) within the exception fields
> > > > > within the vDSO function's struct sgx_enclave_run.
> > > > >
> > > > > The accurate exception information supports the SGX runtime, which is
> > > > > virtually always implemented inside a shared library, by providing
> > > > > accurate information in support of its management of the SGX enclave.
> > > >
> > > > This QA-format is not a great idea, as it kind of tells what are the legit
> > > > questions to ask.
> > >
> > > I will remove the QA-format and just describe the two (before/after)
> > > scenarios.
> > >
> > > > You should describe what the patch does and what are the
> > > > legit reasons for doing that. Unfortunately, in the current form it is very
> > > > hard to get grip of this patch.
> > >
> > > That was the goal of the summary (the first paragraph) at the start of
> > > the changelog. Could you please elaborate how you would like me to
> > > improve it?
> >
> > If I do a search "mktme" through the commit message, it gives
> > me zero results.
>
> Could you please elaborate why you expect "mktme" to show up in the commit
> message?
I'm sorry, my mistake doubled: I ment to write mkwrite, and yes its use was well
described in the commit message.
/Jarkko
next prev parent reply other threads:[~2021-12-28 14:51 UTC|newest]
Thread overview: 155+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-01 19:22 [PATCH 00/25] x86/sgx and selftests/sgx: Support SGX2 Reinette Chatre
2021-12-01 19:22 ` [PATCH 01/25] x86/sgx: Add shortlog descriptions to ENCLS wrappers Reinette Chatre
2021-12-04 18:30 ` Jarkko Sakkinen
2021-12-06 21:13 ` Reinette Chatre
2021-12-11 5:28 ` Jarkko Sakkinen
2021-12-13 22:06 ` Reinette Chatre
2021-12-01 19:23 ` [PATCH 02/25] x86/sgx: Add wrappers for SGX2 functions Reinette Chatre
2021-12-04 22:04 ` Jarkko Sakkinen
2021-12-06 21:15 ` Reinette Chatre
2021-12-01 19:23 ` [PATCH 03/25] x86/sgx: Support VMA permissions exceeding enclave permissions Reinette Chatre
2021-12-04 22:25 ` Jarkko Sakkinen
2021-12-04 22:27 ` Jarkko Sakkinen
2021-12-06 21:16 ` Reinette Chatre
2021-12-11 5:39 ` Jarkko Sakkinen
2021-12-13 22:08 ` Reinette Chatre
2021-12-01 19:23 ` [PATCH 04/25] x86/sgx: Add pfn_mkwrite() handler for present PTEs Reinette Chatre
2021-12-04 22:43 ` Jarkko Sakkinen
2021-12-06 21:18 ` Reinette Chatre
2021-12-11 7:37 ` Jarkko Sakkinen
2021-12-13 22:09 ` Reinette Chatre
2021-12-28 14:51 ` Jarkko Sakkinen [this message]
2021-12-01 19:23 ` [PATCH 05/25] x86/sgx: Introduce runtime protection bits Reinette Chatre
2021-12-03 19:28 ` Andy Lutomirski
2021-12-03 22:12 ` Reinette Chatre
2021-12-04 0:38 ` Andy Lutomirski
2021-12-04 1:14 ` Reinette Chatre
2021-12-04 17:56 ` Andy Lutomirski
2021-12-04 23:55 ` Reinette Chatre
2021-12-13 22:34 ` Reinette Chatre
2021-12-04 23:57 ` Jarkko Sakkinen
2021-12-06 21:20 ` Reinette Chatre
2021-12-11 7:42 ` Jarkko Sakkinen
2021-12-13 22:10 ` Reinette Chatre
2021-12-28 14:52 ` Jarkko Sakkinen
2022-01-06 17:46 ` Reinette Chatre
2022-01-07 12:16 ` Jarkko Sakkinen
2022-01-07 16:14 ` Haitao Huang
2022-01-08 15:45 ` Jarkko Sakkinen
2022-01-08 15:51 ` Jarkko Sakkinen
2022-01-08 16:22 ` Jarkko Sakkinen
2022-01-10 22:05 ` Haitao Huang
2022-01-11 1:53 ` Jarkko Sakkinen
2022-01-11 1:55 ` Jarkko Sakkinen
2022-01-11 2:03 ` Jarkko Sakkinen
2022-01-11 2:15 ` Jarkko Sakkinen
2022-01-11 3:48 ` Haitao Huang
2022-01-12 23:48 ` Jarkko Sakkinen
2022-01-13 2:41 ` Haitao Huang
2022-01-14 21:36 ` Jarkko Sakkinen
2022-01-11 17:13 ` Reinette Chatre
2022-01-12 23:50 ` Jarkko Sakkinen
2022-01-12 23:56 ` Jarkko Sakkinen
2022-01-13 20:09 ` Nathaniel McCallum
2022-01-13 21:42 ` Reinette Chatre
2022-01-14 21:53 ` Jarkko Sakkinen
2022-01-14 21:57 ` Jarkko Sakkinen
2022-01-14 22:00 ` Jarkko Sakkinen
2022-01-14 22:17 ` Jarkko Sakkinen
2022-01-14 22:23 ` Jarkko Sakkinen
2022-01-14 22:34 ` Jarkko Sakkinen
2022-01-14 23:05 ` Reinette Chatre
2022-01-14 23:15 ` Jarkko Sakkinen
2022-01-15 0:01 ` Reinette Chatre
2022-01-15 0:27 ` Jarkko Sakkinen
2022-01-15 0:41 ` Reinette Chatre
2022-01-15 1:18 ` Jarkko Sakkinen
2022-01-15 11:56 ` Jarkko Sakkinen
2022-01-15 11:59 ` Jarkko Sakkinen
2022-01-17 13:13 ` Nathaniel McCallum
2022-01-18 1:59 ` Jarkko Sakkinen
2022-01-18 2:22 ` Jarkko Sakkinen
2022-01-18 3:31 ` Jarkko Sakkinen
2022-01-18 20:59 ` Reinette Chatre
2022-01-20 12:53 ` Jarkko Sakkinen
2022-01-20 16:52 ` Reinette Chatre
2022-01-26 14:41 ` Jarkko Sakkinen
2022-01-15 16:49 ` Jarkko Sakkinen
2022-01-18 21:18 ` Reinette Chatre
2022-01-17 13:27 ` Nathaniel McCallum
2022-01-18 21:11 ` Reinette Chatre
2021-12-04 22:50 ` Jarkko Sakkinen
2021-12-06 21:28 ` Reinette Chatre
2021-12-01 19:23 ` [PATCH 06/25] x86/sgx: Use more generic name for enclave cpumask function Reinette Chatre
2021-12-04 22:56 ` Jarkko Sakkinen
2021-12-06 21:29 ` Reinette Chatre
2021-12-01 19:23 ` [PATCH 07/25] x86/sgx: Move PTE zap code to separate function Reinette Chatre
2021-12-04 22:59 ` Jarkko Sakkinen
2021-12-06 21:30 ` Reinette Chatre
2021-12-11 7:52 ` Jarkko Sakkinen
2021-12-13 22:11 ` Reinette Chatre
2021-12-28 14:55 ` Jarkko Sakkinen
2022-01-06 17:46 ` Reinette Chatre
2022-01-07 12:26 ` Jarkko Sakkinen
2021-12-01 19:23 ` [PATCH 08/25] x86/sgx: Make SGX IPI callback available internally Reinette Chatre
2021-12-04 23:00 ` Jarkko Sakkinen
2021-12-06 21:36 ` Reinette Chatre
2021-12-11 7:53 ` Jarkko Sakkinen
2021-12-01 19:23 ` [PATCH 09/25] x86/sgx: Keep record of SGX page type Reinette Chatre
2021-12-04 23:03 ` Jarkko Sakkinen
2021-12-01 19:23 ` [PATCH 10/25] x86/sgx: Support enclave page permission changes Reinette Chatre
2021-12-02 23:48 ` Dave Hansen
2021-12-03 18:18 ` Reinette Chatre
2021-12-03 0:32 ` Dave Hansen
2021-12-03 18:18 ` Reinette Chatre
2021-12-03 18:14 ` Dave Hansen
2021-12-03 18:49 ` Reinette Chatre
2021-12-03 19:38 ` Andy Lutomirski
2021-12-03 22:34 ` Reinette Chatre
2021-12-04 0:42 ` Andy Lutomirski
2021-12-04 1:35 ` Reinette Chatre
2021-12-04 23:08 ` Jarkko Sakkinen
2021-12-06 20:19 ` Dave Hansen
2021-12-11 5:17 ` Jarkko Sakkinen
2021-12-06 21:42 ` Reinette Chatre
2021-12-11 7:57 ` Jarkko Sakkinen
2021-12-13 22:12 ` Reinette Chatre
2021-12-28 14:56 ` Jarkko Sakkinen
2021-12-01 19:23 ` [PATCH 11/25] selftests/sgx: Add test for EPCM " Reinette Chatre
2021-12-01 19:23 ` [PATCH 12/25] selftests/sgx: Add test for TCS page " Reinette Chatre
2021-12-01 19:23 ` [PATCH 13/25] x86/sgx: Support adding of pages to initialized enclave Reinette Chatre
2021-12-03 0:38 ` Dave Hansen
2021-12-03 18:47 ` Reinette Chatre
2021-12-04 23:13 ` Jarkko Sakkinen
2021-12-06 21:44 ` Reinette Chatre
2021-12-11 8:00 ` Jarkko Sakkinen
2021-12-13 22:12 ` Reinette Chatre
2021-12-28 14:57 ` Jarkko Sakkinen
2022-03-01 15:13 ` Jarkko Sakkinen
2022-03-01 17:08 ` Reinette Chatre
2021-12-01 19:23 ` [PATCH 14/25] x86/sgx: Tighten accessible memory range after enclave initialization Reinette Chatre
2021-12-04 23:14 ` Jarkko Sakkinen
2021-12-06 21:45 ` Reinette Chatre
2021-12-11 8:01 ` Jarkko Sakkinen
2021-12-01 19:23 ` [PATCH 15/25] selftests/sgx: Test two different SGX2 EAUG flows Reinette Chatre
2021-12-01 19:23 ` [PATCH 16/25] x86/sgx: Support modifying SGX page type Reinette Chatre
2021-12-04 23:45 ` Jarkko Sakkinen
2021-12-06 21:48 ` Reinette Chatre
2021-12-11 8:02 ` Jarkko Sakkinen
2021-12-13 17:43 ` Dave Hansen
2021-12-21 8:52 ` Jarkko Sakkinen
2021-12-01 19:23 ` [PATCH 17/25] x86/sgx: Support complete page removal Reinette Chatre
2021-12-04 23:45 ` Jarkko Sakkinen
2021-12-06 21:49 ` Reinette Chatre
2021-12-01 19:23 ` [PATCH 18/25] selftests/sgx: Introduce dynamic entry point Reinette Chatre
2021-12-01 19:23 ` [PATCH 19/25] selftests/sgx: Introduce TCS initialization enclave operation Reinette Chatre
2021-12-01 19:23 ` [PATCH 20/25] selftests/sgx: Test complete changing of page type flow Reinette Chatre
2021-12-01 19:23 ` [PATCH 21/25] selftests/sgx: Test faulty enclave behavior Reinette Chatre
2021-12-01 19:23 ` [PATCH 22/25] selftests/sgx: Test invalid access to removed enclave page Reinette Chatre
2021-12-01 19:23 ` [PATCH 23/25] selftests/sgx: Test reclaiming of untouched page Reinette Chatre
2021-12-01 19:23 ` [PATCH 24/25] x86/sgx: Free up EPC pages directly to support large page ranges Reinette Chatre
2021-12-04 23:47 ` Jarkko Sakkinen
2021-12-06 22:07 ` Reinette Chatre
2021-12-01 19:23 ` [PATCH 25/25] selftests/sgx: Page removal stress test Reinette Chatre
2021-12-02 18:30 ` [PATCH 00/25] x86/sgx and selftests/sgx: Support SGX2 Dave Hansen
2021-12-02 20:38 ` Nathaniel McCallum
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=YcskWr6yH/8AjDPv@iki.fi \
--to=jarkko@kernel.org \
--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=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=reinette.chatre@intel.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 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).