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

[Jarkko Sakkinen <jarkko@kernel.org>]

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