| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <13b0ccfa-ea39-4b8a-f09b-6fa5c371ce9b@amazon.com>
Date: Thu, 28 May 2020 21:05:12 +0300
From: "Paraschiv, Andra-Irina" <andraprs@...zon.com>
To: Stefan Hajnoczi <stefanha@...il.com>
CC: <linux-kernel@...r.kernel.org>,
Anthony Liguori <aliguori@...zon.com>,
Benjamin Herrenschmidt <benh@...nel.crashing.org>,
Colm MacCarthaigh <colmmacc@...zon.com>,
Bjoern Doebel <doebel@...zon.de>,
David Woodhouse <dwmw@...zon.co.uk>,
Frank van der Linden <fllinden@...zon.com>,
"Alexander Graf" <graf@...zon.de>,
Martin Pohlack <mpohlack@...zon.de>,
Matt Wilson <msw@...zon.com>,
Paolo Bonzini <pbonzini@...hat.com>,
Balbir Singh <sblbir@...zon.com>,
Stefano Garzarella <sgarzare@...hat.com>,
"Stefan Hajnoczi" <stefanha@...hat.com>,
Stewart Smith <trawets@...zon.com>,
"Uwe Dannowski" <uwed@...zon.de>, <kvm@...r.kernel.org>,
<ne-devel-upstream@...zon.com>
Subject: Re: [PATCH v3 01/18] nitro_enclaves: Add ioctl interface definition
On 27/05/2020 11:49, Stefan Hajnoczi wrote:
> On Tue, May 26, 2020 at 01:13:17AM +0300, Andra Paraschiv wrote:
>> The Nitro Enclaves driver handles the enclave lifetime management. This
>> includes enclave creation, termination and setting up its resources such
>> as memory and CPU.
>>
>> An enclave runs alongside the VM that spawned it. It is abstracted as a
>> process running in the VM that launched it. The process interacts with
>> the NE driver, that exposes an ioctl interface for creating an enclave
>> and setting up its resources.
>>
>> Include part of the KVM ioctls in the provided ioctl interface, with
>> additional NE ioctl commands that e.g. triggers the enclave run.
>>
>> Signed-off-by: Alexandru Vasile <lexnv@...zon.com>
>> Signed-off-by: Andra Paraschiv <andraprs@...zon.com>
>> ---
>> Changelog
>>
>> v2 -> v3
>>
>> * Remove the GPL additional wording as SPDX-License-Identifier is already in
>> place.
>>
>> v1 -> v2
>>
>> * Add ioctl for getting enclave image load metadata.
>> * Update NE_ENCLAVE_START ioctl name to NE_START_ENCLAVE.
>> * Add entry in Documentation/userspace-api/ioctl/ioctl-number.rst for NE ioctls.
>> * Update NE ioctls definition based on the updated ioctl range for major and
>> minor.
>> ---
>> .../userspace-api/ioctl/ioctl-number.rst | 5 +-
>> include/linux/nitro_enclaves.h | 11 ++++
>> include/uapi/linux/nitro_enclaves.h | 65 +++++++++++++++++++
>> 3 files changed, 80 insertions(+), 1 deletion(-)
>> create mode 100644 include/linux/nitro_enclaves.h
>> create mode 100644 include/uapi/linux/nitro_enclaves.h
>>
>> diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
>> index f759edafd938..8a19b5e871d3 100644
>> --- a/Documentation/userspace-api/ioctl/ioctl-number.rst
>> +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
>> @@ -325,8 +325,11 @@ Code Seq# Include File Comments
>> 0xAC 00-1F linux/raw.h
>> 0xAD 00 Netfilter device in development:
>> <mailto:rusty@...tcorp.com.au>
>> -0xAE all linux/kvm.h Kernel-based Virtual Machine
>> +0xAE 00-1F linux/kvm.h Kernel-based Virtual Machine
>> <mailto:kvm@...r.kernel.org>
>> +0xAE 40-FF linux/kvm.h Kernel-based Virtual Machine
>> + <mailto:kvm@...r.kernel.org>
>> +0xAE 20-3F linux/nitro_enclaves.h Nitro Enclaves
>> 0xAF 00-1F linux/fsl_hypervisor.h Freescale hypervisor
>> 0xB0 all RATIO devices in development:
>> <mailto:vgo@...io.de>
> Reusing KVM ioctls seems a little hacky. Even the ioctls that are used
> by this driver don't use all the fields or behave in the same way as
> kvm.ko.
>
> For example, the memory regions slot number is not used by Nitro
> Enclaves.
>
> It would be cleaner to define NE-specific ioctls instead.
Indeed, a couple of fields / parameters are not used during the KVM
ioctl calls for now.
Will adapt the logic to follow a similar model of creating VMs and
adding resources, with NE ioctls.
>
>> diff --git a/include/linux/nitro_enclaves.h b/include/linux/nitro_enclaves.h
>> new file mode 100644
>> index 000000000000..d91ef2bfdf47
>> --- /dev/null
>> +++ b/include/linux/nitro_enclaves.h
>> @@ -0,0 +1,11 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
>> + */
>> +
>> +#ifndef _LINUX_NITRO_ENCLAVES_H_
>> +#define _LINUX_NITRO_ENCLAVES_H_
>> +
>> +#include <uapi/linux/nitro_enclaves.h>
>> +
>> +#endif /* _LINUX_NITRO_ENCLAVES_H_ */
>> diff --git a/include/uapi/linux/nitro_enclaves.h b/include/uapi/linux/nitro_enclaves.h
>> new file mode 100644
>> index 000000000000..3413352baf32
>> --- /dev/null
>> +++ b/include/uapi/linux/nitro_enclaves.h
>> @@ -0,0 +1,65 @@
>> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
>> +/*
>> + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
>> + */
>> +
>> +#ifndef _UAPI_LINUX_NITRO_ENCLAVES_H_
>> +#define _UAPI_LINUX_NITRO_ENCLAVES_H_
>> +
>> +#include <linux/kvm.h>
>> +#include <linux/types.h>
>> +
>> +/* Nitro Enclaves (NE) Kernel Driver Interface */
>> +
>> +/**
>> + * The command is used to get information needed for in-memory enclave image
>> + * loading e.g. offset in enclave memory to start placing the enclave image.
>> + *
>> + * The image load metadata is an in / out data structure. It includes info
>> + * provided by the caller - flags - and returns the offset in enclave memory
>> + * where to start placing the enclave image.
>> + */
>> +#define NE_GET_IMAGE_LOAD_METADATA _IOWR(0xAE, 0x20, struct image_load_metadata)
>> +
>> +/**
>> + * The command is used to trigger enclave start after the enclave resources,
>> + * such as memory and CPU, have been set.
>> + *
>> + * The enclave start metadata is an in / out data structure. It includes info
>> + * provided by the caller - enclave cid and flags - and returns the slot uid
>> + * and the cid (if input cid is 0).
>> + */
>> +#define NE_START_ENCLAVE _IOWR(0xAE, 0x21, struct enclave_start_metadata)
>> +
> image_load_metadata->flags and enclave_start_metadata->flags constants
> are missing.
I added them now in this file. Thank you.
>
>> +/* Metadata necessary for in-memory enclave image loading. */
>> +struct image_load_metadata {
>> + /**
>> + * Flags to determine the enclave image type e.g. Enclave Image Format
>> + * (EIF) (in).
>> + */
>> + __u64 flags;
>> +
>> + /**
>> + * Offset in enclave memory where to start placing the enclave image
>> + * (out).
>> + */
>> + __u64 memory_offset;
>> +};
> What about feature bits or a API version number field? If you add
> features to the NE driver, how will userspace detect them?
>
> Even if you intend to always compile userspace against the exact kernel
> headers that the program will run on, it can still be useful to have an
> API version for informational purposes and to easily prevent user
> errors (running a new userspace binary on an old kernel where the API is
> different).
Good point. I'll add a get API version ioctl for this purpose.
>
> Finally, reserved struct fields may come in handy in the future. That
> way userspace and the kernel don't need to explicitly handle multiple
> struct sizes.
True, I've seen this pattern of including reserved fields e.g. in a
couple of KVM data structures.
Thanks for feedback, Stefan.
Andra
Amazon Development Center (Romania) S.R.L. registered office: 27A Sf. Lazar Street, UBC5, floor 2, Iasi, Iasi County, 700045, Romania. Registered in Romania. Registration number J22/2621/2005.
Powered by blists - more mailing lists