Re: [PATCH v36 12/24] x86/sgx: Add SGX_IOC_ENCLAVE_CREATE

[Jarkko Sakkinen]

On Thu, Aug 27, 2020 at 04:24:50PM +0300, Jarkko Sakkinen wrote:
> > > + * @arg:	userspace pointer to a struct sgx_enclave_create instance
> > > + *
> > > + * Allocate kernel data structures for a new enclave and execute ECREATE after
> > > + * verifying the correctness of the provided SECS.
> > > + *
> > > + * Note, enforcement of restricted and disallowed attributes is deferred until
> > > + * sgx_ioc_enclave_init(), only the architectural correctness of the SECS is
> > > + * checked by sgx_ioc_enclave_create().
> > 
> > From that same review:
> > 
> > "Well, I don't see that checking. Where is it?"
> > 
> > Ok, I'm going to stop here. Please go over v33's review and either
> > address *all* feedback or incorporate it into your patches if you agree
> > with it but do not silently ignore it. One of the things I very strongly
> > detest is ignored review comments.

OK, so sgx_validate_secs() is the validation of what the CPU requires
from the contents of the SECS. That is mean by "architectural
correctness".

I spotted the glitch that makes this confusing.

The change that the comment is related is

https://lore.kernel.org/linux-sgx/20200716135303.276442-16-jarkko.sakkinen@xxxxxxxxxxxxxxx/

This check in sgx_encl_init() should be relocated to this commit:

	/* Check that the required attributes have been authorized. */
	if (encl->secs_attributes & ~encl->allowed_attributes)
		return -EACCES;

It is the "enforcement of restricted and disallowed attributes" part.

Does this make sense to you?

> > -- 
> > Regards/Gruss,
> >     Boris.

/Jarkko