Re: [PATCH v26 10/22] x86/sgx: Linux Enclave Driver

[Jethro Beekman <jethro@fortanix.com>]

[-- Attachment #1: Type: text/plain, Size: 2979 bytes --]

On 2020-02-13 19:07, Sean Christopherson wrote:
> On Thu, Feb 13, 2020 at 02:59:52PM +0100, Jethro Beekman wrote:
>> On 2020-02-09 22:25, Jarkko Sakkinen wrote:
>>> +/**
>>> + * struct sgx_enclave_add_pages - parameter structure for the
>>> + *                                %SGX_IOC_ENCLAVE_ADD_PAGE ioctl
>>> + * @src:	start address for the page data
>>> + * @offset:	starting page offset
>>> + * @length:	length of the data (multiple of the page size)
>>> + * @secinfo:	address for the SECINFO data
>>> + * @flags:	page control flags
>>> + * @count:	number of bytes added (multiple of the page size)
>>> + */
>>> +struct sgx_enclave_add_pages {
>>> +	__u64	src;
>>> +	__u64	offset;
>>> +	__u64	length;
>>> +	__u64	secinfo;
>>> +	__u64	flags;
>>> +	__u64	count;
>>> +};
>>
>> Compared to the last time I looked at the patch set, this API removes the
>> ability to measure individual pages chunks. That is not acceptable.
> 
> Why is it not acceptable?  E.g. what specific use case do you have that
> _requires_ on measuring partial 4k pages of an enclave?

The use case is someone gives me an enclave and I want to load it. If I don't load it exactly as the enclave author specified, the enclave hash will be different, and it won't work.

>> On 2019-10-11 16:37, Sean Christopherson wrote:
>>> Hiding the 256-byte granualarity from userspace is a good idea as it's not
>>> intrinsically tied to the SGX architecture and exists only because of
>>> latency requirements.
>>
>> What do you mean by "it's not intrinsically tied to the SGX architecture"?
>> This is a fundamental part of the SGX instruction set. This is the
>> instruction definition from the SDM: "EEXTEND—Extend Uninitialized Enclave
>> Measurement by 256 Bytes".
> 
> SGX fundamentally works at a 4k granularity.  EEXTEND is special cased
> because extending the measurement is a slow operation, i.e. EEXTEND on more
> than 256 byte chunks, *with the current implementation*, would exceeded
> latency requirements, e.g. block interrupts for too long and hose the
> kernel.
> 
> A future implementation of SGX could change the latency of extending the
> measurement, e.g. a different algorithm that is slower/faster, and so could
> introduce EEXTEND2 which would work at a different granularity than EEXTEND.
> 
> EEXTEND could have avoided the latency problems via other methods, e.g. by
> being interruptible a la EINIT and/or by being restartable.  But that ship
> has sailed, so to avoid future complication in the kernel's ABI we're
> proposing/advocating supporting only measuring at a 4k granularity.

It doesn't really matter what the reason for the current EEXTEND implementation is. It's there now in the ISA, it needs to be supported. If EEXTEND2 (or whatever) is added to the ISA, it will likely influence the enclave hash, so userspace would need to specify what instruction is used for measuring anyway.

--
Jethro Beekman | Fortanix


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4054 bytes --]