Re: [RFC PATCH v3 01/10] x86/sgx: Introduce mechanism to prevent new initializations of EPC pages

Jarkko Sakkinen
2022-04-03T08:12:36Z

On Fri, Apr 01, 2022 at 10:24:00PM +0800, Cathy Zhang wrote:
> diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
> index b3c2e8d58142..00668e50848d 100644
> --- a/arch/x86/kernel/cpu/sgx/ioctl.c
> +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
> @@ -147,6 +147,7 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs)
> static long sgx_ioc_enclave_create(struct sgx_encl *encl, void __user *arg)
> {
> struct sgx_enclave_create create_arg;
> + int srcu_idx;
> void *secs;
> int ret;
>
> @@ -162,9 +163,20 @@ static long sgx_ioc_enclave_create(struct sgx_encl *encl, void __user *arg)
>
> if (copy_from_user(secs, (void __user *)create_arg.src, PAGE_SIZE))
> ret = -EFAULT;
> - else
> + else {
> + srcu_idx = srcu_read_lock(&sgx_lock_epc_srcu);
> + if (sgx_epc_is_locked()) {
> + srcu_read_unlock(&sgx_lock_epc_srcu, srcu_idx);
> + ret = -EBUSY;
> + goto out;
> + }
> +
> ret = sgx_encl_create(encl, secs);
>
> + srcu_read_unlock(&sgx_lock_epc_srcu, srcu_idx);
> + }
> +
> +out:

Nit: I'd rename this as 'err:'.

> kfree(secs);
> return ret;
> }

Please, take advantage of the label you created anyway:

if (copy_from_user(secs, (void __user *)create_arg.src, PAGE_SIZE {
ret = -EFAULT;
goto out;
}

Then fail cases have the exact same rollback procedure, and the code is
easier to read because it is less nested.

BR, Jarkko