From: Jarkko Sakkinen <jarkko@kernel.org>
To: Reinette Chatre <reinette.chatre@intel.com>,
dave.hansen@linux.intel.com, linux-sgx@vger.kernel.org
Cc: haitao.huang@intel.com
Subject: Re: [RFC PATCH 2/4] x86/sgx: Set dirty bit after modifying page contents
Date: Sat, 07 May 2022 01:27:00 +0300 [thread overview]
Message-ID: <de2e1a1d69331bb521222eb93b5ffa6d00af354c.camel@kernel.org> (raw)
In-Reply-To: <b0596095e09ee4a14a3802c5ce91372a0dc21220.1651171455.git.reinette.chatre@intel.com>
On Thu, 2022-04-28 at 13:11 -0700, Reinette Chatre wrote:
> Recent commit 08999b2489b4 ("x86/sgx: Free backing memory
> after faulting the enclave page") expanded __sgx_encl_eldu()
> to clear an enclave page's PCMD (Paging Crypto MetaData)
> from the PCMD page in the backing store after the enclave
> page is restored to the enclave.
>
> Since the PCMD page in the backing store is modified the page
> should be set as dirty when releasing the reference to
> ensure the modified data is retained.
>
> Fixes: 08999b2489b4 ("x86/sgx: Free backing memory after faulting the enclave page")
> Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
> ---
> arch/x86/kernel/cpu/sgx/encl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c
> index e5d2661800ac..e03f124ce772 100644
> --- a/arch/x86/kernel/cpu/sgx/encl.c
> +++ b/arch/x86/kernel/cpu/sgx/encl.c
> @@ -98,7 +98,7 @@ static int __sgx_encl_eldu(struct sgx_encl_page *encl_page,
> kunmap_atomic(pcmd_page);
> kunmap_atomic((void *)(unsigned long)pginfo.contents);
>
> - sgx_encl_put_backing(&b, false);
> + sgx_encl_put_backing(&b, true);
>
> sgx_encl_truncate_backing_page(encl, page_index);
>
So, the implementation is:
void sgx_encl_put_backing(struct sgx_backing *backing, bool do_write)
{
if (do_write) {
set_page_dirty(backing->pcmd);
set_page_dirty(backing->contents);
}
put_page(backing->pcmd);
put_page(backing->contents);
}
And we only want to set dirty for PCMD part, right?
Thus, perhaps we should fix it instead by:
set_page_dirty(backing->pcmd);
put_page(backing->pcmd):
put_page(backing->contents);
?
I would not mind getting rid of that function anyway. It's kind
of bad wrapping IMHO.
BR, Jarkko
next prev parent reply other threads:[~2022-05-06 22:25 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-28 20:11 [RFC PATCH 0/4] SGX shmem backing store issue Reinette Chatre
2022-04-28 20:11 ` [RFC PATCH 1/4] x86/sgx: Do not free backing memory on ENCLS[ELDU] failure Reinette Chatre
2022-04-28 21:30 ` Dave Hansen
2022-04-28 22:20 ` Reinette Chatre
2022-04-28 22:53 ` Dave Hansen
2022-04-28 23:49 ` Reinette Chatre
2022-05-03 2:01 ` Kai Huang
2022-05-07 17:25 ` Jarkko Sakkinen
2022-05-09 17:17 ` Reinette Chatre
2022-05-10 0:36 ` Kai Huang
2022-05-11 10:26 ` Jarkko Sakkinen
2022-05-11 18:29 ` Haitao Huang
2022-05-11 22:00 ` Kai Huang
2022-05-12 21:14 ` Jarkko Sakkinen
2022-05-06 22:09 ` Jarkko Sakkinen
2022-04-28 20:11 ` [RFC PATCH 2/4] x86/sgx: Set dirty bit after modifying page contents Reinette Chatre
2022-04-28 21:40 ` Dave Hansen
2022-04-28 22:41 ` Reinette Chatre
2022-05-06 22:27 ` Jarkko Sakkinen [this message]
2022-05-06 22:40 ` Reinette Chatre
2022-05-07 18:01 ` Jarkko Sakkinen
2022-04-28 20:11 ` [RFC PATCH 3/4] x86/sgx: Obtain backing storage page with enclave mutex held Reinette Chatre
2022-04-28 21:58 ` Dave Hansen
2022-04-28 22:44 ` Reinette Chatre
2022-05-06 22:43 ` Jarkko Sakkinen
2022-04-28 20:11 ` [RFC PATCH 4/4] x86/sgx: Do not allocate backing pages when loading from backing store Reinette Chatre
2022-04-28 21:12 ` [RFC PATCH 0/4] SGX shmem backing store issue Dave Hansen
2022-04-29 18:50 ` Reinette Chatre
2022-04-29 19:45 ` Dave Hansen
2022-04-30 3:22 ` Reinette Chatre
2022-04-30 15:52 ` Reinette Chatre
2022-05-02 14:36 ` Dave Hansen
2022-05-02 17:11 ` Reinette Chatre
2022-05-02 21:33 ` Dave Hansen
2022-05-04 22:13 ` Reinette Chatre
2022-05-04 22:58 ` Dave Hansen
2022-05-04 23:36 ` Reinette Chatre
2022-05-04 23:50 ` Dave Hansen
2022-05-05 0:08 ` Reinette Chatre
2022-05-04 23:05 ` Dave Hansen
2022-05-07 17:46 ` Jarkko Sakkinen
2022-05-07 17:48 ` Jarkko Sakkinen
2022-05-09 17:09 ` Reinette Chatre
2022-05-10 22:28 ` Jarkko Sakkinen
2022-05-11 17:23 ` Reinette Chatre
2022-05-12 14:10 ` Jarkko Sakkinen
2022-04-28 21:29 ` Dave Hansen
2022-04-28 22:20 ` Reinette Chatre
2022-05-04 6:40 ` Jarkko Sakkinen
2022-05-05 6:09 ` 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=de2e1a1d69331bb521222eb93b5ffa6d00af354c.camel@kernel.org \
--to=jarkko@kernel.org \
--cc=dave.hansen@linux.intel.com \
--cc=haitao.huang@intel.com \
--cc=linux-sgx@vger.kernel.org \
--cc=reinette.chatre@intel.com \
/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).