Message-ID: <op.1iilcwhywjvjmi@hhuan26-mobl1.mshome.net>
Date:   Fri, 04 Mar 2022 10:27:58 -0600
From:   "Haitao Huang" <haitao.huang@...ux.intel.com>
To:     linux-sgx@...r.kernel.org, "Jarkko Sakkinen" <jarkko@...nel.org>
Cc:     "Reinette Chatre" <reinette.chatre@...el.com>,
        "Nathaniel McCallum" <nathaniel@...fian.com>,
        "Dave Hansen" <dave.hansen@...ux.intel.com>,
        "Thomas Gleixner" <tglx@...utronix.de>,
        "Ingo Molnar" <mingo@...hat.com>, "Borislav Petkov" <bp@...en8.de>,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>,
        "open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)" 
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH RFC] x86: Add SGX_IOC_ENCLAVE_AUGMENT_PAGES

On Fri, 04 Mar 2022 06:28:52 -0600, Jarkko Sakkinen <jarkko@...nel.org>  
wrote:

> With SGX1 an enclave needs to be created with its maximum memory demands
> allocated. Pages cannot be added to an enclave after it is initialized.
> SGX2 introduces a new function, ENCLS[EAUG], that can be used to add  
> pages
> to an initialized enclave. With SGX2 the enclave still needs to set aside
> address space for its maximum memory demands during enclave creation, but
> all pages need not be added before enclave initialization. Pages can be
> added during enclave runtime.
>
> Add support for dynamically adding pages to an initialized enclave with
> SGX_IOC_ENCLAVE_AUGMENT_PAGES, which performs EAUG's to a given range of
> pages. Do not enforce any particular permissions from kernel, like is  
> done
> for the pages added during the pre-initialization phase, as enclave
> controls the final permissions and content for these pages by issuing
> either ENCLU[EACCEPT] (empty RW) or ENCLU[EACCEPTCOPY] (arbitrary data  
> and
> permissions).
>
> Explicit EAUG ioctl is a better choice than an implicit EAUG from a page
> fault handler because it allows to have O(1) number of kernel-enclave  
> round
> trips for EAUG-EACCEPT{COPY} process, instead of O(n), as it is in the  
> case
> when a page fault handler EAUG single page at a time.
>
> Cc: Reinette Chatre <reinette.chatre@...el.com>
> Cc: Nathaniel McCallum <nathaniel@...fian.com>
> Signed-off-by: Jarkko Sakkinen <jarkko@...nel.org>
> ---
> Is contained in sgx2-v2.1 branch of  
> git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-sgx.git
> ---
>  arch/x86/include/uapi/asm/sgx.h |  14 +++
>  arch/x86/kernel/cpu/sgx/ioctl.c | 159 ++++++++++++++++++++++++++++++++
>  2 files changed, 173 insertions(+)
>
> diff --git a/arch/x86/include/uapi/asm/sgx.h  
> b/arch/x86/include/uapi/asm/sgx.h
> index c4e0326d281d..2b3a606e78fe 100644
> --- a/arch/x86/include/uapi/asm/sgx.h
> +++ b/arch/x86/include/uapi/asm/sgx.h
> @@ -35,6 +35,8 @@ enum sgx_page_flags {
>  	_IOWR(SGX_MAGIC, 0x06, struct sgx_enclave_modt)
>  #define SGX_IOC_ENCLAVE_REMOVE_PAGES \
>  	_IOWR(SGX_MAGIC, 0x08, struct sgx_enclave_remove_pages)
> +#define SGX_IOC_ENCLAVE_AUGMENT_PAGES \
> +	_IOWR(SGX_MAGIC, 0x09, struct sgx_enclave_augment_pages)
> /**
>   * struct sgx_enclave_create - parameter structure for the
> @@ -138,6 +140,18 @@ struct sgx_enclave_remove_pages {
>  	__u64 count;
>  };
> +/**
> + * struct sgx_enclave_augment_pages - parameter structure for the  
> %SGX_IOC_ENCLAVE_ADD_PAGE ioctl
> + * @offset:	starting page offset
> + * @length:	length of the data (multiple of the page size)
> + * @count:	number of bytes added (multiple of the page size)
> + */
> +struct sgx_enclave_augment_pages {
> +	__u64 offset;
> +	__u64 length;
> +	__u64 count;
> +};
> +

As I stated in another thread, we need a mechanism to allow EAUG page  
lazily, e.g., on #PF. Can we add a field here to indicate that?

>  struct sgx_enclave_run;
> /**
> diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c  
> b/arch/x86/kernel/cpu/sgx/ioctl.c
> index 3ad4320ff6ae..1cb410d97ac7 100644
> --- a/arch/x86/kernel/cpu/sgx/ioctl.c
> +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
> @@ -1320,6 +1320,162 @@ static long sgx_ioc_enclave_remove_pages(struct  
> sgx_encl *encl,
>  	return ret;
>  }
> +/*
> + * Performa EAUG operation for an enclave.
> + */
> +static int sgx_encl_augment_page(struct sgx_encl *encl, unsigned long  
> offset)
> +{
> +	struct sgx_pageinfo pginfo = {0};
> +	struct sgx_encl_page *encl_page;
> +	struct sgx_epc_page *epc_page;
> +	struct sgx_va_page *va_page;
> +	u64 secinfo_flags;
> +	int ret;
> +
> +	/*
> +	 * Ignore internal permission checking for dynamically added pages.
> +	 * They matter only for data added during the pre-initialization phase.
> +	 * The enclave decides the permissions by the means of EACCEPT,
> +	 * EACCEPTCOPY and EMODPE.
> +	 */
> +	secinfo_flags = SGX_SECINFO_R | SGX_SECINFO_W | SGX_SECINFO_X;
> +	encl_page = sgx_encl_page_alloc(encl, offset, secinfo_flags);
> +	if (IS_ERR(encl_page))
> +		return PTR_ERR(encl_page);
> +
> +	epc_page = sgx_alloc_epc_page(encl_page, true);
> +	if (IS_ERR(epc_page)) {
> +		ret = PTR_ERR(epc_page);
> +		goto err_alloc_epc_page;
> +	}
> +
> +	va_page = sgx_encl_grow(encl);
> +	if (IS_ERR(va_page)) {
> +		ret = PTR_ERR(va_page);
> +		goto err_grow;
> +	}
> +
> +	mutex_lock(&encl->lock);
> +
> +	/*
> +	 * Adding to encl->va_pages must be done under encl->lock.  Ditto for
> +	 * deleting (via sgx_encl_shrink()) in the error path.
> +	 */
> +	if (va_page)
> +		list_add(&va_page->list, &encl->va_pages);
> +
> +	/*
> +	 * Insert prior to EADD in case of OOM.  EADD modifies MRENCLAVE, i.e.
> +	 * can't be gracefully unwound, while failure on EADD/EXTEND is limited
> +	 * to userspace errors (or kernel/hardware bugs).
> +	 */
> +	ret = xa_insert(&encl->page_array, PFN_DOWN(encl_page->desc),
> +			encl_page, GFP_KERNEL);
> +
> +	/*
> +	 * If ret == -EBUSY then page was created in another flow while
> +	 * running without encl->lock
> +	 */
> +	if (ret)
> +		goto err_xa_insert;
> +
> +	pginfo.secs = (unsigned  
> long)sgx_get_epc_virt_addr(encl->secs.epc_page);
> +	pginfo.addr = encl_page->desc & PAGE_MASK;
> +	pginfo.metadata = 0;
> +
> +	ret = __eaug(&pginfo, sgx_get_epc_virt_addr(epc_page));
> +	if (ret)
> +		goto err_eaug;
> +
> +	encl_page->encl = encl;
> +	encl_page->epc_page = epc_page;
> +	encl_page->type = SGX_PAGE_TYPE_REG;
> +	encl->secs_child_cnt++;
> +
> +	sgx_mark_page_reclaimable(encl_page->epc_page);
> +
> +	mutex_unlock(&encl->lock);
> +
> +	return 0;
> +
> +err_eaug:
> +	xa_erase(&encl->page_array, PFN_DOWN(encl_page->desc));
> +
> +err_xa_insert:
> +	sgx_encl_shrink(encl, va_page);
> +	mutex_unlock(&encl->lock);
> +
> +err_grow:
> +	sgx_encl_free_epc_page(epc_page);
> +
> +err_alloc_epc_page:
> +	kfree(encl_page);
> +
> +	return VM_FAULT_SIGBUS;
> +}
> +
> +/**
> + * sgx_ioc_enclave_augment_pages() - The handler for  
> %SGX_IOC_ENCLAVE_AUGMENT_PAGES
> + * @encl:       an enclave pointer
> + * @arg:	a user pointer to a struct sgx_enclave_augment_pages instance
> + *
> + * Request to augment pages to an initialized enclave. The pages must be
> + * acknowledged by the enclave by issuing either ENCLU[EACCEPT] (for a
> + * trivial empty data page) or ENCLU[EACCEPTCOPY] (for a page with  
> arbitrary
> + * permissions and user provided data).
> + *
> + * mmap() protection bits for augmented pages can be arbitrary, i.e.  
> their
> + * permissions are only capped by the VMA, not by EPCM permissions.  
> This is
> + * a constraint because enclave can internally control the final EPCM  
> versions,
> + * for which the kernel has no say.
> + *
> + * Return:
> + * - 0:		Success.
> + * - -EACCES:	The source page is located in a noexec partition.
> + * - -ENOMEM:	Out of EPC pages.
> + * - -EINTR:	The call was interrupted before data was processed.
> + * - -EIO:	EAUG failed.
> + * - -errno:	POSIX error.
> + */
> +static long sgx_ioc_enclave_augment_pages(struct sgx_encl *encl, void  
> __user *user_arg)
> +{
> +	struct sgx_enclave_augment_pages arg;
> +	unsigned long c;
> +	int ret;
> +
> +	if (!test_bit(SGX_ENCL_INITIALIZED, &encl->flags))
> +		return -EINVAL;
> +
> +	if (copy_from_user(&arg, user_arg, sizeof(arg)))
> +		return -EFAULT;
> +
> +	if (sgx_validate_offset_length(encl, arg.offset, arg.length))
> +		return -EINVAL;
> +
> +	for (c = 0 ; c < arg.length; c += PAGE_SIZE) {
> +		if (signal_pending(current)) {
> +			if (!c)
> +				ret = -ERESTARTSYS;
> +
> +			break;
> +		}
> +
> +		if (need_resched())
> +			cond_resched();
> +
> +		ret = sgx_encl_augment_page(encl, arg.offset + c);
> +		if (ret)
> +			break;
> +	}
> +
> +	arg.count = c;
> +
> +	if (copy_to_user(user_arg, &arg, sizeof(arg)))
> +		return -EFAULT;
> +
> +	return ret;
> +}
> +
>  long sgx_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
>  {
>  	struct sgx_encl *encl = filep->private_data;
> @@ -1350,6 +1506,9 @@ long sgx_ioctl(struct file *filep, unsigned int  
> cmd, unsigned long arg)
>  	case SGX_IOC_ENCLAVE_REMOVE_PAGES:
>  		ret = sgx_ioc_enclave_remove_pages(encl, (void __user *)arg);
>  		break;
> +	case SGX_IOC_ENCLAVE_AUGMENT_PAGES:
> +		ret = sgx_ioc_enclave_augment_pages(encl, (void __user *)arg);
> +		break;
>  	default:
>  		ret = -ENOIOCTLCMD;
>  		break;

Powered by blists - more mailing lists