Re: [PATCH v22 12/24] x86/sgx: Linux Enclave Driver

From: Borislav Petkov
Date: Tue Oct 08 2019 - 13:59:31 EST


> Subject: Re: [PATCH v22 12/24] x86/sgx: Linux Enclave Driver

Title needs a verb: "Add a Linux ... "

On Tue, Sep 03, 2019 at 05:26:43PM +0300, Jarkko Sakkinen wrote:
> Intel Software Guard eXtensions (SGX) is a set of CPU instructions that
> can be used by applications to set aside private regions of code and
> data. The code outside the SGX hosted software entity is disallowed to
> access the memory inside the enclave enforced by the CPU. We call these
> entities as enclaves.
>
> This commit implements a driver that provides an ioctl API to construct
> and run enclaves. Enclaves are constructed from pages residing in
> reserved physical memory areas. The contents of these pages can only be
> accessed when they are mapped as part of an enclave, by a hardware
> thread running inside the enclave.
>
> The starting state of an enclave consists of a fixed measured set of
> pages that are copied to the EPC during the construction process by
> using ENCLS leaf functions and Software Enclave Control Structure (SECS)
> that defines the enclave properties.
>
> Enclave are constructed by using ENCLS leaf functions ECREATE, EADD and
> EINIT. ECREATE initializes SECS, EADD copies pages from system memory to
> the EPC and EINIT check a given signed measurement and moves the enclave
> into a state ready for execution.
>
> An initialized enclave can only be accessed through special Thread Control
> Structure (TCS) pages by using ENCLU (ring-3 only) leaf EENTER. This leaf
> function converts a thread into enclave mode and continues the execution in
> the offset defined by the TCS provided to EENTER. An enclave is exited
> through syscall, exception, interrupts or by explicitly calling another
> ENCLU leaf EEXIT.
>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxxxxxxxx>
> Co-developed-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> Co-developed-by: Serge Ayoun <serge.ayoun@xxxxxxxxx>
> Signed-off-by: Serge Ayoun <serge.ayoun@xxxxxxxxx>
> Co-developed-by: Shay Katz-zamir <shay.katz-zamir@xxxxxxxxx>
> Signed-off-by: Shay Katz-zamir <shay.katz-zamir@xxxxxxxxx>
> Co-developed-by: Suresh Siddha <suresh.b.siddha@xxxxxxxxx>
> Signed-off-by: Suresh Siddha <suresh.b.siddha@xxxxxxxxx>
> ---
> Documentation/ioctl/ioctl-number.rst | 1 +
> arch/x86/include/uapi/asm/sgx.h | 55 +++
> arch/x86/include/uapi/asm/sgx_errno.h | 2 +-
> arch/x86/kernel/cpu/sgx/Makefile | 6 +-
> arch/x86/kernel/cpu/sgx/driver.c | 251 +++++++++++
> arch/x86/kernel/cpu/sgx/driver.h | 37 ++
> arch/x86/kernel/cpu/sgx/encl.c | 365 +++++++++++++++
> arch/x86/kernel/cpu/sgx/encl.h | 100 +++++
> arch/x86/kernel/cpu/sgx/ioctl.c | 612 ++++++++++++++++++++++++++
> arch/x86/kernel/cpu/sgx/main.c | 25 +-
> arch/x86/kernel/cpu/sgx/reclaim.c | 2 +-
> arch/x86/kernel/cpu/sgx/sgx.h | 1 +
> 12 files changed, 1444 insertions(+), 13 deletions(-)

That's a fat one. :)

> create mode 100644 arch/x86/include/uapi/asm/sgx.h
> create mode 100644 arch/x86/kernel/cpu/sgx/driver.c
> create mode 100644 arch/x86/kernel/cpu/sgx/driver.h
> create mode 100644 arch/x86/kernel/cpu/sgx/encl.c
> create mode 100644 arch/x86/kernel/cpu/sgx/encl.h
> create mode 100644 arch/x86/kernel/cpu/sgx/ioctl.c
>
> diff --git a/Documentation/ioctl/ioctl-number.rst b/Documentation/ioctl/ioctl-number.rst
> index 7f8dcae7a230..83df9c17c127 100644
> --- a/Documentation/ioctl/ioctl-number.rst
> +++ b/Documentation/ioctl/ioctl-number.rst
> @@ -320,6 +320,7 @@ Code Seq# Include File Comments
> <mailto:tlewis@xxxxxxxxxxxxxx>
> 0xA3 90-9F linux/dtlk.h
> 0xA4 00-1F uapi/linux/tee.h Generic TEE subsystem
> +0xA4 00-1F uapi/asm/sgx.h Intel SGX subsystem (a legit conflict as TEE and SGX do not co-exist)
> 0xAA 00-3F linux/uapi/linux/userfaultfd.h
> 0xAB 00-1F linux/nbd.h
> 0xAC 00-1F linux/raw.h
> diff --git a/arch/x86/include/uapi/asm/sgx.h b/arch/x86/include/uapi/asm/sgx.h
> new file mode 100644
> index 000000000000..c45eeed68144
> --- /dev/null
> +++ b/arch/x86/include/uapi/asm/sgx.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) WITH Linux-syscall-note */

checkpatch is bitching for some reason, I guess it doesn't like the
"WITH" thing or so:

WARNING: 'SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) WITH Linux-syscall-note */' is not supported in LICENSES/...
#98: FILE: arch/x86/include/uapi/asm/sgx.h:1:
+/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) WITH Linux-syscall-note */

WARNING: 'SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause WITH Linux-syscall-note */' is not supported in LICENSES/...
#159: FILE: arch/x86/include/uapi/asm/sgx_errno.h:1:
+/* SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause WITH Linux-syscall-note */

And building this fails:

arch/x86/kernel/cpu/sgx/encl.c: In function âsgx_mmu_notifier_releaseâ:
arch/x86/kernel/cpu/sgx/encl.c:77:3: error: implicit declaration of function âmmu_notifier_unregister_no_releaseâ; did you mean âmmu_notifier_unregisterâ? [-Werror=implicit-function-declaration]
mmu_notifier_unregister_no_release(mn, mm);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
mmu_notifier_unregister
arch/x86/kernel/cpu/sgx/encl.c:78:3: error: implicit declaration of function âmmu_notifier_call_srcuâ; did you mean âmmu_notifier_releaseâ? [-Werror=implicit-function-declaration]
mmu_notifier_call_srcu(&encl_mm->rcu,
^~~~~~~~~~~~~~~~~~~~~~
mmu_notifier_release
cc1: some warnings being treated as errors
make[4]: *** [scripts/Makefile.build:265: arch/x86/kernel/cpu/sgx/encl.o] Error 1
make[4]: *** Waiting for unfinished jobs....
make[3]: *** [scripts/Makefile.build:509: arch/x86/kernel/cpu/sgx] Error 2
make[2]: *** [scripts/Makefile.build:509: arch/x86/kernel/cpu] Error 2
make[1]: *** [scripts/Makefile.build:509: arch/x86/kernel] Error 2
make: *** [Makefile:1670: arch/x86] Error 2
make: *** Waiting for unfinished jobs....

Got a fixed version which I can review instead?

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette