Hi Jarkko,
> -----Original Message-----
> From: Jarkko Sakkinen <jarkko@xxxxxxxxxx>
> Sent: Sunday, April 3, 2022 4:14 PM
> To: Zhang, Cathy <cathy.zhang@xxxxxxxxx>
> Cc: linux-sgx@xxxxxxxxxxxxxxx; x86@xxxxxxxxxx; Chatre, Reinette
> <reinette.chatre@xxxxxxxxx>; Hansen, Dave <dave.hansen@xxxxxxxxx>; Raj,
> Ashok <ashok.raj@xxxxxxxxx>
> Subject: Re: [RFC PATCH v3 01/10] x86/sgx: Introduce mechanism to prevent
> new initializations of EPC pages
>
> 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:'.
Renamed 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.
Update is done. The error branches in this function will all "goto err".
>
> BR, Jarkko
>
