[PATCH RESEND 0/3] x86/sgx: eextend ioctl

[PATCH RESEND 0/3] x86/sgx: eextend ioctl

[Raoul Strackx]

The sgx driver can only load enclaves whose pages are fully measured. This may exclude existing enclaves from running. This patch adds a new ioctl to measure 256 byte chunks at a time. Such a requirement has been discussed before:

https://lore.kernel.org/linux-sgx/20200220221038.GA26618@linux.intel.com/T/#m93597f53d354201e72e26d93a968f167fcdf5930


Raoul Strackx (3):
  x86/sgx: Adding eextend ioctl
  x86/sgx: Fix compatibility issue with OPENSSL < 1.1.0
  x86/sgx: eextend ioctl selftest

 arch/x86/include/uapi/asm/sgx.h         | 11 +++++
 arch/x86/kernel/cpu/sgx/ioctl.c         | 81 ++++++++++++++++++++++++++++-----
 tools/testing/selftests/sgx/defines.h   |  1 +
 tools/testing/selftests/sgx/load.c      | 55 ++++++++++++++++++----
 tools/testing/selftests/sgx/main.h      |  1 +
 tools/testing/selftests/sgx/sigstruct.c | 43 ++++++++---------
 6 files changed, 152 insertions(+), 40 deletions(-)

-- 
2.7.4

Re: [PATCH RESEND 0/3] x86/sgx: eextend ioctl

[Dave Hansen]

On 3/31/21 5:50 AM, Raoul Strackx wrote:
> The sgx driver can only load enclaves whose pages are fully measured.
> This may exclude existing enclaves from running. This patch adds a
> new ioctl to measure 256 byte chunks at a time.

The changelogs here are pretty sparse.  Could you explain in a bit more
detail what's going on?

A review of the relevant pieces of the SGX architecture would be
appreciated.

Re: [PATCH RESEND 0/3] x86/sgx: eextend ioctl

[Raoul Strackx]

On 3/31/21 5:53 PM, Dave Hansen wrote:
> On 3/31/21 5:50 AM, Raoul Strackx wrote:
>> The sgx driver can only load enclaves whose pages are fully measured.
>> This may exclude existing enclaves from running. This patch adds a
>> new ioctl to measure 256 byte chunks at a time.
> 
> The changelogs here are pretty sparse.  Could you explain in a bit more
> detail what's going on?
> 
> A review of the relevant pieces of the SGX architecture would be
> appreciated.
> 

Yes the explanation was very succinct. A more elaborate explanation:

BACKGROUND
Creation of an SGX enclave consists of three steps. First, a new enclave 
environment is created by the ECREATE leaf function. Some enclave settings 
are specified at this step by passing an SGX Enclave Control Structure 
(SECS) that contains the enclave MRENCLAVE, MRSIGNER, etc. This 
instruction also starts a cryptographic log of the enclave being built. 
(This log should eventually result in the MRENCLAVE.)
Second, pages are added to the enclave. The EADD leaf function copies 4KB 
data to an empty EPC page. The cryptographic log records (among other 
properties) the location and access rights of the page being added. It 
_does not_ include a measurement of the page content. When the enclave 
writer wishes to ensure the content of the enclave page as well, she must 
use the EEXTEND leaf function. Extending the enclave cryptographic log can 
only be done per 256 bytes. Extending the log with a full 4K page thus 
requires 16 invocations of the EEXTEND leaf function.
Finally, the enclave is finalized by the EINIT leaf function. Any new 
invocations of the EADD or EEXTEND leaf functions will result in a fault. 
With EINIT a number of checks are performed as well. A cryptographic hash 
is computed over the final cryptographic log and compared to the MRENCLAVE 
field of the SECS structure passed to the ECREATE leaf function (see step 
one). The signature (MRSIGNER) over this MRENCLAVE is verified as well. 
When all checks pass, the enclave enters an executable state.

PROBLEM STATEMENT
The SGX driver currently only supports extending the cryptographic log as 
part of the EADD leaf function and _must_ measure full 4K pages. Not all 
enclaves may have been constructed within these constraints. Such enclaves 
currently cannot be build on the Linux platform. Trying to do so will 
result in a different cryptographic log; the MRENCLAVE specified at 
enclave creation time will not match the cryptographic log kept by the 
processor and EINIT will fail.

SOLUTION OF THIS PATCH
This patch adds a new ioctl to enable userspace to execute EEXTEND leaf 
functions per 256 bytes of enclave memory. This enables enclaves to be 
build as specified by enclave providers.



I'm still very new to how Linux kernel patches are handled. Where
would such a more elaborate description usually go: the cover page, the
commit that introduces the ioctl or both?

Regards,
Raoul

Re: [PATCH RESEND 0/3] x86/sgx: eextend ioctl

[Dave Hansen]

On 4/1/21 7:56 AM, Raoul Strackx wrote:
> 
> SOLUTION OF THIS PATCH
> This patch adds a new ioctl to enable userspace to execute EEXTEND leaf 
> functions per 256 bytes of enclave memory. This enables enclaves to be 
> build as specified by enclave providers.

I think tying the user ABI to the SGX architecture this closely is a
mistake.

Do we need another ioctl() or can we just relax the existing add_pages
ioctl() to allow unaligned addresses?

Re: [PATCH RESEND 0/3] x86/sgx: eextend ioctl

[Raoul Strackx]

On 4/1/21 6:11 PM, Dave Hansen wrote:
> On 4/1/21 7:56 AM, Raoul Strackx wrote:
>>
>> SOLUTION OF THIS PATCH
>> This patch adds a new ioctl to enable userspace to execute EEXTEND leaf 
>> functions per 256 bytes of enclave memory. This enables enclaves to be 
>> build as specified by enclave providers.
> 
> I think tying the user ABI to the SGX architecture this closely is a
> mistake.
> 
> Do we need another ioctl() or can we just relax the existing add_pages
> ioctl() to allow unaligned addresses?
> 

I've considered this. In order to do an EEXTEND without an EADD, we'd
need to add a flag DONT_ADD_PAGES flag to `add_pages` ioctl as well. Two
separate ioctls, one for adding, another for extending made more sense
to me.

Raoul

Re: [PATCH RESEND 0/3] x86/sgx: eextend ioctl

[Dave Hansen]

On 4/1/21 10:49 AM, Raoul Strackx wrote:
> On 4/1/21 6:11 PM, Dave Hansen wrote:
>> On 4/1/21 7:56 AM, Raoul Strackx wrote:
>>> SOLUTION OF THIS PATCH
>>> This patch adds a new ioctl to enable userspace to execute EEXTEND leaf 
>>> functions per 256 bytes of enclave memory. This enables enclaves to be 
>>> build as specified by enclave providers.
>> I think tying the user ABI to the SGX architecture this closely is a
>> mistake.
>>
>> Do we need another ioctl() or can we just relax the existing add_pages
>> ioctl() to allow unaligned addresses?
>>
> I've considered this. In order to do an EEXTEND without an EADD, we'd
> need to add a flag DONT_ADD_PAGES flag to `add_pages` ioctl as well. Two
> separate ioctls, one for adding, another for extending made more sense
> to me.

So, we're talking here about pages that have been EEADDED, but for which
we do not want to include the entire contents of the page?  Do these
contents always include the beginning of the page, or can the holes be
anywhere?

Re: [PATCH RESEND 0/3] x86/sgx: eextend ioctl

[Jethro Beekman]

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

On 2021-04-01 20:40, Dave Hansen wrote:
> On 4/1/21 10:49 AM, Raoul Strackx wrote:
>> On 4/1/21 6:11 PM, Dave Hansen wrote:
>>> On 4/1/21 7:56 AM, Raoul Strackx wrote:
>>>> SOLUTION OF THIS PATCH
>>>> This patch adds a new ioctl to enable userspace to execute EEXTEND leaf 
>>>> functions per 256 bytes of enclave memory. This enables enclaves to be 
>>>> build as specified by enclave providers.
>>> I think tying the user ABI to the SGX architecture this closely is a
>>> mistake.
>>>
>>> Do we need another ioctl() or can we just relax the existing add_pages
>>> ioctl() to allow unaligned addresses?
>>>
>> I've considered this. In order to do an EEXTEND without an EADD, we'd
>> need to add a flag DONT_ADD_PAGES flag to `add_pages` ioctl as well. Two
>> separate ioctls, one for adding, another for extending made more sense
>> to me.
> 
> So, we're talking here about pages that have been EEADDED, but for which
> we do not want to include the entire contents of the page?  Do these
> contents always include the beginning of the page, or can the holes be
> anywhere?

Holes can be anywhere, and EEXTEND calls need not be sequential in memory address or even relate to the most recently EADDed page.

--
Jethro Beekman | Fortanix



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

Re: [PATCH RESEND 0/3] x86/sgx: eextend ioctl

[Dave Hansen]

On 4/2/21 1:38 AM, Jethro Beekman wrote:
>> So, we're talking here about pages that have been EEADDED, but for
>> which we do not want to include the entire contents of the page?
>> Do these contents always include the beginning of the page, or can
>> the holes be anywhere?
> Holes can be anywhere, and EEXTEND calls need not be sequential in
> memory address or even relate to the most recently EADDed page.

I think you're referring to the SGX architecture itself here.  The
architecture permits this, right?

But, why would an enclave loader application ever do this?  Is this
something we want to support in Linux?

Re: [PATCH RESEND 0/3] x86/sgx: eextend ioctl

[Jethro Beekman]

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

On 2021-04-02 17:53, Dave Hansen wrote:
> On 4/2/21 1:38 AM, Jethro Beekman wrote:
>>> So, we're talking here about pages that have been EEADDED, but for
>>> which we do not want to include the entire contents of the page?
>>> Do these contents always include the beginning of the page, or can
>>> the holes be anywhere?
>> Holes can be anywhere, and EEXTEND calls need not be sequential in
>> memory address or even relate to the most recently EADDed page.
> 
> I think you're referring to the SGX architecture itself here.  The
> architecture permits this, right?

Yes.

> But, why would an enclave loader application ever do this? 

e.g. to save space

> Is this something we want to support in Linux?

Why not? Is there a good reason to not fully support this part of the CPU architecture?

--
Jethro Beekman | Fortanix


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

Re: [PATCH RESEND 0/3] x86/sgx: eextend ioctl

[Dave Hansen]

On 4/2/21 11:31 AM, Jethro Beekman wrote:
> On 2021-04-02 17:53, Dave Hansen wrote:
>> On 4/2/21 1:38 AM, Jethro Beekman wrote:
>>>> So, we're talking here about pages that have been EEADDED, but for
>>>> which we do not want to include the entire contents of the page?
>>>> Do these contents always include the beginning of the page, or can
>>>> the holes be anywhere?
>>> Holes can be anywhere, and EEXTEND calls need not be sequential in
>>> memory address or even relate to the most recently EADDed page.
>>
>> I think you're referring to the SGX architecture itself here.  The
>> architecture permits this, right?
> 
> Yes.
> 
>> But, why would an enclave loader application ever do this? 
> 
> e.g. to save space

How does this save space, exactly?  What space does it save?

Let's say I want to add 4352 bytes of data to an enclave.  Today, I have
to page-align the beginning and end of that 4352 bytes, and call the add
ioctl() to add the two resulting pages.  It consumes two EPC pages.

With EEXTEND, if I want to add the data, I only need to page-align the
beginning of the data.  I call add_page on the first page, then eextend
on the 256 bytes.  It consumes two EPC pages.

I guess you can argue that this saves padding out the second page, which
would *theoretically* temporarily eat up one extra page of non-enclave
memory and the cost of a 256-byte memcpy.

>> Is this something we want to support in Linux?
> 
> Why not? Is there a good reason to not fully support this part of the
> CPU architecture?

We don't blindly support CPU features in Linux.  They need to do
something *useful*.  I'm still missing what this does which is useful.

Does any actual, real-world enclave want this functionality?  Why?

P.S. There are plenty of things you can do with the SGX architecture
that we probably won't ever implement in Linux.

Re: [PATCH RESEND 0/3] x86/sgx: eextend ioctl

[Jethro Beekman]

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

On 2021-04-02 20:42, Dave Hansen wrote:
> On 4/2/21 11:31 AM, Jethro Beekman wrote:
>> On 2021-04-02 17:53, Dave Hansen wrote:
>>> On 4/2/21 1:38 AM, Jethro Beekman wrote:
>>>>> So, we're talking here about pages that have been EEADDED, but for
>>>>> which we do not want to include the entire contents of the page?
>>>>> Do these contents always include the beginning of the page, or can
>>>>> the holes be anywhere?
>>>> Holes can be anywhere, and EEXTEND calls need not be sequential in
>>>> memory address or even relate to the most recently EADDed page.
>>>
>>> I think you're referring to the SGX architecture itself here.  The
>>> architecture permits this, right?
>>
>> Yes.
>>
>>> But, why would an enclave loader application ever do this? 
>>
>> e.g. to save space
> 
> How does this save space, exactly?  What space does it save?

With the current driver interface, if you want to communicate an application binary that has pages that are at least partially measured, you need to communicate the entire page (to ensure the same measurement for the entire page), even though most of that page's contents are irrelevant to the application.

> 
> Let's say I want to add 4352 bytes of data to an enclave.  Today, I have
> to page-align the beginning and end of that 4352 bytes, and call the add
> ioctl() to add the two resulting pages.  It consumes two EPC pages.
> 
> With EEXTEND, if I want to add the data, I only need to page-align the
> beginning of the data.  I call add_page on the first page, then eextend
> on the 256 bytes.  It consumes two EPC pages.
> 
> I guess you can argue that this saves padding out the second page, which
> would *theoretically* temporarily eat up one extra page of non-enclave
> memory and the cost of a 256-byte memcpy.
> 
>>> Is this something we want to support in Linux?
>>
>> Why not? Is there a good reason to not fully support this part of the
>> CPU architecture?
> 
> We don't blindly support CPU features in Linux.  They need to do
> something *useful*.  I'm still missing what this does which is useful.

Enclaves can only be loaded exactly as specified by the developer, this is the whole point of the measurement architecture. By not supporting arbitrary EADD/EEXTEND flows, the SGX application ecosystem is effectively split into two: SGX applications that run everywhere and SGX applications that run everywhere except on Linux. So, the "something useful" is being compatible. Linux has plenty of features that exist solely for compatibility with other systems, such as binfmt_misc.

> 
> Does any actual, real-world enclave want this functionality?  Why?
> 
> P.S. There are plenty of things you can do with the SGX architecture
> that we probably won't ever implement in Linux.
> 

How so? Linux doesn't normally put arbitrary restrictions on what userspace does when it's not interacting with the kernel. Userspace is free to load its own memory contents and execute all the instructions it wants. AFAIK, besides this EEXTEND issue there is nothing of the SGX architecture that SGX applications may be using that's not supported.

--
Jethro Beekman | Fortanix


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

Re: [PATCH RESEND 0/3] x86/sgx: eextend ioctl

[Dave Hansen]

On 4/2/21 12:38 PM, Jethro Beekman wrote:
> On 2021-04-02 20:42, Dave Hansen wrote:
>> On 4/2/21 11:31 AM, Jethro Beekman wrote:
>>> On 2021-04-02 17:53, Dave Hansen wrote:
>>>> But, why would an enclave loader application ever do this?
>>> 
>>> e.g. to save space
>> 
>> How does this save space, exactly?  What space does it save?
> 
> With the current driver interface, if you want to communicate an 
> application binary that has pages that are at least partially
> measured, you need to communicate the entire page (to ensure the same
> measurement for the entire page), even though most of  that page's contents
> are irrelevant to the application.

Again, how does this save space?

Are you literally talking about the temporary cost of allocating *one* page?

>> We don't blindly support CPU features in Linux.  They need to do
>> something *useful*.  I'm still missing what this does which is
>> useful.
> 
> Enclaves can only be loaded exactly as specified by the developer,
this is the whole point of the measurement architecture. By not
supporting arbitrary EADD/EEXTEND flows, the SGX application ecosystem
is effectively split into two: SGX applications that run everywhere and
SGX applications that run everywhere except on Linux. So, the "something
useful" is being compatible. Linux has plenty of features that exist
solely for compatibility with other systems, such as binfmt_misc.

That's a mildly compelling argument.  Is it theoretical or practical?
Are folks *practically* going to run the same enclave binaries on Linux
and Windows?

I guess the enclave never interacts with the OS directly, so this is
_possible_.  But, are enclaves really that divorced from the "runtimes"
which *are* OS-specific?

>> Does any actual, real-world enclave want this functionality?  Why?

I didn't see an answer on this one.

>> P.S. There are plenty of things you can do with the SGX
>> architecture that we probably won't ever implement in Linux.
> 
> How so? 

For example, the architecture allows swapping VA pages and guest enclave
pages.  But, we may never do either of those.

Re: [PATCH RESEND 0/3] x86/sgx: eextend ioctl

[Jethro Beekman]

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

On 2021-04-02 21:50, Dave Hansen wrote:
> 
> On 4/2/21 12:38 PM, Jethro Beekman wrote:
>> On 2021-04-02 20:42, Dave Hansen wrote:
>>> On 4/2/21 11:31 AM, Jethro Beekman wrote:
>>>> On 2021-04-02 17:53, Dave Hansen wrote:
>>>>> But, why would an enclave loader application ever do this?
>>>>
>>>> e.g. to save space
>>>
>>> How does this save space, exactly?  What space does it save?
>>
>> With the current driver interface, if you want to communicate an 
>> application binary that has pages that are at least partially
>> measured, you need to communicate the entire page (to ensure the same
>> measurement for the entire page), even though most of  that page's contents
>> are irrelevant to the application.
> 
> Again, how does this save space?
> 
> Are you literally talking about the temporary cost of allocating *one* page?

No I'm talking about the amount of disk space/network traffic needed to distribute the application.

> 
>>> We don't blindly support CPU features in Linux.  They need to do
>>> something *useful*.  I'm still missing what this does which is
>>> useful.
>>
>> Enclaves can only be loaded exactly as specified by the developer,
> this is the whole point of the measurement architecture. By not
> supporting arbitrary EADD/EEXTEND flows, the SGX application ecosystem
> is effectively split into two: SGX applications that run everywhere and
> SGX applications that run everywhere except on Linux. So, the "something
> useful" is being compatible. Linux has plenty of features that exist
> solely for compatibility with other systems, such as binfmt_misc.
> 
> That's a mildly compelling argument.  Is it theoretical or practical?
> Are folks *practically* going to run the same enclave binaries on Linux
> and Windows?

This is certainly practical. As you mention below, enclaves don't interact with the OS, so this should really be the default. It's quite puzzling to me that the Intel SGX SDK /doesn't/ let you do this (easily, it's possible with https://github.com/fortanix/rust-sgx/blob/master/sgxs-tools/src/bin/isgx-pe2sgx.rs). The x86_64-fortanix-unknown-sgx target that's part of Rust is fully designed to be OS-agnostic.

> 
> I guess the enclave never interacts with the OS directly, so this is
> _possible_.  But, are enclaves really that divorced from the "runtimes"
> which *are* OS-specific?
> 
>>> Does any actual, real-world enclave want this functionality?  Why?
> 
> I didn't see an answer on this one.

Yes, we have enclaves that use this functionality. They already exist so they can't be changed (without changing the measurement) and we'd like to stop using the out of tree driver as soon as possible. However, we are not able to load the enclaves.

> 
>>> P.S. There are plenty of things you can do with the SGX
>>> architecture that we probably won't ever implement in Linux.
>>
>> How so? 
> 
> For example, the architecture allows swapping VA pages and guest enclave
> pages.  But, we may never do either of those.
> 

This is not an application-visible part of the architecture.* Resource management is squarely in the kernel's purview.

(* I suppose you could argue that without VA paging the practical enclave size is limited to 512× the EPC size, so ~46GiB for systems with 128MiB PRM Considering the overhead of EPC paging in general, that really seems more of a theoretical limitations)

--
Jethro Beekman | Fortanix


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

Re: [PATCH RESEND 0/3] x86/sgx: eextend ioctl

[Dave Hansen]

On 4/2/21 1:20 PM, Jethro Beekman wrote:
> On 2021-04-02 21:50, Dave Hansen wrote:
>> Again, how does this save space?
>>
>> Are you literally talking about the temporary cost of allocating *one* page?
> 
> No I'm talking about the amount of disk space/network traffic needed
> to distribute the application.

Am I just horribly confused about how executable formats work?

Executables go through an SGX loader that copies them into SGX memory
with the kernel's help.

That executable can have *ANY* format, really.

Then, a loader needs to read that format and turn it into data that can
be shoved into the kernel.  The simplest way to do this is to just
mmap() the executable and point the kernel ioctl()'s at the executable's
pages one-by-one.

The other way a loader *could* work is to copy the data to a temporary
location and then hand the temporary location to the SGX ioctl()s.

Let's say the kernel *REQUIRED* page-aligned and page-sized ioctl()
arguments forever.  If an executable had a 256-byte-sized chunk of data,
all the loader would have to do is allocate a page, copy the data in
there, and then pass that whole page into the ioctl().

In other words, the loading restrictions (page alignment) have little to
nothing to do with the format of the thing loading the executable.

But, in no way does a kernel page-size-based ABI *REQUIRE* that an
underlying binary format represent things only in page-sized chunks.
Look at how many page-sized executables there are in /bin.  Yet, they
can only be mapped into the address space in PAGE_SIZE increments.

>>>> Does any actual, real-world enclave want this functionality?  Why?
>>
>> I didn't see an answer on this one.
> 
> Yes, we have enclaves that use this functionality. They already exist
> so they can't be changed (without changing the measurement) and we'd
> like to stop using the out of tree driver as soon as possible.
> However, we are not able to load the enclaves.
OK, so please give this series another shot.  Please explain why you
*ACTUALLY* need it and what the goals are.  Please explain why you can't
just relax the restrictions of the existing add ioctl() to take
<PAGE_SIZE arguments.

As far as I can tell, there are only two coherent arguments for this
functionality:
1. It makes the loader simpler so that it doesn't need temporary pages
2. It would allow old enclaves created with non-upstream-Linux SGX
   implementations to end up with the same signatures on these
   implementations as upstream Linux.

I find both of those pretty weak arguments.  Doing #2 just for the
out-of-tree Linux implementation also encourages folks to establish ABI
out of the tree and then foist it on upstream later.  That's not super cool.
But, I guess this would be nice to the folks that have gone to the
trouble of building SGX enclaves for all these years with no upstream
support.

I'll try to look at it with fresh eyes once this is in place.

Re: [PATCH RESEND 0/3] x86/sgx: eextend ioctl

[Jethro Beekman]

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

On 2021-04-02 22:48, Dave Hansen wrote:
> On 4/2/21 1:20 PM, Jethro Beekman wrote:
>> On 2021-04-02 21:50, Dave Hansen wrote:
>>> Again, how does this save space?
>>>
>>> Are you literally talking about the temporary cost of allocating *one* page?
>>
>> No I'm talking about the amount of disk space/network traffic needed
>> to distribute the application.
> 
> Am I just horribly confused about how executable formats work?
> 
> Executables go through an SGX loader that copies them into SGX memory
> with the kernel's help.
> 
> That executable can have *ANY* format, really.
> 
> Then, a loader needs to read that format and turn it into data that can
> be shoved into the kernel.

Sure, you can define any compression format or conversion rules between executable formats. But the native “executable format” for SGX is very clearly defined in the Intel SDM as a specific sequence of ECREATE, EADD, EEXTEND and EINIT calls. It's that sequence that's used for loading the enclave and it's that sequence that's used for code signing. So when I say save space I mean save space in the native format.

Not EEXTENDing unnecessarily also reduces enclave load time if you're looking for additional arguments.

> The simplest way to do this is to just
> mmap() the executable and point the kernel ioctl()'s at the executable's
> pages one-by-one.
> 
> The other way a loader *could* work is to copy the data to a temporary
> location and then hand the temporary location to the SGX ioctl()s.
> 
> Let's say the kernel *REQUIRED* page-aligned and page-sized ioctl()
> arguments forever.  If an executable had a 256-byte-sized chunk of data,
> all the loader would have to do is allocate a page, copy the data in
> there, and then pass that whole page into the ioctl().
> 
> In other words, the loading restrictions (page alignment) have little to
> nothing to do with the format of the thing loading the executable.
> 
> But, in no way does a kernel page-size-based ABI *REQUIRE* that an
> underlying binary format represent things only in page-sized chunks.
> Look at how many page-sized executables there are in /bin.  Yet, they
> can only be mapped into the address space in PAGE_SIZE increments.
> 
>>>>> Does any actual, real-world enclave want this functionality?  Why?
>>>
>>> I didn't see an answer on this one.
>>
>> Yes, we have enclaves that use this functionality. They already exist
>> so they can't be changed (without changing the measurement) and we'd
>> like to stop using the out of tree driver as soon as possible.
>> However, we are not able to load the enclaves.
> OK, so please give this series another shot.  Please explain why you
> *ACTUALLY* need it and what the goals are.  Please explain why you can't
> just relax the restrictions of the existing add ioctl() to take
> <PAGE_SIZE arguments.
> 
> As far as I can tell, there are only two coherent arguments for this
> functionality:
> 1. It makes the loader simpler so that it doesn't need temporary pages
> 2. It would allow old enclaves created with non-upstream-Linux SGX
>    implementations to end up with the same signatures on these
>    implementations as upstream Linux.
> 
> I find both of those pretty weak arguments.  Doing #2 just for the
> out-of-tree Linux implementation also encourages folks to establish ABI
> out of the tree and then foist it on upstream later.  That's not super cool.
> But, I guess this would be nice to the folks that have gone to the
> trouble of building SGX enclaves for all these years with no upstream
> support.

Linux doesn't exist in a vacuum. People who write SGX applications write those applications for SGX, not for Linux. For Linux to claim to support SGX version 1, it should support all SGX version 1 applications. The current implementation is not SGX version 1, it's some limited subset of it.

The cost to Linux for supporting all of SGX version 1 seems excessively low.

SGX defines lots of things and you may not see the use case for all of this immediately. No one has a usecase for creating enclaves with SECS.SSAFRAMESIZE = 1000 or TCS.NSSA = 3. Why did you not demand checks for this in the ECREATE/EADD ioctls?

> 
> I'll try to look at it with fresh eyes once this is in place.
> 

--
Jethro Beekman | Fortanix


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

Re: [PATCH RESEND 0/3] x86/sgx: eextend ioctl

[Dave Hansen]

On 4/8/21 8:27 AM, Jethro Beekman wrote:
> But the native “executable format” for SGX is very clearly defined in
> the Intel SDM as a specific sequence of ECREATE, EADD, EEXTEND and
> EINIT calls. It's that sequence that's used for loading the enclave
> and it's that sequence that's used for code signing. So when I say
> save space I mean save space in the native format.
> 
> Not EEXTENDing unnecessarily also reduces enclave load time if
> you're looking for additional arguments.
I look forward to all of this being clearly explained in your resubmission.

> SGX defines lots of things and you may not see the use case for all
> of this immediately. No one has a usecase for creating enclaves with
> SECS.SSAFRAMESIZE = 1000 or TCS.NSSA = 3. Why did you not demand
> checks for this in the ECREATE/EADD ioctls?
There's a difference between adding code to support a feature and adding
code to *RESTRICT* use of a feature.

Re: [PATCH RESEND 0/3] x86/sgx: eextend ioctl

[Jarkko Sakkinen]

On Fri, Apr 02, 2021 at 08:31:19PM +0200, Jethro Beekman wrote:
> On 2021-04-02 17:53, Dave Hansen wrote:
> > On 4/2/21 1:38 AM, Jethro Beekman wrote:
> >>> So, we're talking here about pages that have been EEADDED, but for
> >>> which we do not want to include the entire contents of the page?
> >>> Do these contents always include the beginning of the page, or can
> >>> the holes be anywhere?
> >> Holes can be anywhere, and EEXTEND calls need not be sequential in
> >> memory address or even relate to the most recently EADDed page.
> > 
> > I think you're referring to the SGX architecture itself here.  The
> > architecture permits this, right?
> 
> Yes.
> 
> > But, why would an enclave loader application ever do this? 
> 
> e.g. to save space
> 
> > Is this something we want to support in Linux?
> 
> Why not? Is there a good reason to not fully support this part of the CPU architecture?

Yes, in generic sense :-)

If one would disagree, that would be same as saying that everything should
execute in ring-0 because that only gives "full support".

/Jarkko

Re: [PATCH RESEND 0/3] x86/sgx: eextend ioctl

[Jethro Beekman]

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

On 2021-04-04 18:04, Jarkko Sakkinen wrote:
> On Fri, Apr 02, 2021 at 08:31:19PM +0200, Jethro Beekman wrote:
>> On 2021-04-02 17:53, Dave Hansen wrote:
>>> On 4/2/21 1:38 AM, Jethro Beekman wrote:
>>>>> So, we're talking here about pages that have been EEADDED, but for
>>>>> which we do not want to include the entire contents of the page?
>>>>> Do these contents always include the beginning of the page, or can
>>>>> the holes be anywhere?
>>>> Holes can be anywhere, and EEXTEND calls need not be sequential in
>>>> memory address or even relate to the most recently EADDed page.
>>>
>>> I think you're referring to the SGX architecture itself here.  The
>>> architecture permits this, right?
>>
>> Yes.
>>
>>> But, why would an enclave loader application ever do this? 
>>
>> e.g. to save space
>>
>>> Is this something we want to support in Linux?
>>
>> Why not? Is there a good reason to not fully support this part of the CPU architecture?
> 
> Yes, in generic sense :-)
> 
> If one would disagree, that would be same as saying that everything should
> execute in ring-0 because that only gives "full support".

How is that the same? Please make an effort to reasonably interpret what I'm saying.

--
Jethro Beekman | Fortanix




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

Re: [PATCH RESEND 0/3] x86/sgx: eextend ioctl

[Jethro Beekman]

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

On 2021-04-01 18:11, Dave Hansen wrote:
> On 4/1/21 7:56 AM, Raoul Strackx wrote:
>>
>> SOLUTION OF THIS PATCH
>> This patch adds a new ioctl to enable userspace to execute EEXTEND leaf 
>> functions per 256 bytes of enclave memory. This enables enclaves to be 
>> build as specified by enclave providers.
> 
> I think tying the user ABI to the SGX architecture this closely is a
> mistake.
> 
> Do we need another ioctl() or can we just relax the existing add_pages
> ioctl() to allow unaligned addresses?
> 

Some version of the driver used to pass a 16-bit bitset of which of the 16 256-byte parts of a 4096-byte page to measure. This was removed at some point in favor of a separate ioctl to be added later. That is this. See the discussion linked in the cover letter.

--
Jethro Beekman | Fortanix


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