lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <D3QWGTWLAX0R.11U42QH2Y1TLB@kernel.org>
Date: Tue, 27 Aug 2024 21:19:38 +0300
From: "Jarkko Sakkinen" <jarkko@...nel.org>
To: "Dmitrii Kuvaiskii" <dmitrii.kuvaiskii@...el.com>,
 <dave.hansen@...ux.intel.com>, <kai.huang@...el.com>,
 <haitao.huang@...ux.intel.com>, <reinette.chatre@...el.com>,
 <linux-sgx@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Cc: <mona.vij@...el.com>, <kailun.qin@...el.com>, <stable@...r.kernel.org>
Subject: Re: [PATCH v5 1/3] x86/sgx: Split SGX_ENCL_PAGE_BEING_RECLAIMED
 into two flags

On Wed Aug 21, 2024 at 1:02 PM EEST, Dmitrii Kuvaiskii wrote:
> The page reclaimer thread sets SGX_ENC_PAGE_BEING_RECLAIMED flag when
> the enclave page is being reclaimed (moved to the backing store). This
> flag however has two logical meanings:
>
> 1. Don't attempt to load the enclave page (the page is busy), see
>    __sgx_encl_load_page().
> 2. Don't attempt to remove the PCMD page corresponding to this enclave
>    page (the PCMD page is busy), see reclaimer_writing_to_pcmd().
>
> To reflect these two meanings, split SGX_ENCL_PAGE_BEING_RECLAIMED into
> two flags: SGX_ENCL_PAGE_BUSY and SGX_ENCL_PAGE_PCMD_BUSY. Currently,
> both flags are set only when the enclave page is being reclaimed (by the
> page reclaimer thread). A future commit will introduce new cases when
> the enclave page is being operated on; these new cases will set only the
> SGX_ENCL_PAGE_BUSY flag.
>
> Cc: stable@...r.kernel.org
> Signed-off-by: Dmitrii Kuvaiskii <dmitrii.kuvaiskii@...el.com>
> Reviewed-by: Haitao Huang <haitao.huang@...ux.intel.com>
> Acked-by: Kai Huang <kai.huang@...el.com>
> ---
>  arch/x86/kernel/cpu/sgx/encl.c | 16 +++++++---------
>  arch/x86/kernel/cpu/sgx/encl.h | 10 ++++++++--
>  arch/x86/kernel/cpu/sgx/main.c |  4 ++--
>  3 files changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c
> index 279148e72459..c0a3c00284c8 100644
> --- a/arch/x86/kernel/cpu/sgx/encl.c
> +++ b/arch/x86/kernel/cpu/sgx/encl.c
> @@ -46,10 +46,10 @@ static int sgx_encl_lookup_backing(struct sgx_encl *encl, unsigned long page_ind
>   * a check if an enclave page sharing the PCMD page is in the process of being
>   * reclaimed.
>   *
> - * The reclaimer sets the SGX_ENCL_PAGE_BEING_RECLAIMED flag when it
> - * intends to reclaim that enclave page - it means that the PCMD page
> - * associated with that enclave page is about to get some data and thus
> - * even if the PCMD page is empty, it should not be truncated.
> + * The reclaimer sets the SGX_ENCL_PAGE_PCMD_BUSY flag when it intends to
> + * reclaim that enclave page - it means that the PCMD page associated with that
> + * enclave page is about to get some data and thus even if the PCMD page is
> + * empty, it should not be truncated.
>   *
>   * Context: Enclave mutex (&sgx_encl->lock) must be held.
>   * Return: 1 if the reclaimer is about to write to the PCMD page
> @@ -77,8 +77,7 @@ static int reclaimer_writing_to_pcmd(struct sgx_encl *encl,
>  		 * Stop when reaching the SECS page - it does not
>  		 * have a page_array entry and its reclaim is
>  		 * started and completed with enclave mutex held so
> -		 * it does not use the SGX_ENCL_PAGE_BEING_RECLAIMED
> -		 * flag.
> +		 * it does not use the SGX_ENCL_PAGE_PCMD_BUSY flag.
>  		 */
>  		if (addr == encl->base + encl->size)
>  			break;
> @@ -91,8 +90,7 @@ static int reclaimer_writing_to_pcmd(struct sgx_encl *encl,
>  		 * VA page slot ID uses same bit as the flag so it is important
>  		 * to ensure that the page is not already in backing store.
>  		 */
> -		if (entry->epc_page &&
> -		    (entry->desc & SGX_ENCL_PAGE_BEING_RECLAIMED)) {
> +		if (entry->epc_page && (entry->desc & SGX_ENCL_PAGE_PCMD_BUSY)) {
>  			reclaimed = 1;
>  			break;
>  		}
> @@ -257,7 +255,7 @@ static struct sgx_encl_page *__sgx_encl_load_page(struct sgx_encl *encl,
>  
>  	/* Entry successfully located. */
>  	if (entry->epc_page) {
> -		if (entry->desc & SGX_ENCL_PAGE_BEING_RECLAIMED)
> +		if (entry->desc & SGX_ENCL_PAGE_BUSY)
>  			return ERR_PTR(-EBUSY);
>  
>  		return entry;
> diff --git a/arch/x86/kernel/cpu/sgx/encl.h b/arch/x86/kernel/cpu/sgx/encl.h
> index f94ff14c9486..b566b8ad5f33 100644
> --- a/arch/x86/kernel/cpu/sgx/encl.h
> +++ b/arch/x86/kernel/cpu/sgx/encl.h
> @@ -22,8 +22,14 @@
>  /* 'desc' bits holding the offset in the VA (version array) page. */
>  #define SGX_ENCL_PAGE_VA_OFFSET_MASK	GENMASK_ULL(11, 3)
>  
> -/* 'desc' bit marking that the page is being reclaimed. */
> -#define SGX_ENCL_PAGE_BEING_RECLAIMED	BIT(3)
> +/* 'desc' bit indicating that the page is busy (being reclaimed). */
> +#define SGX_ENCL_PAGE_BUSY	BIT(2)
> +
> +/*
> + * 'desc' bit indicating that PCMD page associated with the enclave page is
> + * busy (because the enclave page is being reclaimed).
> + */
> +#define SGX_ENCL_PAGE_PCMD_BUSY	BIT(3)
>  
>  struct sgx_encl_page {
>  	unsigned long desc;
> diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
> index 166692f2d501..e94b09c43673 100644
> --- a/arch/x86/kernel/cpu/sgx/main.c
> +++ b/arch/x86/kernel/cpu/sgx/main.c
> @@ -204,7 +204,7 @@ static void sgx_encl_ewb(struct sgx_epc_page *epc_page,
>  	void *va_slot;
>  	int ret;
>  
> -	encl_page->desc &= ~SGX_ENCL_PAGE_BEING_RECLAIMED;
> +	encl_page->desc &= ~(SGX_ENCL_PAGE_BUSY | SGX_ENCL_PAGE_PCMD_BUSY);
>  
>  	va_page = list_first_entry(&encl->va_pages, struct sgx_va_page,
>  				   list);
> @@ -340,7 +340,7 @@ static void sgx_reclaim_pages(void)
>  			goto skip;
>  		}
>  
> -		encl_page->desc |= SGX_ENCL_PAGE_BEING_RECLAIMED;
> +		encl_page->desc |= SGX_ENCL_PAGE_BUSY | SGX_ENCL_PAGE_PCMD_BUSY;
>  		mutex_unlock(&encl_page->encl->lock);
>  		continue;
>  

Reviewed-by: Jarkko Sakkinen <jarkko@...nel.org>

BR, Jarkko

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ