[RFC 00/12] Multi-Key Total Memory Encryption API (MKTME)

[RFC 00/12] Multi-Key Total Memory Encryption API (MKTME)

[Alison Schofield]

Seeking comments on the APIs supporting MKTME on future Intel platforms.

MKTME (Multi-Key Total Memory Encryption) is a technology supporting
memory encryption on upcoming Intel platforms. Whereas TME allows
encryption of the entire system memory using a single key, MKTME
allows mulitple encryption domains, each having their own key. While 
the main use case for the feature is virtual machine isolation, the
API needs the flexibility to work for a wide range of use cases.

This RFC presents the 2 API additions that enable userspace to:
 1) Create Encryption Keys: Kernel Key Service type "mktme"
 2) Use the Encryption Keys: system call encrypt_mprotect()

In order to share between: the Kernel Key Service, the new system call,
and the existing mm code, helper functions were created in arch/x86/mktme

This patchset is built upon Kirill Shutemov's patchset for the core MKTME
support. You can find that here:
git://git.kernel.org/pub/scm/linux/kernel/git/kas/linux.git mktme/wip


Alison Schofield (12):
  docs/x86: Document the Multi-Key Total Memory Encryption API
  mm: Generalize the mprotect implementation to support extensions
  syscall/x86: Wire up a new system call for memory encryption keys
  x86/mm: Add helper functions to manage memory encryption keys
  x86/mm: Add a helper function to set keyid bits in encrypted VMA's
  mm: Add the encrypt_mprotect() system call
  x86/mm: Add helper functions to track encrypted VMA's
  mm: Track VMA's in use for each memory encryption keyid
  mm: Restrict memory encryption to anonymous VMA's
  x86/pconfig: Program memory encryption keys on a system-wide basis
  keys/mktme: Add a new key service type for memory encryption keys
  keys/mktme: Do not revoke in use memory encryption keys

 Documentation/x86/mktme-keys.txt       | 153 ++++++++++++++++
 arch/x86/Kconfig                       |   1 +
 arch/x86/entry/syscalls/syscall_32.tbl |   1 +
 arch/x86/entry/syscalls/syscall_64.tbl |   1 +
 arch/x86/include/asm/intel_pconfig.h   |  42 ++++-
 arch/x86/include/asm/mktme.h           |  21 +++
 arch/x86/mm/mktme.c                    | 141 ++++++++++++++
 fs/exec.c                              |   4 +-
 include/keys/mktme-type.h              |  28 +++
 include/linux/key.h                    |   2 +
 include/linux/mm.h                     |   9 +-
 include/linux/syscalls.h               |   2 +
 include/uapi/asm-generic/unistd.h      |   4 +-
 kernel/fork.c                          |   2 +
 kernel/sys_ni.c                        |   2 +
 mm/mmap.c                              |  12 ++
 mm/mprotect.c                          |  93 +++++++++-
 mm/nommu.c                             |   4 +
 security/keys/Kconfig                  |  11 ++
 security/keys/Makefile                 |   1 +
 security/keys/internal.h               |   6 +
 security/keys/keyctl.c                 |   7 +
 security/keys/mktme_keys.c             | 325 +++++++++++++++++++++++++++++++++
 23 files changed, 855 insertions(+), 17 deletions(-)
 create mode 100644 Documentation/x86/mktme-keys.txt
 create mode 100644 include/keys/mktme-type.h
 create mode 100644 security/keys/mktme_keys.c

-- 
2.14.1

[RFC 01/12] docs/x86: Document the Multi-Key Total Memory Encryption API

[Alison Schofield]

Document the API's used for MKTME on Intel platforms.
MKTME: Multi-KEY Total Memory Encryption

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 Documentation/x86/mktme-keys.txt | 153 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 153 insertions(+)
 create mode 100644 Documentation/x86/mktme-keys.txt

diff --git a/Documentation/x86/mktme-keys.txt b/Documentation/x86/mktme-keys.txt
new file mode 100644
index 000000000000..2dea7acd2a17
--- /dev/null
+++ b/Documentation/x86/mktme-keys.txt
@@ -0,0 +1,153 @@
+MKTME (Multi-Key Total Memory Encryption) is a technology that allows
+memory encryption on Intel platforms. Whereas TME (Total Memory Encryption)
+allows encryption of the entire system memory using a single key, MKTME
+allows multiple encryption domains, each having their own key. The main use
+case for the feature is virtual machine isolation. The API's introduced here
+are intended to offer flexibility to work in a wide range of uses.
+
+The externally available Intel Architecture Spec:
+https://software.intel.com/sites/default/files/managed/a5/16/Multi-Key-Total-Memory-Encryption-Spec.pdf
+
+============================  API Overview  ============================
+
+There are 2 MKTME specific API's that enable userspace to create and use
+the memory encryption keys:
+
+1) Kernel Key Service: MKTME Type
+
+   MKTME is a new key type added to the existing Kernel Key Services
+   to support the memory encryption keys. The MKTME service manages
+   the addition and removal of MKTME keys. It maps userspace keys
+   to hardware keyids and programs the hardware with user requested
+   encryption parameters.
+
+   o An understanding of the Kernel Key Service is required in order
+     to use the MKTME key type as it is a subset of that service.
+
+   o MKTME keys are a limited resource. There is a single pool of
+     MKTME keys for a system and that pool can be from 3 to 63 keys.
+     With that in mind, userspace may take advantage of the kernel
+     key services sharing and permissions model for userspace keys.
+     One key can be shared as long as each user has the permission
+     of "KEY_NEED_VIEW" to use it.
+
+   o MKTME key type uses capabilities to restrict the allocation
+     of keys. It only requires CAP_SYS_RESOURCE, but will accept
+     the broader capability of CAP_SYS_ADMIN.  See capabilities(7).
+
+   o The MKTME key service blocks kernel key service commands that
+     could lead to reprogramming of in use keys, or loss of keys from
+     the pool. This means MKTME does not allow a key to be invalidated,
+     unlinked, or timed out. These operations are blocked by MKTME as
+     it creates all keys with the internal flag KEY_FLAG_KEEP.
+
+   o MKTME does not support the keyctl option of UPDATE. Userspace
+     may change the programming of a key by revoking it and adding
+     a new key with the updated encryption options (or vice-versa).
+
+2) System Call: encrypt_mprotect()
+
+   MKTME encryption is requested by calling encrypt_mprotect(). The
+   caller passes the serial number to a previously allocated and
+   programmed encryption key. That handle was created with the MKTME
+   Key Service.
+
+   o The caller must have KEY_NEED_VIEW permission on the key
+
+   o The range of memory that is to be protected must be mapped as
+     ANONYMOUS. If it is not, the entire encrypt_mprotect() request
+     fails with EINVAL.
+
+   o As an extension to the existing mprotect() system call,
+     encrypt_mprotect() supports the legacy mprotect behavior plus
+     the enabling of memory encryption. That means that in addition
+     to encrypting the memory, the protection flags will be updated
+     as requested in the call.
+
+   o Additional mprotect() calls to memory already protected with
+     MKTME will not alter the MKTME status.
+
+======================  Usage: MKTME Key Service  ======================
+
+MKTME is enabled on supported Intel platforms by selecting
+CONFIG_X86_INTEL_MKTME which selects CONFIG_MKTME_KEYS.
+
+Allocating MKTME Keys via command line or system call:
+    keyctl add mktme name "[options]" ring
+
+    key_serial_t add_key(const char *type, const char *description,
+                         const void *payload, size_t plen,
+                         key_serial_t keyring);
+
+Revoking MKTME Keys via command line or system call::
+   keyctl revoke <key>
+
+   long keyctl(KEYCTL_REVOKE, key_serial_t key);
+
+Options Field Definition:
+    userkey=      ASCII HEX value encryption key. Defaults to a CPU
+		  generated key if a userkey is not defined here.
+
+    algorithm=    Encryption algorithm name as a string.
+		  Valid algorithm: "aes-xts-128"
+
+    tweak=        ASCII HEX value tweak key. Tweak key will be added to the
+                  userkey...  (need to be clear here that this is being sent
+                  to the hardware - kernel not messing w it)
+
+    entropy=      ascii hex value entropy.
+                  This entropy will be used to generated the CPU key and
+		  the tweak key when CPU generated key is requested.
+
+Algorithm Dependencies:
+    AES-XTS 128 is the only supported algorithm.
+    There are only 2 ways that AES-XTS 128 may be used:
+
+    1) User specified encryption key
+	- The user specified encryption key must be exactly
+	  16 ASCII Hex bytes (128 bits).
+	- A tweak key must be specified and it must be exactly
+	  16 ASCII Hex bytes (128 bits).
+	- No entropy field is accepted.
+
+    2) CPU generated encryption key
+	- When no user specified encryption key is provided, the
+	  default encryption key will be CPU generated.
+	- User must specify 16 ASCII Hex bytes of entropy. This 
+	  entropy will be used by the CPU to generate both the
+	  encryption key and the tweak key.
+	- No entropy field is accepted.
+
+======================  Usage: encrypt_mprotect()  ======================
+
+System Call encrypt_mprotect()::
+
+    This system call is an extension of the existing mprotect() system
+    call. It requires the same parameters as legary mprotect() plus
+    one additional parameter, the keyid. Userspace must provide the
+    key serial number assigned through the kernel key service.
+
+    int encrypt_mprotect(void *addr, size_t len, int prot, int keyid);
+
+======================  Usage: Sample Roundtrip  ======================
+
+Sample usage of MKTME Key Service API with encrypt_mprotect() API:
+
+  Add a key:
+        key = add_key(mktme, name, options, strlen(option), keyring);
+
+  Map memory:
+        ptr = mmap(NULL, size, prot, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
+
+  Protect memory:
+        ret = syscall(sys_encrypt_mprotect, ptr, size, prot, keyid);
+
+  Use protected memory:
+        ................
+
+  Free memory:
+        ret = munmap(ptr, size);
+
+  Revoke key:
+        ret = keyctl(KEYCTL_REVOKE, key);
+
-- 
2.14.1

[RFC 02/12] mm: Generalize the mprotect implementation to support extensions

[Alison Schofield]

Today mprotect is implemented to support legacy mprotect behavior
plus an extension for memory protection keys. Make it more generic
so that it can support additional extensions in the future.

This is done is preparation for adding a new system call for memory
encyption keys. The intent is that the new encrypted mprotect will be
another extension to legacy mprotect.

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 mm/mprotect.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/mm/mprotect.c b/mm/mprotect.c
index 68dc476310c0..56e64ef7931e 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -35,6 +35,8 @@
 
 #include "internal.h"
 
+#define NO_PKEY  -1
+
 static unsigned long change_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
 		unsigned long addr, unsigned long end, pgprot_t newprot,
 		int dirty_accountable, int prot_numa)
@@ -402,9 +404,9 @@ mprotect_fixup(struct vm_area_struct *vma, struct vm_area_struct **pprev,
 }
 
 /*
- * pkey==-1 when doing a legacy mprotect()
+ * When pkey==NO_PKEY we get legacy mprotect behavior here.
  */
-static int do_mprotect_pkey(unsigned long start, size_t len,
+static int do_mprotect_ext(unsigned long start, size_t len,
 		unsigned long prot, int pkey)
 {
 	unsigned long nstart, end, tmp, reqprot;
@@ -528,7 +530,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len,
 SYSCALL_DEFINE3(mprotect, unsigned long, start, size_t, len,
 		unsigned long, prot)
 {
-	return do_mprotect_pkey(start, len, prot, -1);
+	return do_mprotect_ext(start, len, prot, NO_PKEY);
 }
 
 #ifdef CONFIG_ARCH_HAS_PKEYS
@@ -536,7 +538,7 @@ SYSCALL_DEFINE3(mprotect, unsigned long, start, size_t, len,
 SYSCALL_DEFINE4(pkey_mprotect, unsigned long, start, size_t, len,
 		unsigned long, prot, int, pkey)
 {
-	return do_mprotect_pkey(start, len, prot, pkey);
+	return do_mprotect_ext(start, len, prot, pkey);
 }
 
 SYSCALL_DEFINE2(pkey_alloc, unsigned long, flags, unsigned long, init_val)
-- 
2.14.1

[RFC 03/12] syscall/x86: Wire up a new system call for memory encryption keys

[Alison Schofield]

encrypt_mprotect() is a new system call to support memory encryption.

It takes the same parameters as legacy mprotect, plus an additional
key serial number that is mapped to an encryption keyid.

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 arch/x86/entry/syscalls/syscall_32.tbl | 1 +
 arch/x86/entry/syscalls/syscall_64.tbl | 1 +
 include/linux/syscalls.h               | 2 ++
 include/uapi/asm-generic/unistd.h      | 4 +++-
 kernel/sys_ni.c                        | 2 ++
 5 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index 3cf7b533b3d1..f41ad857d5c6 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -398,3 +398,4 @@
 384	i386	arch_prctl		sys_arch_prctl			__ia32_compat_sys_arch_prctl
 385	i386	io_pgetevents		sys_io_pgetevents		__ia32_compat_sys_io_pgetevents
 386	i386	rseq			sys_rseq			__ia32_sys_rseq
+387	i386	encrypt_mprotect	sys_encrypt_mprotect		__ia32_sys_encrypt_mprotect
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index f0b1709a5ffb..cf2decfa6119 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -343,6 +343,7 @@
 332	common	statx			__x64_sys_statx
 333	common	io_pgetevents		__x64_sys_io_pgetevents
 334	common	rseq			__x64_sys_rseq
+335	common	encrypt_mprotect	__x64_sys_encrypt_mprotect
 
 #
 # x32-specific system call numbers start at 512 to avoid cache impact
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 3ed377d0c46c..7dc0ed3a182e 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -904,6 +904,8 @@ asmlinkage long sys_statx(int dfd, const char __user *path, unsigned flags,
 			  unsigned mask, struct statx __user *buffer);
 asmlinkage long sys_rseq(struct rseq __user *rseq, uint32_t rseq_len,
 			 int flags, uint32_t sig);
+asmlinkage long sys_encrypt_mprotect(unsigned long start, size_t len,
+				     unsigned long prot, int serial);
 
 /*
  * Architecture-specific system calls
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 42990676a55e..d2cb0af68160 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -734,9 +734,11 @@ __SYSCALL(__NR_pkey_free,     sys_pkey_free)
 __SYSCALL(__NR_statx,     sys_statx)
 #define __NR_io_pgetevents 292
 __SC_COMP(__NR_io_pgetevents, sys_io_pgetevents, compat_sys_io_pgetevents)
+#define __NR_encrypt_mprotect 293
+__SYSCALL(__NR_encrypt_mprotect, sys_encrypt_mprotect)
 
 #undef __NR_syscalls
-#define __NR_syscalls 293
+#define __NR_syscalls 294
 
 /*
  * 32 bit systems traditionally used different
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index df556175be50..1b48f709c265 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -336,6 +336,8 @@ COND_SYSCALL(pkey_mprotect);
 COND_SYSCALL(pkey_alloc);
 COND_SYSCALL(pkey_free);
 
+/* multi-key total memory encryption keys */
+COND_SYSCALL(encrypt_mprotect);
 
 /*
  * Architecture specific weak syscall entries.
-- 
2.14.1

[RFC 04/12] x86/mm: Add helper functions to manage memory encryption keys

[Alison Schofield]

Define a global mapping structure to track the mapping of userspace
keys to hardware keyids in MKTME (Multi-Key Total Memory Encryption).
This data will be used for the memory encryption system call and the
kernel key service API.

Implement helper functions to access this mapping structure and make
them visible to the MKTME Kernel Key Service: security/keys/mktme_keys

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 arch/x86/include/asm/mktme.h | 11 ++++++
 arch/x86/mm/mktme.c          | 85 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 96 insertions(+)

diff --git a/arch/x86/include/asm/mktme.h b/arch/x86/include/asm/mktme.h
index dbfbd955da98..f6acd551457f 100644
--- a/arch/x86/include/asm/mktme.h
+++ b/arch/x86/include/asm/mktme.h
@@ -13,6 +13,17 @@ extern phys_addr_t mktme_keyid_mask;
 extern int mktme_nr_keyids;
 extern int mktme_keyid_shift;
 
+/* Manage mappings between hardware keyids and userspace keys */
+extern int mktme_map_alloc(void);
+extern void mktme_map_free(void);
+extern void mktme_map_lock(void);
+extern void mktme_map_unlock(void);
+extern int mktme_map_get_free_keyid(void);
+extern void mktme_map_clear_keyid(int keyid);
+extern void mktme_map_set_keyid(int keyid, unsigned int serial);
+extern int mktme_map_keyid_from_serial(unsigned int serial);
+extern unsigned int mktme_map_serial_from_keyid(int keyid);
+
 extern struct page_ext_operations page_mktme_ops;
 
 #define page_keyid page_keyid
diff --git a/arch/x86/mm/mktme.c b/arch/x86/mm/mktme.c
index 660caf6a5ce1..5246d8323359 100644
--- a/arch/x86/mm/mktme.c
+++ b/arch/x86/mm/mktme.c
@@ -63,6 +63,91 @@ int vma_keyid(struct vm_area_struct *vma)
 	return (prot & mktme_keyid_mask) >> mktme_keyid_shift;
 }
 
+/*
+ * struct mktme_mapping and the mktme_map_* functions manage the mapping
+ * of userspace keys to hardware keyids in MKTME. They are used by the
+ * the encrypt_mprotect system call and the MKTME Key Service API.
+ */
+struct mktme_mapping {
+	struct mutex	lock;		/* protect this map & HW state */
+	unsigned int	mapped_keyids;
+	unsigned int	serial[];
+};
+
+struct mktme_mapping *mktme_map;
+
+static inline long mktme_map_size(void)
+{
+	long size = 0;
+
+	size += sizeof(mktme_map);
+	size += sizeof(mktme_map->serial[0]) * mktme_nr_keyids;
+	return size;
+}
+
+int mktme_map_alloc(void)
+{
+	mktme_map = kzalloc(mktme_map_size(), GFP_KERNEL);
+	if (!mktme_map)
+		return 0;
+	mutex_init(&mktme_map->lock);
+	return 1;
+}
+
+void mktme_map_free(void)
+{
+	kfree(mktme_map);
+}
+
+void mktme_map_lock(void)
+{
+	mutex_lock(&mktme_map->lock);
+}
+
+void mktme_map_unlock(void)
+{
+	mutex_unlock(&mktme_map->lock);
+}
+
+void mktme_map_set_keyid(int keyid, unsigned int serial)
+{
+	mktme_map->serial[keyid] = serial;
+	mktme_map->mapped_keyids++;
+}
+
+void mktme_map_clear_keyid(int keyid)
+{
+	mktme_map->serial[keyid] = 0;
+	mktme_map->mapped_keyids--;
+}
+
+unsigned int mktme_map_serial_from_keyid(int keyid)
+{
+	return mktme_map->serial[keyid];
+}
+
+int mktme_map_keyid_from_serial(unsigned int serial)
+{
+	int i;
+
+	for (i = 1; i < mktme_nr_keyids; i++)
+		if (mktme_map->serial[i] == serial)
+			return i;
+	return 0;
+}
+
+int mktme_map_get_free_keyid(void)
+{
+	int i;
+
+	if (mktme_map->mapped_keyids < mktme_nr_keyids) {
+		for (i = 1; i < mktme_nr_keyids; i++)
+			if (mktme_map->serial[i] == 0)
+				return i;
+	}
+	return 0;
+}
+
 void prep_encrypted_page(struct page *page, int order, int keyid, bool zero)
 {
 	int i;
-- 
2.14.1

[RFC 05/12] x86/mm: Add a helper function to set keyid bits in encrypted VMA's

[Alison Schofield]

Store the memory encryption keyid in the upper bits of vm_page_prot
that match position of keyid, bits 51:46, in a PTE.

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 arch/x86/include/asm/mktme.h |  3 +++
 arch/x86/mm/mktme.c          | 15 +++++++++++++++
 include/linux/mm.h           |  4 ++++
 3 files changed, 22 insertions(+)

diff --git a/arch/x86/include/asm/mktme.h b/arch/x86/include/asm/mktme.h
index f6acd551457f..b707f800b68f 100644
--- a/arch/x86/include/asm/mktme.h
+++ b/arch/x86/include/asm/mktme.h
@@ -13,6 +13,9 @@ extern phys_addr_t mktme_keyid_mask;
 extern int mktme_nr_keyids;
 extern int mktme_keyid_shift;
 
+/* Set the encryption keyid bits in a VMA */
+extern void mprotect_set_encrypt(struct vm_area_struct *vma, int newkeyid);
+
 /* Manage mappings between hardware keyids and userspace keys */
 extern int mktme_map_alloc(void);
 extern void mktme_map_free(void);
diff --git a/arch/x86/mm/mktme.c b/arch/x86/mm/mktme.c
index 5246d8323359..5ee7f37e9cd0 100644
--- a/arch/x86/mm/mktme.c
+++ b/arch/x86/mm/mktme.c
@@ -63,6 +63,21 @@ int vma_keyid(struct vm_area_struct *vma)
 	return (prot & mktme_keyid_mask) >> mktme_keyid_shift;
 }
 
+/* Set the encryption keyid bits in a VMA */
+void mprotect_set_encrypt(struct vm_area_struct *vma, int newkeyid)
+{
+	int oldkeyid = vma_keyid(vma);
+	pgprotval_t newprot;
+
+	if (newkeyid == oldkeyid)
+		return;
+
+	newprot = pgprot_val(vma->vm_page_prot);
+	newprot &= ~mktme_keyid_mask;
+	newprot |= (unsigned long)newkeyid << mktme_keyid_shift;
+	vma->vm_page_prot = __pgprot(newprot);
+}
+
 /*
  * struct mktme_mapping and the mktme_map_* functions manage the mapping
  * of userspace keys to hardware keyids in MKTME. They are used by the
diff --git a/include/linux/mm.h b/include/linux/mm.h
index a4ce26aa0b65..ac85c0805761 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2799,5 +2799,9 @@ void __init setup_nr_node_ids(void);
 static inline void setup_nr_node_ids(void) {}
 #endif
 
+#ifndef CONFIG_X86_INTEL_MKTME
+static inline void mprotect_set_encrypt(struct vm_area_struct *vma,
+					int newkeyid) {}
+#endif /* CONFIG_X86_INTEL_MKTME */
 #endif /* __KERNEL__ */
 #endif /* _LINUX_MM_H */
-- 
2.14.1

[RFC 06/12] mm: Add the encrypt_mprotect() system call

[Alison Schofield]

Implement memory encryption with a new system call that is an
extension of the legacy mprotect() system call.

In encrypt_mprotect the caller must pass a handle to a previously
allocated and programmed encryption key. Validate the key and store
the keyid bits in the vm_page_prot for each VMA in the protection
range.

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 fs/exec.c           |  4 ++--
 include/linux/key.h |  2 ++
 include/linux/mm.h  |  3 ++-
 mm/mprotect.c       | 67 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 4 files changed, 67 insertions(+), 9 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index a1a246062561..b681a413db9c 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -754,8 +754,8 @@ int setup_arg_pages(struct linux_binprm *bprm,
 	vm_flags |= mm->def_flags;
 	vm_flags |= VM_STACK_INCOMPLETE_SETUP;
 
-	ret = mprotect_fixup(vma, &prev, vma->vm_start, vma->vm_end,
-			vm_flags);
+	ret = mprotect_fixup(vma, &prev, vma->vm_start, vma->vm_end, vm_flags,
+			     -1);
 	if (ret)
 		goto out_unlock;
 	BUG_ON(prev != vma);
diff --git a/include/linux/key.h b/include/linux/key.h
index e58ee10f6e58..fb8a7d5f6149 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -346,6 +346,8 @@ static inline key_serial_t key_serial(const struct key *key)
 
 extern void key_set_timeout(struct key *, unsigned);
 
+extern key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
+				 key_perm_t perm);
 /*
  * The permissions required on a key that we're looking up.
  */
diff --git a/include/linux/mm.h b/include/linux/mm.h
index ac85c0805761..0f9422c7841e 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1579,7 +1579,8 @@ extern unsigned long change_protection(struct vm_area_struct *vma, unsigned long
 			      int dirty_accountable, int prot_numa);
 extern int mprotect_fixup(struct vm_area_struct *vma,
 			  struct vm_area_struct **pprev, unsigned long start,
-			  unsigned long end, unsigned long newflags);
+			  unsigned long end, unsigned long newflags,
+			  int newkeyid);
 
 /*
  * doesn't attempt to fault and will return short.
diff --git a/mm/mprotect.c b/mm/mprotect.c
index 56e64ef7931e..6c2e1106525c 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -28,14 +28,17 @@
 #include <linux/ksm.h>
 #include <linux/uaccess.h>
 #include <linux/mm_inline.h>
+#include <linux/key.h>
 #include <asm/pgtable.h>
 #include <asm/cacheflush.h>
 #include <asm/mmu_context.h>
 #include <asm/tlbflush.h>
+#include <asm/mktme.h>
 
 #include "internal.h"
 
 #define NO_PKEY  -1
+#define NO_KEYID -1
 
 static unsigned long change_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
 		unsigned long addr, unsigned long end, pgprot_t newprot,
@@ -310,7 +313,8 @@ unsigned long change_protection(struct vm_area_struct *vma, unsigned long start,
 
 int
 mprotect_fixup(struct vm_area_struct *vma, struct vm_area_struct **pprev,
-	unsigned long start, unsigned long end, unsigned long newflags)
+	       unsigned long start, unsigned long end, unsigned long newflags,
+	       int newkeyid)
 {
 	struct mm_struct *mm = vma->vm_mm;
 	unsigned long oldflags = vma->vm_flags;
@@ -320,10 +324,24 @@ mprotect_fixup(struct vm_area_struct *vma, struct vm_area_struct **pprev,
 	int error;
 	int dirty_accountable = 0;
 
+	/*
+	 * Flags match and Keyids match or we have NO_KEYID.
+	 * This _fixup is usually called from do_mprotect_ext() except
+	 * for one special case: caller fs/exec.c/setup_arg_pages()
+	 * In that case, newkeyid is passed as -1 (NO_KEYID).
+	 */
+	if (newflags == oldflags &&
+	    (newkeyid == vma_keyid(vma) || newkeyid == NO_KEYID)) {
+		*pprev = vma;
+		return 0;
+	}
+	/* Flags match and Keyid changes */
 	if (newflags == oldflags) {
+		mprotect_set_encrypt(vma, newkeyid);
 		*pprev = vma;
 		return 0;
 	}
+	/* Flags and Keyids both change, continue. */
 
 	/*
 	 * If we make a private mapping writable we increase our commit;
@@ -373,6 +391,8 @@ mprotect_fixup(struct vm_area_struct *vma, struct vm_area_struct **pprev,
 	}
 
 success:
+	if (newkeyid != NO_KEYID)
+		mprotect_set_encrypt(vma, newkeyid);
 	/*
 	 * vm_flags and vm_page_prot are protected by the mmap_sem
 	 * held in write mode.
@@ -404,10 +424,15 @@ mprotect_fixup(struct vm_area_struct *vma, struct vm_area_struct **pprev,
 }
 
 /*
- * When pkey==NO_PKEY we get legacy mprotect behavior here.
+ * do_mprotect_ext() supports the legacy mprotect behavior plus extensions
+ * for protection keys and memory encryption keys. These extensions are
+ * mutually exclusive and the behavior is:
+ *	(pkey==NO_PKEY && keyid==NO_KEYID) ==> legacy mprotect
+ *	(pkey is valid)  ==> legacy mprotect plus protection key extensions
+ *	(keyid is valid) ==> legacy mprotect plus encryption key extensions
  */
 static int do_mprotect_ext(unsigned long start, size_t len,
-		unsigned long prot, int pkey)
+			   unsigned long prot, int pkey, int keyid)
 {
 	unsigned long nstart, end, tmp, reqprot;
 	struct vm_area_struct *vma, *prev;
@@ -505,7 +530,8 @@ static int do_mprotect_ext(unsigned long start, size_t len,
 		tmp = vma->vm_end;
 		if (tmp > end)
 			tmp = end;
-		error = mprotect_fixup(vma, &prev, nstart, tmp, newflags);
+		error = mprotect_fixup(vma, &prev, nstart, tmp, newflags,
+				       keyid);
 		if (error)
 			goto out;
 		nstart = tmp;
@@ -530,7 +556,7 @@ static int do_mprotect_ext(unsigned long start, size_t len,
 SYSCALL_DEFINE3(mprotect, unsigned long, start, size_t, len,
 		unsigned long, prot)
 {
-	return do_mprotect_ext(start, len, prot, NO_PKEY);
+	return do_mprotect_ext(start, len, prot, NO_PKEY, NO_KEYID);
 }
 
 #ifdef CONFIG_ARCH_HAS_PKEYS
@@ -538,7 +564,7 @@ SYSCALL_DEFINE3(mprotect, unsigned long, start, size_t, len,
 SYSCALL_DEFINE4(pkey_mprotect, unsigned long, start, size_t, len,
 		unsigned long, prot, int, pkey)
 {
-	return do_mprotect_ext(start, len, prot, pkey);
+	return do_mprotect_ext(start, len, prot, pkey, NO_KEYID);
 }
 
 SYSCALL_DEFINE2(pkey_alloc, unsigned long, flags, unsigned long, init_val)
@@ -587,3 +613,32 @@ SYSCALL_DEFINE1(pkey_free, int, pkey)
 }
 
 #endif /* CONFIG_ARCH_HAS_PKEYS */
+
+#ifdef CONFIG_X86_INTEL_MKTME
+
+SYSCALL_DEFINE4(encrypt_mprotect, unsigned long, start, size_t, len,
+		unsigned long, prot, key_serial_t, serial)
+{
+	key_ref_t key_ref;
+	int ret, keyid;
+
+	/* TODO MKTME key service must be initialized */
+
+	key_ref = lookup_user_key(serial, 0, KEY_NEED_VIEW);
+	if (IS_ERR(key_ref))
+		return PTR_ERR(key_ref);
+
+	mktme_map_lock();
+	keyid = mktme_map_keyid_from_serial(serial);
+	if (!keyid) {
+		mktme_map_unlock();
+		key_ref_put(key_ref);
+		return -EINVAL;
+	}
+	ret = do_mprotect_ext(start, len, prot, NO_PKEY, keyid);
+	mktme_map_unlock();
+	key_ref_put(key_ref);
+	return ret;
+}
+
+#endif /* CONFIG_X86_INTEL_MKTME */
-- 
2.14.1

[RFC 07/12] x86/mm: Add helper functions to track encrypted VMA's

[Alison Schofield]

In order to safely manage the usage of memory encryption keys, VMA's
using each keyid need to be tracked. This tracking allows the Kernel
Key Service to know when the keyid resource is actually in use, or
when it is idle and may be considered for reuse.

Define a global atomic encrypt_count array to track the number of VMA's
oustanding for each encryption keyid.

Implement helper functions to manipulate this encrypt_count array.

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 arch/x86/include/asm/mktme.h |  7 +++++++
 arch/x86/mm/mktme.c          | 39 +++++++++++++++++++++++++++++++++++++++
 include/linux/mm.h           |  2 ++
 3 files changed, 48 insertions(+)

diff --git a/arch/x86/include/asm/mktme.h b/arch/x86/include/asm/mktme.h
index b707f800b68f..5f3fa0c39c1c 100644
--- a/arch/x86/include/asm/mktme.h
+++ b/arch/x86/include/asm/mktme.h
@@ -16,6 +16,13 @@ extern int mktme_keyid_shift;
 /* Set the encryption keyid bits in a VMA */
 extern void mprotect_set_encrypt(struct vm_area_struct *vma, int newkeyid);
 
+/* Manage the references to outstanding VMA's per encryption key */
+extern int vma_alloc_encrypt_array(void);
+extern void vma_free_encrypt_array(void);
+extern int vma_read_encrypt_ref(int keyid);
+extern void vma_get_encrypt_ref(struct vm_area_struct *vma);
+extern void vma_put_encrypt_ref(struct vm_area_struct *vma);
+
 /* Manage mappings between hardware keyids and userspace keys */
 extern int mktme_map_alloc(void);
 extern void mktme_map_free(void);
diff --git a/arch/x86/mm/mktme.c b/arch/x86/mm/mktme.c
index 5ee7f37e9cd0..5690ef51a79a 100644
--- a/arch/x86/mm/mktme.c
+++ b/arch/x86/mm/mktme.c
@@ -163,6 +163,45 @@ int mktme_map_get_free_keyid(void)
 	return 0;
 }
 
+/*
+ *  Helper functions manage the encrypt_count[] array that tracks the
+ *  VMA's outstanding for each encryption keyid. The gets & puts are
+ *  used in core mm code that allocates and free's VMA's. The alloc,
+ *  free, and read functions are used by the MKTME key service to
+ *  manage key allocation and programming.
+ */
+atomic_t *encrypt_count;
+
+int vma_alloc_encrypt_array(void)
+{
+	encrypt_count = kcalloc(mktme_nr_keyids, sizeof(atomic_t), GFP_KERNEL);
+	if (!encrypt_count)
+		return -ENOMEM;
+	return 0;
+}
+
+void vma_free_encrypt_array(void)
+{
+	kfree(encrypt_count);
+}
+
+int vma_read_encrypt_ref(int keyid)
+{
+	return atomic_read(&encrypt_count[keyid]);
+}
+
+void vma_get_encrypt_ref(struct vm_area_struct *vma)
+{
+	if (vma_keyid(vma))
+		atomic_inc(&encrypt_count[vma_keyid(vma)]);
+}
+
+void vma_put_encrypt_ref(struct vm_area_struct *vma)
+{
+	if (vma_keyid(vma))
+		atomic_dec(&encrypt_count[vma_keyid(vma)]);
+}
+
 void prep_encrypted_page(struct page *page, int order, int keyid, bool zero)
 {
 	int i;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 0f9422c7841e..b217c699dbab 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2803,6 +2803,8 @@ static inline void setup_nr_node_ids(void) {}
 #ifndef CONFIG_X86_INTEL_MKTME
 static inline void mprotect_set_encrypt(struct vm_area_struct *vma,
 					int newkeyid) {}
+static inline void vma_get_encrypt_ref(struct vm_area_struct *vma) {}
+static inline void vma_put_encrypt_ref(struct vm_area_struct *vma) {}
 #endif /* CONFIG_X86_INTEL_MKTME */
 #endif /* __KERNEL__ */
 #endif /* _LINUX_MM_H */
-- 
2.14.1

[RFC 08/12] mm: Track VMA's in use for each memory encryption keyid

[Alison Schofield]

Keep track of the VMA's oustanding for each memory encryption keyid.
The count is used by the MKTME (Multi-Key Total Memory Encryption)
Key Service to determine when it is safe to reprogram a hardware
encryption key.

Approach here is to do gets and puts on the encryption reference
wherever kmem_cache_alloc/free's of vma_area_cachep's are executed.
A couple of these locations will not be hit until cgroup support is
added. One of these locations should never hit, so use a VM_WARN_ON.

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 arch/x86/mm/mktme.c |  2 ++
 kernel/fork.c       |  2 ++
 mm/mmap.c           | 12 ++++++++++++
 mm/nommu.c          |  4 ++++
 4 files changed, 20 insertions(+)

diff --git a/arch/x86/mm/mktme.c b/arch/x86/mm/mktme.c
index 5690ef51a79a..8a7c326d4546 100644
--- a/arch/x86/mm/mktme.c
+++ b/arch/x86/mm/mktme.c
@@ -72,10 +72,12 @@ void mprotect_set_encrypt(struct vm_area_struct *vma, int newkeyid)
 	if (newkeyid == oldkeyid)
 		return;
 
+	vma_put_encrypt_ref(vma);
 	newprot = pgprot_val(vma->vm_page_prot);
 	newprot &= ~mktme_keyid_mask;
 	newprot |= (unsigned long)newkeyid << mktme_keyid_shift;
 	vma->vm_page_prot = __pgprot(newprot);
+	vma_get_encrypt_ref(vma);
 }
 
 /*
diff --git a/kernel/fork.c b/kernel/fork.c
index e5e7a220a124..2d0e507bde7c 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -459,6 +459,7 @@ static __latent_entropy int dup_mmap(struct mm_struct *mm,
 		if (!tmp)
 			goto fail_nomem;
 		*tmp = *mpnt;
+		vma_get_encrypt_ref(tmp);	/* Track encrypted vma's */
 		INIT_LIST_HEAD(&tmp->anon_vma_chain);
 		retval = vma_dup_policy(mpnt, tmp);
 		if (retval)
@@ -539,6 +540,7 @@ static __latent_entropy int dup_mmap(struct mm_struct *mm,
 fail_nomem_anon_vma_fork:
 	mpol_put(vma_policy(tmp));
 fail_nomem_policy:
+	vma_put_encrypt_ref(tmp);		/* Track encrypted vma's */
 	kmem_cache_free(vm_area_cachep, tmp);
 fail_nomem:
 	retval = -ENOMEM;
diff --git a/mm/mmap.c b/mm/mmap.c
index 4c604eb644b4..7390b8b69fd6 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -182,6 +182,7 @@ static struct vm_area_struct *remove_vma(struct vm_area_struct *vma)
 	if (vma->vm_file)
 		fput(vma->vm_file);
 	mpol_put(vma_policy(vma));
+	vma_put_encrypt_ref(vma);
 	kmem_cache_free(vm_area_cachep, vma);
 	return next;
 }
@@ -913,6 +914,7 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
 			anon_vma_merge(vma, next);
 		mm->map_count--;
 		mpol_put(vma_policy(next));
+		vma_put_encrypt_ref(next);
 		kmem_cache_free(vm_area_cachep, next);
 		/*
 		 * In mprotect's case 6 (see comments on vma_merge),
@@ -1744,6 +1746,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
 		goto unacct_error;
 	}
 
+	vma_get_encrypt_ref(vma);
 	vma->vm_mm = mm;
 	vma->vm_start = addr;
 	vma->vm_end = addr + len;
@@ -1839,6 +1842,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
 unmap_and_free_vma:
 	vma->vm_file = NULL;
 	fput(file);
+	vma_put_encrypt_ref(vma);
 
 	/* Undo any partial mapping done by a device driver. */
 	unmap_region(mm, vma, prev, vma->vm_start, vma->vm_end);
@@ -2653,6 +2657,7 @@ int __split_vma(struct mm_struct *mm, struct vm_area_struct *vma,
 		new->vm_pgoff += ((addr - vma->vm_start) >> PAGE_SHIFT);
 	}
 
+	vma_get_encrypt_ref(new);
 	err = vma_dup_policy(vma, new);
 	if (err)
 		goto out_free_vma;
@@ -2686,6 +2691,7 @@ int __split_vma(struct mm_struct *mm, struct vm_area_struct *vma,
  out_free_mpol:
 	mpol_put(vma_policy(new));
  out_free_vma:
+	vma_put_encrypt_ref(new);
 	kmem_cache_free(vm_area_cachep, new);
 	return err;
 }
@@ -3007,6 +3013,7 @@ static int do_brk_flags(unsigned long addr, unsigned long len, unsigned long fla
 		return -ENOMEM;
 	}
 
+	vma_get_encrypt_ref(vma);
 	INIT_LIST_HEAD(&vma->anon_vma_chain);
 	vma->vm_mm = mm;
 	vma->vm_ops = &anon_vm_ops;
@@ -3229,6 +3236,7 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap,
 		new_vma->vm_pgoff = pgoff;
 		if (vma_dup_policy(vma, new_vma))
 			goto out_free_vma;
+		vma_get_encrypt_ref(new_vma);
 		INIT_LIST_HEAD(&new_vma->anon_vma_chain);
 		if (anon_vma_clone(new_vma, vma))
 			goto out_free_mempol;
@@ -3243,6 +3251,7 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap,
 
 out_free_mempol:
 	mpol_put(vma_policy(new_vma));
+	vma_put_encrypt_ref(new_vma);
 out_free_vma:
 	kmem_cache_free(vm_area_cachep, new_vma);
 out:
@@ -3372,6 +3381,9 @@ static struct vm_area_struct *__install_special_mapping(
 	if (unlikely(vma == NULL))
 		return ERR_PTR(-ENOMEM);
 
+	/* Do not expect a memory encrypted vma here */
+	VM_WARN_ON(vma_keyid(vma));
+
 	INIT_LIST_HEAD(&vma->anon_vma_chain);
 	vma->vm_mm = mm;
 	vma->vm_start = addr;
diff --git a/mm/nommu.c b/mm/nommu.c
index 73f66e81cfb0..85f04c174638 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -769,6 +769,7 @@ static void delete_vma(struct mm_struct *mm, struct vm_area_struct *vma)
 	if (vma->vm_file)
 		fput(vma->vm_file);
 	put_nommu_region(vma->vm_region);
+	vma_put_encrypt_ref(vma);
 	kmem_cache_free(vm_area_cachep, vma);
 }
 
@@ -1215,6 +1216,7 @@ unsigned long do_mmap(struct file *file,
 	if (!vma)
 		goto error_getting_vma;
 
+	vma_get_encrypt_ref(vma);
 	region->vm_usage = 1;
 	region->vm_flags = vm_flags;
 	region->vm_pgoff = pgoff;
@@ -1375,6 +1377,7 @@ unsigned long do_mmap(struct file *file,
 	kmem_cache_free(vm_region_jar, region);
 	if (vma->vm_file)
 		fput(vma->vm_file);
+	vma_put_encrypt_ref(vma);
 	kmem_cache_free(vm_area_cachep, vma);
 	return ret;
 
@@ -1486,6 +1489,7 @@ int split_vma(struct mm_struct *mm, struct vm_area_struct *vma,
 	*new = *vma;
 	*region = *vma->vm_region;
 	new->vm_region = region;
+	vma_get_encrypt_ref(new);
 
 	npages = (addr - vma->vm_start) >> PAGE_SHIFT;
 
-- 
2.14.1

[RFC 09/12] mm: Restrict memory encryption to anonymous VMA's

[Alison Schofield]

Memory encryption is only supported for mappings that are ANONYMOUS.
Test the entire range of VMA's in an encrypt_mprotect() request to
make sure they all meet that requirement before encrypting any.

The encrypt_mprotect syscall will return -EINVAL and will not encrypt
any VMA's if this check fails.

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 mm/mprotect.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/mm/mprotect.c b/mm/mprotect.c
index 6c2e1106525c..3384b755aad1 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -311,6 +311,24 @@ unsigned long change_protection(struct vm_area_struct *vma, unsigned long start,
 	return pages;
 }
 
+/*
+ * Encrypted mprotect is only supported on anonymous mappings.
+ * All VMA's in the requested range must be anonymous. If this
+ * test fails on any single VMA, the entire mprotect request fails.
+ */
+bool mem_supports_encryption(struct vm_area_struct *vma, unsigned long end)
+{
+	struct vm_area_struct *test_vma = vma;
+
+	do {
+		if (!vma_is_anonymous(test_vma))
+			return false;
+
+		test_vma = test_vma->vm_next;
+	} while (test_vma && test_vma->vm_start < end);
+	return true;
+}
+
 int
 mprotect_fixup(struct vm_area_struct *vma, struct vm_area_struct **pprev,
 	       unsigned long start, unsigned long end, unsigned long newflags,
@@ -491,6 +509,10 @@ static int do_mprotect_ext(unsigned long start, size_t len,
 				goto out;
 		}
 	}
+	if (keyid > 0 && !mem_supports_encryption(vma, end)) {
+		error = -EINVAL;
+		goto out;
+	}
 	if (start > vma->vm_start)
 		prev = vma;
 
-- 
2.14.1

[RFC 10/12] x86/pconfig: Program memory encryption keys on a system-wide basis

[Alison Schofield]

The kernel manages the MKTME (Multi-Key Total Memory Encryption) Keys
as a system wide single pool of keys. The hardware, however, manages
the keys on a per physical package basis. Each physical package
maintains a key table that all CPU's in that package share.

In order to maintain the consistent, system wide view that the kernel
requires, program all physical packages during a key program request.

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 arch/x86/include/asm/intel_pconfig.h | 42 ++++++++++++++++++++++++++++++------
 1 file changed, 36 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/intel_pconfig.h b/arch/x86/include/asm/intel_pconfig.h
index 3cb002b1d0f9..d3bf0a297e89 100644
--- a/arch/x86/include/asm/intel_pconfig.h
+++ b/arch/x86/include/asm/intel_pconfig.h
@@ -3,6 +3,7 @@
 
 #include <asm/asm.h>
 #include <asm/processor.h>
+#include <linux/cpu.h>
 
 enum pconfig_target {
 	INVALID_TARGET	= 0,
@@ -47,19 +48,48 @@ struct mktme_key_program {
 	u8 key_field_2[64];
 } __packed __aligned(256);
 
-static inline int mktme_key_program(struct mktme_key_program *key_program)
+struct mktme_key_program_info {
+	struct mktme_key_program *key_program;
+	unsigned long status;
+};
+
+static void mktme_package_program(void *key_program_info)
 {
+	struct mktme_key_program_info *info = key_program_info;
 	unsigned long rax = MKTME_KEY_PROGRAM;
 
+	asm volatile(PCONFIG
+		: "=a" (rax), "=b" (info->key_program)
+		: "0" (rax), "1" (info->key_program)
+		: "memory", "cc");
+
+	if (rax != MKTME_PROG_SUCCESS)
+		WRITE_ONCE(info->status, rax);
+}
+
+/*
+ * MKTME keys are managed as a system-wide single pool of keys.
+ * In the hardware, each physical package maintains a separate key
+ * table. Program all physical packages with the same key info to
+ * maintain that system-wide kernel view.
+ */
+static inline int mktme_key_program(struct mktme_key_program *key_program,
+				    cpumask_var_t mktme_cpumask)
+{
+	struct mktme_key_program_info info = {
+		.key_program = key_program,
+		.status = MKTME_PROG_SUCCESS,
+	};
+
 	if (!pconfig_target_supported(MKTME_TARGET))
 		return -ENXIO;
 
-	asm volatile(PCONFIG
-		: "=a" (rax), "=b" (key_program)
-		: "0" (rax), "1" (key_program)
-		: "memory", "cc");
+	get_online_cpus();
+	on_each_cpu_mask(mktme_cpumask, mktme_package_program,
+			 &info, 1);
+	put_online_cpus();
 
-	return rax;
+	return info.status;
 }
 
 #endif	/* _ASM_X86_INTEL_PCONFIG_H */
-- 
2.14.1

[RFC 11/12] keys/mktme: Add a new key service type for memory encryption keys

[Alison Schofield]

MKTME (Multi-Key Total Memory Encryption) is a technology that allows
transparent memory encryption in upcoming Intel platforms. MKTME will
support mulitple encryption domains, each having their own key. The main
use case for the feature is virtual machine isolation. The API needs the
flexibility to work for a wide range of uses.

The MKTME key service type manages the addition and removal of the memory
encryption keys. It maps software keys to hardware keyids and programs
the hardware with the user requested encryption options.

The only supported encryption algorithm is AES-XTS 128.

The MKTME key service is half of the MKTME API level solution. It pairs
with a new memory encryption system call: encrypt_mprotect() that uses
the keys to encrypt memory.

See Documentation/x86/mktme-keys.txt

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 arch/x86/Kconfig           |   1 +
 include/keys/mktme-type.h  |  28 +++++
 security/keys/Kconfig      |  11 ++
 security/keys/Makefile     |   1 +
 security/keys/mktme_keys.c | 278 +++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 319 insertions(+)
 create mode 100644 include/keys/mktme-type.h
 create mode 100644 security/keys/mktme_keys.c

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 023a22568c06..50d8aa6a58e9 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1527,6 +1527,7 @@ config X86_INTEL_MKTME
 	bool "Intel Multi-Key Total Memory Encryption"
 	select DYNAMIC_PHYSICAL_MASK
 	select PAGE_EXTENSION
+	select MKTME_KEYS
 	depends on X86_64 && CPU_SUP_INTEL
 	---help---
 	  Say yes to enable support for Multi-Key Total Memory Encryption.
diff --git a/include/keys/mktme-type.h b/include/keys/mktme-type.h
new file mode 100644
index 000000000000..bebe74cb2b51
--- /dev/null
+++ b/include/keys/mktme-type.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * Key service for Multi-KEY Total Memory Encryption
+ */
+
+#ifndef _KEYS_MKTME_TYPE_H
+#define _KEYS_MKTME_TYPE_H
+
+#include <linux/key.h>
+
+/*
+ * The AES-XTS 128 encryption algorithm requires 128 bits for each
+ * user supplied option: userkey=, tweak=, entropy=.
+ */
+#define MKTME_AES_XTS_SIZE	16
+
+enum mktme_alg {
+	MKTME_ALG_AES_XTS_128,
+};
+
+const char *const mktme_alg_names[] = {
+	[MKTME_ALG_AES_XTS_128]	= "aes_xts_128",
+};
+
+extern struct key_type key_type_mktme;
+
+#endif /* _KEYS_MKTME_TYPE_H */
diff --git a/security/keys/Kconfig b/security/keys/Kconfig
index 6462e6654ccf..c36972113e67 100644
--- a/security/keys/Kconfig
+++ b/security/keys/Kconfig
@@ -101,3 +101,14 @@ config KEY_DH_OPERATIONS
 	 in the kernel.
 
 	 If you are unsure as to whether this is required, answer N.
+
+config MKTME_KEYS
+	bool "Multi-Key Total Memory Encryption Keys"
+	depends on KEYS && X86_INTEL_MKTME
+	help
+	  This option provides support for Multi-Key Total Memory
+	  Encryption (MKTME) on Intel platforms offering the feature.
+	  MKTME allows userspace to manage the hardware encryption
+	  keys through the kernel key services.
+
+	  If you are unsure as to whether this is required, answer N.
diff --git a/security/keys/Makefile b/security/keys/Makefile
index ef1581b337a3..2d9f9a82cb8a 100644
--- a/security/keys/Makefile
+++ b/security/keys/Makefile
@@ -29,3 +29,4 @@ obj-$(CONFIG_KEY_DH_OPERATIONS) += dh.o
 obj-$(CONFIG_BIG_KEYS) += big_key.o
 obj-$(CONFIG_TRUSTED_KEYS) += trusted.o
 obj-$(CONFIG_ENCRYPTED_KEYS) += encrypted-keys/
+obj-$(CONFIG_MKTME_KEYS) += mktme_keys.o
diff --git a/security/keys/mktme_keys.c b/security/keys/mktme_keys.c
new file mode 100644
index 000000000000..dcbce7194647
--- /dev/null
+++ b/security/keys/mktme_keys.c
@@ -0,0 +1,278 @@
+// SPDX-License-Identifier: GPL-3.0
+
+/* Documentation/x86/mktme-keys.txt */
+
+#include <linux/cred.h>
+#include <linux/cpu.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/key.h>
+#include <linux/key-type.h>
+#include <linux/init.h>
+#include <linux/parser.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <asm/intel_pconfig.h>
+#include <asm/mktme.h>
+#include <keys/mktme-type.h>
+#include <keys/user-type.h>
+
+#include "internal.h"
+
+struct kmem_cache *mktme_prog_cache;	/* hardware programming struct */
+cpumask_var_t mktme_cpumask;		/* one cpu per pkg to program keys */
+
+static const char * const mktme_program_err[] = {
+	"KeyID was successfully programmed",	/* 0 */
+	"Invalid KeyID programming command",	/* 1 */
+	"Insufficient entropy",			/* 2 */
+	"KeyID not valid",			/* 3 */
+	"Invalid encryption algorithm chosen",	/* 4 */
+	"Failure to access key table",		/* 5 */
+};
+
+/* If a key is available, program and add the key to the software map. */
+static int mktme_program_key(key_serial_t serial,
+			     struct mktme_key_program *kprog)
+{
+	int keyid, ret;
+
+	keyid = mktme_map_get_free_keyid();
+	if (keyid == 0)
+		return -EDQUOT;
+
+	kprog->keyid = keyid;
+	ret = mktme_key_program(kprog, mktme_cpumask);
+	if (ret == MKTME_PROG_SUCCESS)
+		mktme_map_set_keyid(keyid, serial);
+	else
+		pr_debug("mktme: %s [%d]\n", mktme_program_err[ret], ret);
+
+	return ret;
+}
+
+enum mktme_opt_id	{
+	OPT_ERROR = -1,
+	OPT_USERKEY,
+	OPT_TWEAK,
+	OPT_ENTROPY,
+	OPT_ALGORITHM,
+};
+
+static const match_table_t mktme_token = {
+	{OPT_USERKEY, "userkey=%s"},
+	{OPT_TWEAK, "tweak=%s"},
+	{OPT_ENTROPY, "entropy=%s"},
+	{OPT_ALGORITHM, "algorithm=%s"},
+	{OPT_ERROR, NULL}
+
+};
+
+/*
+ * Algorithm AES-XTS 128 is the only supported encryption algorithm.
+ * CPU Generated Key: requires user supplied entropy and accepts no
+ *		      other options.
+ * User Supplied Key: requires user supplied tweak key and accepts
+ *		      no other options.
+ */
+static int mktme_check_options(struct mktme_key_program *kprog,
+			       unsigned long token_mask)
+{
+	if (!token_mask)
+		return -EINVAL;
+
+	kprog->keyid_ctrl |= MKTME_AES_XTS_128;
+
+	if (!test_bit(OPT_USERKEY, &token_mask)) {
+		if ((!test_bit(OPT_ENTROPY, &token_mask)) ||
+		    (test_bit(OPT_TWEAK, &token_mask)))
+			return -EINVAL;
+
+		kprog->keyid_ctrl |= MKTME_KEYID_SET_KEY_RANDOM;
+	}
+	if (test_bit(OPT_USERKEY, &token_mask)) {
+		if ((test_bit(OPT_ENTROPY, &token_mask)) ||
+		    (!test_bit(OPT_TWEAK, &token_mask)))
+			return -EINVAL;
+
+		kprog->keyid_ctrl |= MKTME_KEYID_SET_KEY_DIRECT;
+	}
+	return 0;
+}
+
+/*
+ * Parse the options and begin to fill in the key programming struct kprog.
+ * Check the lengths of incoming data and push data directly into kprog fields.
+ */
+static int mktme_get_options(char *options, struct mktme_key_program *kprog)
+{
+	int len = MKTME_AES_XTS_SIZE / 2;
+	substring_t args[MAX_OPT_ARGS];
+	unsigned long token_mask = 0;
+	enum mktme_alg alg;
+	char *p = options;
+	int ret, token;
+
+	while ((p = strsep(&options, " \t"))) {
+		if (*p == '\0' || *p == ' ' || *p == '\t')
+			continue;
+		token = match_token(p, mktme_token, args);
+		if (test_and_set_bit(token, &token_mask))
+			return -EINVAL;
+
+		switch (token) {
+		case OPT_USERKEY:
+			if (strlen(args[0].from) != MKTME_AES_XTS_SIZE)
+				return -EINVAL;
+			ret = hex2bin(kprog->key_field_1, args[0].from, len);
+			if (ret < 0)
+				return -EINVAL;
+			break;
+
+		case OPT_TWEAK:
+			if (strlen(args[0].from) != MKTME_AES_XTS_SIZE)
+				return -EINVAL;
+			ret = hex2bin(kprog->key_field_2, args[0].from, len);
+			if (ret < 0)
+				return -EINVAL;
+			break;
+
+		case OPT_ENTROPY:
+			if (strlen(args[0].from) != MKTME_AES_XTS_SIZE)
+				return -EINVAL;
+			/* Applied to both CPU-generated data and tweak keys */
+			ret = hex2bin(kprog->key_field_1, args[0].from, len);
+			ret = hex2bin(kprog->key_field_2, args[0].from, len);
+			if (ret < 0)
+				return -EINVAL;
+			break;
+
+		case OPT_ALGORITHM:
+			alg = match_string(mktme_alg_names,
+					   ARRAY_SIZE(mktme_alg_names),
+					   args[0].from);
+			if (alg != MKTME_ALG_AES_XTS_128)
+				return -EINVAL;
+			break;
+
+		default:
+			return -EINVAL;
+		}
+	}
+	return mktme_check_options(kprog, token_mask);
+}
+
+/* Key Service Command: Creates a software key and programs hardware */
+int mktme_instantiate(struct key *key, struct key_preparsed_payload *prep)
+{
+	struct mktme_key_program *kprog = NULL;
+	size_t datalen = prep->datalen;
+	char *options;
+	int ret = 0;
+
+	if (!capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
+		return -EACCES;
+
+	if (datalen <= 0 || datalen > 1024 || !prep->data)
+		return -EINVAL;
+
+	options = kmemdup(prep->data, datalen + 1, GFP_KERNEL);
+	if (!options)
+		return -ENOMEM;
+
+	options[datalen] = '\0';
+
+	kprog = kmem_cache_zalloc(mktme_prog_cache, GFP_KERNEL);
+	if (!kprog) {
+		kzfree(options);
+		return -ENOMEM;
+	}
+	ret = mktme_get_options(options, kprog);
+	if (ret < 0)
+		goto out;
+
+	mktme_map_lock();
+	ret = mktme_program_key(key->serial, kprog);
+	mktme_map_unlock();
+out:
+	kzfree(options);
+	kmem_cache_free(mktme_prog_cache, kprog);
+	return ret;
+}
+
+struct key_type key_type_mktme = {
+	.name = "mktme",
+	.instantiate = mktme_instantiate,
+	.describe = user_describe,
+};
+
+/*
+ * Build mktme_cpumask to include one cpu per physical package.
+ * The mask is used in mktme_key_program() when the hardware key
+ * table is programmed on a per package basis.
+ */
+static int mktme_build_cpumask(void)
+{
+	int online_cpu, mktme_cpu;
+	int online_pkgid, mktme_pkgid = -1;
+
+	if (!zalloc_cpumask_var(&mktme_cpumask, GFP_KERNEL))
+		return -ENOMEM;
+
+	for_each_online_cpu(online_cpu) {
+		online_pkgid = topology_physical_package_id(online_cpu);
+
+		for_each_cpu(mktme_cpu, mktme_cpumask) {
+			mktme_pkgid = topology_physical_package_id(mktme_cpu);
+			if (mktme_pkgid == online_pkgid)
+				break;
+		}
+		if (mktme_pkgid != online_pkgid)
+			cpumask_set_cpu(online_cpu, mktme_cpumask);
+	}
+	return 0;
+}
+
+/*
+ * Allocate the global key map structure based on the available keyids
+ * at boot time. Create a cache and a cpu_mask to use for programming
+ * the hardware. Initialize the encrypt_count array to track VMA's per
+ * keyid. Once all that succeeds, register the 'mktme' key type.
+ */
+static int __init init_mktme(void)
+{
+	int ret;
+
+	/* Verify keys are present */
+	if (!(mktme_nr_keyids > 0))
+		return -EINVAL;
+
+	if (!mktme_map_alloc())
+		return -ENOMEM;
+
+	mktme_prog_cache = KMEM_CACHE(mktme_key_program, SLAB_PANIC);
+	if (!mktme_prog_cache)
+		goto free_map;
+
+	if (vma_alloc_encrypt_array() < 0)
+		goto free_cache;
+
+	if (mktme_build_cpumask() < 0)
+		goto free_array;
+
+	ret = register_key_type(&key_type_mktme);
+	if (!ret)
+		return ret;
+
+	free_cpumask_var(mktme_cpumask);
+free_array:
+	vma_free_encrypt_array();
+free_cache:
+	kmem_cache_destroy(mktme_prog_cache);
+free_map:
+	mktme_map_free();
+
+	return -ENOMEM;
+}
+
+late_initcall(init_mktme);
-- 
2.14.1

[RFC 12/12] keys/mktme: Do not revoke in use memory encryption keys

[Alison Schofield]

The MKTME key service maps userspace keys to hardware keyids. Those
keys are used in a new system call that encrypts memory. The keys
need to be tightly controlled. One example is that userspace keys
should not be revoked while the hardware keyid slot is still in use.

The KEY_FLAG_KEEP bit offers good control. The mktme service uses
that flag to prevent userspace keys from going away without proper
synchronization with the mktme service type.

The problem is that we need a safe and synchronous way to revoke keys.
The way .revoke methods function now, the key service type is called late
in the revoke process for cleanup after the fact. The mktme key service
has no means to consider and perhaps reject the revoke request.

This proposal inserts the MKTME revoke call earlier into the existing
keyctl <revoke> path. If it is safe to revoke the key, MKTME key service
will turn off KEY_FLAG_KEEP and let the revoke continue and succeed.
Otherwise, not safe, KEY_FLAG_KEEP stays on, which causes the normal
path of revoke to fail.

For the MKTME Key Service, a revoke may be done safely when there are
no outstanding memory mappings encrypted with the key being revoked.

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 security/keys/internal.h   |  6 ++++++
 security/keys/keyctl.c     |  7 +++++++
 security/keys/mktme_keys.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 60 insertions(+)

diff --git a/security/keys/internal.h b/security/keys/internal.h
index 9f8208dc0e55..9fb871522efe 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -316,4 +316,10 @@ static inline void key_check(const struct key *key)
 
 #endif
 
+#ifdef CONFIG_MKTME_KEYS
+extern void mktme_revoke_key(struct key *key);
+#else
+static inline void mktme_revoke_key(struct key *key) {}
+#endif /* CONFIG_MKTME_KEYS */
+
 #endif /* _INTERNAL_H */
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 1ffe60bb2845..86d2596ff275 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -363,6 +363,9 @@ long keyctl_update_key(key_serial_t id,
  * and any links to the key will be automatically garbage collected after a
  * certain amount of time (/proc/sys/kernel/keys/gc_delay).
  *
+ * The MKTME key service type checks if a memory encryption key is in use
+ * before allowing a revoke to proceed.
+ *
  * Keys with KEY_FLAG_KEEP set should not be revoked.
  *
  * If successful, 0 is returned.
@@ -387,6 +390,10 @@ long keyctl_revoke_key(key_serial_t id)
 
 	key = key_ref_to_ptr(key_ref);
 	ret = 0;
+
+	if (strcmp(key->type->name, "mktme") == 0)
+		mktme_revoke_key(key);
+
 	if (test_bit(KEY_FLAG_KEEP, &key->flags))
 		ret = -EPERM;
 	else
diff --git a/security/keys/mktme_keys.c b/security/keys/mktme_keys.c
index dcbce7194647..c665be860538 100644
--- a/security/keys/mktme_keys.c
+++ b/security/keys/mktme_keys.c
@@ -31,6 +31,52 @@ static const char * const mktme_program_err[] = {
 	"Failure to access key table",		/* 5 */
 };
 
+static int mktme_clear_programmed_key(int keyid)
+{
+	struct mktme_key_program *kprog = NULL;
+	int ret;
+
+	kprog = kmem_cache_zalloc(mktme_prog_cache, GFP_KERNEL);
+	if (!kprog)
+		return -ENOMEM;
+
+	kprog->keyid = keyid;
+	kprog->keyid_ctrl = MKTME_KEYID_CLEAR_KEY;
+	ret = mktme_key_program(kprog, mktme_cpumask);
+	if (ret == MKTME_PROG_SUCCESS)
+		mktme_map_clear_keyid(keyid);
+	else
+		pr_debug("mktme: %s [%d]\n", mktme_program_err[ret], ret);
+
+	kmem_cache_free(mktme_prog_cache, kprog);
+	return ret;
+}
+
+/*
+ * If the key is not in use, clear the hardware programming and
+ * allow the revoke to continue by clearing KEY_FLAG_KEEP.
+ */
+void mktme_revoke_key(struct key *key)
+{
+	int keyid, vma_count;
+
+	mktme_map_lock();
+	keyid = mktme_map_keyid_from_serial(key->serial);
+	if (keyid <= 0)
+		goto out;
+
+	vma_count = vma_read_encrypt_ref(keyid);
+	if (vma_count > 0) {
+		pr_debug("mktme not freeing keyid[%d] encrypt_count[%d]\n",
+			 keyid, vma_count);
+		goto out;
+	}
+	if (!mktme_clear_programmed_key(keyid))
+		clear_bit(KEY_FLAG_KEEP, &key->flags);
+out:
+	mktme_map_unlock();
+}
+
 /* If a key is available, program and add the key to the software map. */
 static int mktme_program_key(key_serial_t serial,
 			     struct mktme_key_program *kprog)
@@ -193,6 +239,7 @@ int mktme_instantiate(struct key *key, struct key_preparsed_payload *prep)
 
 	mktme_map_lock();
 	ret = mktme_program_key(key->serial, kprog);
+	set_bit(KEY_FLAG_KEEP, &key->flags);
 	mktme_map_unlock();
 out:
 	kzfree(options);
-- 
2.14.1

Re: [RFC 01/12] docs/x86: Document the Multi-Key Total Memory Encryption API

[Randy Dunlap]

On 09/07/2018 03:34 PM, Alison Schofield wrote:
> Document the API's used for MKTME on Intel platforms.
> MKTME: Multi-KEY Total Memory Encryption
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>

Hi,
A few comments below...

> ---
>  Documentation/x86/mktme-keys.txt | 153 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 153 insertions(+)
>  create mode 100644 Documentation/x86/mktme-keys.txt
> 
> diff --git a/Documentation/x86/mktme-keys.txt b/Documentation/x86/mktme-keys.txt
> new file mode 100644
> index 000000000000..2dea7acd2a17
> --- /dev/null
> +++ b/Documentation/x86/mktme-keys.txt
> @@ -0,0 +1,153 @@
> +MKTME (Multi-Key Total Memory Encryption) is a technology that allows
> +memory encryption on Intel platforms. Whereas TME (Total Memory Encryption)
> +allows encryption of the entire system memory using a single key, MKTME
> +allows multiple encryption domains, each having their own key. The main use
> +case for the feature is virtual machine isolation. The API's introduced here
> +are intended to offer flexibility to work in a wide range of uses.
> +
> +The externally available Intel Architecture Spec:
> +https://software.intel.com/sites/default/files/managed/a5/16/Multi-Key-Total-Memory-Encryption-Spec.pdf
> +
> +============================  API Overview  ============================
> +
> +There are 2 MKTME specific API's that enable userspace to create and use
> +the memory encryption keys:
> +
> +1) Kernel Key Service: MKTME Type
> +
> +   MKTME is a new key type added to the existing Kernel Key Services
> +   to support the memory encryption keys. The MKTME service manages
> +   the addition and removal of MKTME keys. It maps userspace keys
> +   to hardware keyids and programs the hardware with user requested
> +   encryption parameters.
> +
> +   o An understanding of the Kernel Key Service is required in order
> +     to use the MKTME key type as it is a subset of that service.

reference available?

> +
> +   o MKTME keys are a limited resource. There is a single pool of
> +     MKTME keys for a system and that pool can be from 3 to 63 keys.
> +     With that in mind, userspace may take advantage of the kernel
> +     key services sharing and permissions model for userspace keys.
> +     One key can be shared as long as each user has the permission
> +     of "KEY_NEED_VIEW" to use it.
> +
> +   o MKTME key type uses capabilities to restrict the allocation
> +     of keys. It only requires CAP_SYS_RESOURCE, but will accept
> +     the broader capability of CAP_SYS_ADMIN.  See capabilities(7).
> +
> +   o The MKTME key service blocks kernel key service commands that
> +     could lead to reprogramming of in use keys, or loss of keys from
> +     the pool. This means MKTME does not allow a key to be invalidated,
> +     unlinked, or timed out. These operations are blocked by MKTME as
> +     it creates all keys with the internal flag KEY_FLAG_KEEP.
> +
> +   o MKTME does not support the keyctl option of UPDATE. Userspace
> +     may change the programming of a key by revoking it and adding
> +     a new key with the updated encryption options (or vice-versa).
> +
> +2) System Call: encrypt_mprotect()
> +
> +   MKTME encryption is requested by calling encrypt_mprotect(). The
> +   caller passes the serial number to a previously allocated and
> +   programmed encryption key. That handle was created with the MKTME
> +   Key Service.
> +
> +   o The caller must have KEY_NEED_VIEW permission on the key

                                                             key.

> +
> +   o The range of memory that is to be protected must be mapped as
> +     ANONYMOUS. If it is not, the entire encrypt_mprotect() request
> +     fails with EINVAL.
> +
> +   o As an extension to the existing mprotect() system call,
> +     encrypt_mprotect() supports the legacy mprotect behavior plus
> +     the enabling of memory encryption. That means that in addition
> +     to encrypting the memory, the protection flags will be updated
> +     as requested in the call.
> +
> +   o Additional mprotect() calls to memory already protected with
> +     MKTME will not alter the MKTME status.
> +
> +======================  Usage: MKTME Key Service  ======================
> +
> +MKTME is enabled on supported Intel platforms by selecting
> +CONFIG_X86_INTEL_MKTME which selects CONFIG_MKTME_KEYS.
> +
> +Allocating MKTME Keys via command line or system call:
> +    keyctl add mktme name "[options]" ring
> +
> +    key_serial_t add_key(const char *type, const char *description,
> +                         const void *payload, size_t plen,
> +                         key_serial_t keyring);
> +
> +Revoking MKTME Keys via command line or system call::
> +   keyctl revoke <key>
> +
> +   long keyctl(KEYCTL_REVOKE, key_serial_t key);
> +
> +Options Field Definition:
> +    userkey=      ASCII HEX value encryption key. Defaults to a CPU
> +		  generated key if a userkey is not defined here.
> +
> +    algorithm=    Encryption algorithm name as a string.
> +		  Valid algorithm: "aes-xts-128"
> +
> +    tweak=        ASCII HEX value tweak key. Tweak key will be added to the
> +                  userkey...  (need to be clear here that this is being sent
> +                  to the hardware - kernel not messing w it)

                                                          with it)

> +
> +    entropy=      ascii hex value entropy.

                     ASCII

> +                  This entropy will be used to generated the CPU key and

                                               to generate the

> +		  the tweak key when CPU generated key is requested.
> +
> +Algorithm Dependencies:
> +    AES-XTS 128 is the only supported algorithm.
> +    There are only 2 ways that AES-XTS 128 may be used:
> +
> +    1) User specified encryption key
> +	- The user specified encryption key must be exactly
> +	  16 ASCII Hex bytes (128 bits).
> +	- A tweak key must be specified and it must be exactly
> +	  16 ASCII Hex bytes (128 bits).
> +	- No entropy field is accepted.
> +
> +    2) CPU generated encryption key
> +	- When no user specified encryption key is provided, the
> +	  default encryption key will be CPU generated.
> +	- User must specify 16 ASCII Hex bytes of entropy. This 

above line ends with a space.  Please drop it.

> +	  entropy will be used by the CPU to generate both the
> +	  encryption key and the tweak key.
> +	- No entropy field is accepted.
> +
> +======================  Usage: encrypt_mprotect()  ======================
> +
> +System Call encrypt_mprotect()::
> +
> +    This system call is an extension of the existing mprotect() system
> +    call. It requires the same parameters as legary mprotect() plus

                                                legacy

> +    one additional parameter, the keyid. Userspace must provide the
> +    key serial number assigned through the kernel key service.
> +
> +    int encrypt_mprotect(void *addr, size_t len, int prot, int keyid);
> +
> +======================  Usage: Sample Roundtrip  ======================
> +
> +Sample usage of MKTME Key Service API with encrypt_mprotect() API:
> +
> +  Add a key:
> +        key = add_key(mktme, name, options, strlen(option), keyring);

                                               strlen(options),

> +
> +  Map memory:
> +        ptr = mmap(NULL, size, prot, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
> +
> +  Protect memory:
> +        ret = syscall(sys_encrypt_mprotect, ptr, size, prot, keyid);
> +
> +  Use protected memory:
> +        ................
> +
> +  Free memory:
> +        ret = munmap(ptr, size);
> +
> +  Revoke key:
> +        ret = keyctl(KEYCTL_REVOKE, key);
> +
> 


-- 
~Randy

RE: [RFC 00/12] Multi-Key Total Memory Encryption API (MKTME)

[Huang, Kai]

> -----Original Message-----
> From: keyrings-owner@vger.kernel.org [mailto:keyrings-
> owner@vger.kernel.org] On Behalf Of Alison Schofield
> Sent: Saturday, September 8, 2018 10:23 AM
> To: dhowells@redhat.com; tglx@linutronix.de
> Cc: Huang, Kai <kai.huang@intel.com>; Nakajima, Jun
> <jun.nakajima@intel.com>; Shutemov, Kirill <kirill.shutemov@intel.com>;
> Hansen, Dave <dave.hansen@intel.com>; Sakkinen, Jarkko
> <jarkko.sakkinen@intel.com>; jmorris@namei.org; keyrings@vger.kernel.org;
> linux-security-module@vger.kernel.org; mingo@redhat.com; hpa@zytor.com;
> x86@kernel.org; linux-mm@kvack.org
> Subject: [RFC 00/12] Multi-Key Total Memory Encryption API (MKTME)
> 
> Seeking comments on the APIs supporting MKTME on future Intel platforms.
> 
> MKTME (Multi-Key Total Memory Encryption) is a technology supporting
> memory encryption on upcoming Intel platforms. Whereas TME allows
> encryption of the entire system memory using a single key, MKTME allows
> mulitple encryption domains, each having their own key. While the main use
> case for the feature is virtual machine isolation, the API needs the flexibility to
> work for a wide range of use cases.
> 
> This RFC presents the 2 API additions that enable userspace to:
>  1) Create Encryption Keys: Kernel Key Service type "mktme"
>  2) Use the Encryption Keys: system call encrypt_mprotect()
> 
> In order to share between: the Kernel Key Service, the new system call, and the
> existing mm code, helper functions were created in arch/x86/mktme

IMHO, we can separate this series into 2 parts, as you did above, and send out them separately. The reason is, in general I think adding new MKTME type to key retention services is not that related to memory management code, namely the encrypt_mprotect() API part.

So if we split the two parts and send them out separately, the first part can be reviewed by keyring and security guys, without involving mm guys, and the encrypt_mprotect() part can be more reviewed more by mm guys. 

And since encrypt_mprotect() is a new syscall, you may need to add more lists for the review, ie, linux-api, and maybe linux-kernel as well.

Thanks,
-Kai

> 
> This patchset is built upon Kirill Shutemov's patchset for the core MKTME
> support. You can find that here:
> git://git.kernel.org/pub/scm/linux/kernel/git/kas/linux.git mktme/wip
> 
> 
> Alison Schofield (12):
>   docs/x86: Document the Multi-Key Total Memory Encryption API
>   mm: Generalize the mprotect implementation to support extensions
>   syscall/x86: Wire up a new system call for memory encryption keys
>   x86/mm: Add helper functions to manage memory encryption keys
>   x86/mm: Add a helper function to set keyid bits in encrypted VMA's
>   mm: Add the encrypt_mprotect() system call
>   x86/mm: Add helper functions to track encrypted VMA's
>   mm: Track VMA's in use for each memory encryption keyid
>   mm: Restrict memory encryption to anonymous VMA's
>   x86/pconfig: Program memory encryption keys on a system-wide basis
>   keys/mktme: Add a new key service type for memory encryption keys
>   keys/mktme: Do not revoke in use memory encryption keys
> 
>  Documentation/x86/mktme-keys.txt       | 153 ++++++++++++++++
>  arch/x86/Kconfig                       |   1 +
>  arch/x86/entry/syscalls/syscall_32.tbl |   1 +
>  arch/x86/entry/syscalls/syscall_64.tbl |   1 +
>  arch/x86/include/asm/intel_pconfig.h   |  42 ++++-
>  arch/x86/include/asm/mktme.h           |  21 +++
>  arch/x86/mm/mktme.c                    | 141 ++++++++++++++
>  fs/exec.c                              |   4 +-
>  include/keys/mktme-type.h              |  28 +++
>  include/linux/key.h                    |   2 +
>  include/linux/mm.h                     |   9 +-
>  include/linux/syscalls.h               |   2 +
>  include/uapi/asm-generic/unistd.h      |   4 +-
>  kernel/fork.c                          |   2 +
>  kernel/sys_ni.c                        |   2 +
>  mm/mmap.c                              |  12 ++
>  mm/mprotect.c                          |  93 +++++++++-
>  mm/nommu.c                             |   4 +
>  security/keys/Kconfig                  |  11 ++
>  security/keys/Makefile                 |   1 +
>  security/keys/internal.h               |   6 +
>  security/keys/keyctl.c                 |   7 +
>  security/keys/mktme_keys.c             | 325
> +++++++++++++++++++++++++++++++++
>  23 files changed, 855 insertions(+), 17 deletions(-)  create mode 100644
> Documentation/x86/mktme-keys.txt  create mode 100644 include/keys/mktme-
> type.h  create mode 100644 security/keys/mktme_keys.c
> 
> --
> 2.14.1

RE: [RFC 01/12] docs/x86: Document the Multi-Key Total Memory Encryption API

[Huang, Kai]

> -----Original Message-----
> From: owner-linux-mm@kvack.org [mailto:owner-linux-mm@kvack.org] On
> Behalf Of Alison Schofield
> Sent: Saturday, September 8, 2018 10:34 AM
> To: dhowells@redhat.com; tglx@linutronix.de
> Cc: Huang, Kai <kai.huang@intel.com>; Nakajima, Jun
> <jun.nakajima@intel.com>; Shutemov, Kirill <kirill.shutemov@intel.com>;
> Hansen, Dave <dave.hansen@intel.com>; Sakkinen, Jarkko
> <jarkko.sakkinen@intel.com>; jmorris@namei.org; keyrings@vger.kernel.org;
> linux-security-module@vger.kernel.org; mingo@redhat.com; hpa@zytor.com;
> x86@kernel.org; linux-mm@kvack.org
> Subject: [RFC 01/12] docs/x86: Document the Multi-Key Total Memory
> Encryption API
> 
> Document the API's used for MKTME on Intel platforms.
> MKTME: Multi-KEY Total Memory Encryption
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>  Documentation/x86/mktme-keys.txt | 153
> +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 153 insertions(+)
>  create mode 100644 Documentation/x86/mktme-keys.txt
> 
> diff --git a/Documentation/x86/mktme-keys.txt b/Documentation/x86/mktme-
> keys.txt
> new file mode 100644
> index 000000000000..2dea7acd2a17
> --- /dev/null
> +++ b/Documentation/x86/mktme-keys.txt
> @@ -0,0 +1,153 @@
> +MKTME (Multi-Key Total Memory Encryption) is a technology that allows
> +memory encryption on Intel platforms. Whereas TME (Total Memory
> +Encryption) allows encryption of the entire system memory using a
> +single key, MKTME allows multiple encryption domains, each having their
> +own key. The main use case for the feature is virtual machine
> +isolation. The API's introduced here are intended to offer flexibility to work in a
> wide range of uses.
> +
> +The externally available Intel Architecture Spec:
> +https://software.intel.com/sites/default/files/managed/a5/16/Multi-Key-
> +Total-Memory-Encryption-Spec.pdf
> +
> +============================  API Overview
> +============================
> +
> +There are 2 MKTME specific API's that enable userspace to create and
> +use the memory encryption keys:
> +
> +1) Kernel Key Service: MKTME Type
> +
> +   MKTME is a new key type added to the existing Kernel Key Services
> +   to support the memory encryption keys. The MKTME service manages
> +   the addition and removal of MKTME keys. It maps userspace keys
> +   to hardware keyids and programs the hardware with user requested
> +   encryption parameters.
> +
> +   o An understanding of the Kernel Key Service is required in order
> +     to use the MKTME key type as it is a subset of that service.
> +
> +   o MKTME keys are a limited resource. There is a single pool of
> +     MKTME keys for a system and that pool can be from 3 to 63 keys.

Why 3 to 63 keys? Architecturally we are able to support up to 15-bit keyID, although in the first generation server we only support 6-bit keyID, which is 63 key/keyIDs (excluding keyID 0, which is TME's keyID).

> +     With that in mind, userspace may take advantage of the kernel
> +     key services sharing and permissions model for userspace keys.
> +     One key can be shared as long as each user has the permission
> +     of "KEY_NEED_VIEW" to use it.
> +
> +   o MKTME key type uses capabilities to restrict the allocation
> +     of keys. It only requires CAP_SYS_RESOURCE, but will accept
> +     the broader capability of CAP_SYS_ADMIN.  See capabilities(7).
> +
> +   o The MKTME key service blocks kernel key service commands that
> +     could lead to reprogramming of in use keys, or loss of keys from
> +     the pool. This means MKTME does not allow a key to be invalidated,
> +     unlinked, or timed out. These operations are blocked by MKTME as
> +     it creates all keys with the internal flag KEY_FLAG_KEEP.
> +
> +   o MKTME does not support the keyctl option of UPDATE. Userspace
> +     may change the programming of a key by revoking it and adding
> +     a new key with the updated encryption options (or vice-versa).
> +
> +2) System Call: encrypt_mprotect()
> +
> +   MKTME encryption is requested by calling encrypt_mprotect(). The
> +   caller passes the serial number to a previously allocated and
> +   programmed encryption key. That handle was created with the MKTME
> +   Key Service.
> +
> +   o The caller must have KEY_NEED_VIEW permission on the key
> +
> +   o The range of memory that is to be protected must be mapped as
> +     ANONYMOUS. If it is not, the entire encrypt_mprotect() request
> +     fails with EINVAL.
> +
> +   o As an extension to the existing mprotect() system call,
> +     encrypt_mprotect() supports the legacy mprotect behavior plus
> +     the enabling of memory encryption. That means that in addition
> +     to encrypting the memory, the protection flags will be updated
> +     as requested in the call.
> +
> +   o Additional mprotect() calls to memory already protected with
> +     MKTME will not alter the MKTME status.

I think it's better to separate encrypt_mprotect() into another doc so both parts can be reviewed easier.

> +
> +======================  Usage: MKTME Key Service
> +======================
> +
> +MKTME is enabled on supported Intel platforms by selecting
> +CONFIG_X86_INTEL_MKTME which selects CONFIG_MKTME_KEYS.
> +
> +Allocating MKTME Keys via command line or system call:
> +    keyctl add mktme name "[options]" ring
> +
> +    key_serial_t add_key(const char *type, const char *description,
> +                         const void *payload, size_t plen,
> +                         key_serial_t keyring);
> +
> +Revoking MKTME Keys via command line or system call::
> +   keyctl revoke <key>
> +
> +   long keyctl(KEYCTL_REVOKE, key_serial_t key);
> +
> +Options Field Definition:
> +    userkey=      ASCII HEX value encryption key. Defaults to a CPU
> +		  generated key if a userkey is not defined here.
> +
> +    algorithm=    Encryption algorithm name as a string.
> +		  Valid algorithm: "aes-xts-128"
> +
> +    tweak=        ASCII HEX value tweak key. Tweak key will be added to the
> +                  userkey...  (need to be clear here that this is being sent
> +                  to the hardware - kernel not messing w it)
> +
> +    entropy=      ascii hex value entropy.
> +                  This entropy will be used to generated the CPU key and
> +		  the tweak key when CPU generated key is requested.
> +
> +Algorithm Dependencies:
> +    AES-XTS 128 is the only supported algorithm.
> +    There are only 2 ways that AES-XTS 128 may be used:
> +
> +    1) User specified encryption key
> +	- The user specified encryption key must be exactly
> +	  16 ASCII Hex bytes (128 bits).
> +	- A tweak key must be specified and it must be exactly
> +	  16 ASCII Hex bytes (128 bits).
> +	- No entropy field is accepted.
> +
> +    2) CPU generated encryption key
> +	- When no user specified encryption key is provided, the
> +	  default encryption key will be CPU generated.
> +	- User must specify 16 ASCII Hex bytes of entropy. This
> +	  entropy will be used by the CPU to generate both the
> +	  encryption key and the tweak key.
> +	- No entropy field is accepted.

This is not true. The spec says in CPU generated random mode, both 'key' and 'tweak' part are used to generate the final key and tweak respectively.

Actually, simple 'XOR' is used to generate the final key:

case KEYID_SET_KEY_RANDOM:
	......
	(* Mix user supplied entropy to the data key and tweak key *)
	TMP_RND_DATA_KEY = TMP_RND_KEY XOR
		TMP_KEY_PROGRAM_STRUCT.KEY_FIELD_1.BYTES[15:0];
	TMP_RND_TWEAK_KEY = TMP_RND_TWEAK_KEY XOR
		TMP_KEY_PROGRAM_STRUCT.KEY_FIELD_2.BYTES[15:0];

So I think we can either just remove 'entropy' parameter, since we can use both 'userkey' and 'tweak' even for random key mode.

In fact, which might be better IMHO, we can simply disallow or ignore 'userkey' and 'tweak' part for random key mode, since if we allow user to specify both entropies, and if user passes value with all 1, we are effectively making the key and tweak to be all 1, which is not random anymore.

Instead, kernel can generate random for both entropies, or we can simply uses 0, ignoring user input.

Thanks,
-Kai
> +
> +======================  Usage: encrypt_mprotect()
> +======================
> +
> +System Call encrypt_mprotect()::
> +
> +    This system call is an extension of the existing mprotect() system
> +    call. It requires the same parameters as legary mprotect() plus
> +    one additional parameter, the keyid. Userspace must provide the
> +    key serial number assigned through the kernel key service.
> +
> +    int encrypt_mprotect(void *addr, size_t len, int prot, int keyid);
> +
> +======================  Usage: Sample Roundtrip
> ======================
> +
> +Sample usage of MKTME Key Service API with encrypt_mprotect() API:
> +
> +  Add a key:
> +        key = add_key(mktme, name, options, strlen(option), keyring);
> +
> +  Map memory:
> +        ptr = mmap(NULL, size, prot, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
> +
> +  Protect memory:
> +        ret = syscall(sys_encrypt_mprotect, ptr, size, prot, keyid);
> +
> +  Use protected memory:
> +        ................
> +
> +  Free memory:
> +        ret = munmap(ptr, size);
> +
> +  Revoke key:
> +        ret = keyctl(KEYCTL_REVOKE, key);
> +
> --
> 2.14.1

RE: [RFC 10/12] x86/pconfig: Program memory encryption keys on a system-wide basis

[Huang, Kai]

> -----Original Message-----
> From: Schofield, Alison
> Sent: Saturday, September 8, 2018 10:38 AM
> To: dhowells@redhat.com; tglx@linutronix.de
> Cc: Huang, Kai <kai.huang@intel.com>; Nakajima, Jun
> <jun.nakajima@intel.com>; Shutemov, Kirill <kirill.shutemov@intel.com>;
> Hansen, Dave <dave.hansen@intel.com>; Sakkinen, Jarkko
> <jarkko.sakkinen@intel.com>; jmorris@namei.org; keyrings@vger.kernel.org;
> linux-security-module@vger.kernel.org; mingo@redhat.com; hpa@zytor.com;
> x86@kernel.org; linux-mm@kvack.org
> Subject: [RFC 10/12] x86/pconfig: Program memory encryption keys on a
> system-wide basis
> 
> The kernel manages the MKTME (Multi-Key Total Memory Encryption) Keys as a
> system wide single pool of keys. The hardware, however, manages the keys on a
> per physical package basis. Each physical package maintains a key table that all
> CPU's in that package share.
> 
> In order to maintain the consistent, system wide view that the kernel requires,
> program all physical packages during a key program request.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>  arch/x86/include/asm/intel_pconfig.h | 42
> ++++++++++++++++++++++++++++++------
>  1 file changed, 36 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/include/asm/intel_pconfig.h
> b/arch/x86/include/asm/intel_pconfig.h
> index 3cb002b1d0f9..d3bf0a297e89 100644
> --- a/arch/x86/include/asm/intel_pconfig.h
> +++ b/arch/x86/include/asm/intel_pconfig.h
> @@ -3,6 +3,7 @@
> 
>  #include <asm/asm.h>
>  #include <asm/processor.h>
> +#include <linux/cpu.h>
> 
>  enum pconfig_target {
>  	INVALID_TARGET	= 0,
> @@ -47,19 +48,48 @@ struct mktme_key_program {
>  	u8 key_field_2[64];
>  } __packed __aligned(256);
> 
> -static inline int mktme_key_program(struct mktme_key_program
> *key_program)
> +struct mktme_key_program_info {
> +	struct mktme_key_program *key_program;
> +	unsigned long status;
> +};
> +
> +static void mktme_package_program(void *key_program_info)
>  {
> +	struct mktme_key_program_info *info = key_program_info;
>  	unsigned long rax = MKTME_KEY_PROGRAM;
> 
> +	asm volatile(PCONFIG
> +		: "=a" (rax), "=b" (info->key_program)

Why do we need "=b" (info->key_program)? To me PCONFIG only reads from rbx, but won't write to it.

> +		: "0" (rax), "1" (info->key_program)
> +		: "memory", "cc");
> +
> +	if (rax != MKTME_PROG_SUCCESS)
> +		WRITE_ONCE(info->status, rax);
> +}
> +
> +/*
> + * MKTME keys are managed as a system-wide single pool of keys.
> + * In the hardware, each physical package maintains a separate key
> + * table. Program all physical packages with the same key info to
> + * maintain that system-wide kernel view.
> + */
> +static inline int mktme_key_program(struct mktme_key_program
> *key_program,
> +				    cpumask_var_t mktme_cpumask)
> +{
> +	struct mktme_key_program_info info = {
> +		.key_program = key_program,
> +		.status = MKTME_PROG_SUCCESS,
> +	};
> +
>  	if (!pconfig_target_supported(MKTME_TARGET))
>  		return -ENXIO;
> 
> -	asm volatile(PCONFIG
> -		: "=a" (rax), "=b" (key_program)
> -		: "0" (rax), "1" (key_program)
> -		: "memory", "cc");
> +	get_online_cpus();
> +	on_each_cpu_mask(mktme_cpumask, mktme_package_program,
> +			 &info, 1);
> +	put_online_cpus();

What is the value of 'mktme_cpumask'? Does it only contain one core for each package? 

Since we are using 'on_each_cpu_mask', I think  we should make sure only one core is set for each node in 'mktme_cpumask'. Otherwise we have to deal with 'DEVICE_BUSY' case, since if one core is already in middle of PCONFIG, the other PCONFIGs on the same node would get 'DEVICE_BUSY' error, but this doesn't mean PCONFIG has failed on that node.

Thanks,
-Kai
> 
> -	return rax;
> +	return info.status;
>  }
> 
>  #endif	/* _ASM_X86_INTEL_PCONFIG_H */
> --
> 2.14.1

RE: [RFC 04/12] x86/mm: Add helper functions to manage memory encryption keys

[Huang, Kai]

> -----Original Message-----
> From: owner-linux-security-module@vger.kernel.org [mailto:owner-linux-
> security-module@vger.kernel.org] On Behalf Of Alison Schofield
> Sent: Saturday, September 8, 2018 10:36 AM
> To: dhowells@redhat.com; tglx@linutronix.de
> Cc: Huang, Kai <kai.huang@intel.com>; Nakajima, Jun
> <jun.nakajima@intel.com>; Shutemov, Kirill <kirill.shutemov@intel.com>;
> Hansen, Dave <dave.hansen@intel.com>; Sakkinen, Jarkko
> <jarkko.sakkinen@intel.com>; jmorris@namei.org; keyrings@vger.kernel.org;
> linux-security-module@vger.kernel.org; mingo@redhat.com; hpa@zytor.com;
> x86@kernel.org; linux-mm@kvack.org
> Subject: [RFC 04/12] x86/mm: Add helper functions to manage memory
> encryption keys
> 
> Define a global mapping structure to track the mapping of userspace keys to
> hardware keyids in MKTME (Multi-Key Total Memory Encryption).
> This data will be used for the memory encryption system call and the kernel key
> service API.
> 
> Implement helper functions to access this mapping structure and make them
> visible to the MKTME Kernel Key Service: security/keys/mktme_keys
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>  arch/x86/include/asm/mktme.h | 11 ++++++
>  arch/x86/mm/mktme.c          | 85
> ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 96 insertions(+)

Maybe it's better to put those changes to include/keys/mktme-type.h, and security/keys/mktme_key.c? It seems you don't have to involve linux-mm and x86 guys by doing so?

Thanks,
-Kai
> 
> diff --git a/arch/x86/include/asm/mktme.h b/arch/x86/include/asm/mktme.h
> index dbfbd955da98..f6acd551457f 100644
> --- a/arch/x86/include/asm/mktme.h
> +++ b/arch/x86/include/asm/mktme.h
> @@ -13,6 +13,17 @@ extern phys_addr_t mktme_keyid_mask;  extern int
> mktme_nr_keyids;  extern int mktme_keyid_shift;
> 
> +/* Manage mappings between hardware keyids and userspace keys */ extern
> +int mktme_map_alloc(void); extern void mktme_map_free(void); extern
> +void mktme_map_lock(void); extern void mktme_map_unlock(void); extern
> +int mktme_map_get_free_keyid(void); extern void
> +mktme_map_clear_keyid(int keyid); extern void mktme_map_set_keyid(int
> +keyid, unsigned int serial); extern int
> +mktme_map_keyid_from_serial(unsigned int serial); extern unsigned int
> +mktme_map_serial_from_keyid(int keyid);
> +
>  extern struct page_ext_operations page_mktme_ops;
> 
>  #define page_keyid page_keyid
> diff --git a/arch/x86/mm/mktme.c b/arch/x86/mm/mktme.c index
> 660caf6a5ce1..5246d8323359 100644
> --- a/arch/x86/mm/mktme.c
> +++ b/arch/x86/mm/mktme.c
> @@ -63,6 +63,91 @@ int vma_keyid(struct vm_area_struct *vma)
>  	return (prot & mktme_keyid_mask) >> mktme_keyid_shift;  }
> 
> +/*
> + * struct mktme_mapping and the mktme_map_* functions manage the
> +mapping
> + * of userspace keys to hardware keyids in MKTME. They are used by the
> + * the encrypt_mprotect system call and the MKTME Key Service API.
> + */
> +struct mktme_mapping {
> +	struct mutex	lock;		/* protect this map & HW state */
> +	unsigned int	mapped_keyids;
> +	unsigned int	serial[];
> +};
> +
> +struct mktme_mapping *mktme_map;
> +
> +static inline long mktme_map_size(void) {
> +	long size = 0;
> +
> +	size += sizeof(mktme_map);
> +	size += sizeof(mktme_map->serial[0]) * mktme_nr_keyids;
> +	return size;
> +}
> +
> +int mktme_map_alloc(void)
> +{
> +	mktme_map = kzalloc(mktme_map_size(), GFP_KERNEL);
> +	if (!mktme_map)
> +		return 0;
> +	mutex_init(&mktme_map->lock);
> +	return 1;
> +}
> +
> +void mktme_map_free(void)
> +{
> +	kfree(mktme_map);
> +}
> +
> +void mktme_map_lock(void)
> +{
> +	mutex_lock(&mktme_map->lock);
> +}
> +
> +void mktme_map_unlock(void)
> +{
> +	mutex_unlock(&mktme_map->lock);
> +}
> +
> +void mktme_map_set_keyid(int keyid, unsigned int serial) {
> +	mktme_map->serial[keyid] = serial;
> +	mktme_map->mapped_keyids++;
> +}
> +
> +void mktme_map_clear_keyid(int keyid)
> +{
> +	mktme_map->serial[keyid] = 0;
> +	mktme_map->mapped_keyids--;
> +}
> +
> +unsigned int mktme_map_serial_from_keyid(int keyid) {
> +	return mktme_map->serial[keyid];
> +}
> +
> +int mktme_map_keyid_from_serial(unsigned int serial) {
> +	int i;
> +
> +	for (i = 1; i < mktme_nr_keyids; i++)
> +		if (mktme_map->serial[i] == serial)
> +			return i;
> +	return 0;
> +}
> +
> +int mktme_map_get_free_keyid(void)
> +{
> +	int i;
> +
> +	if (mktme_map->mapped_keyids < mktme_nr_keyids) {
> +		for (i = 1; i < mktme_nr_keyids; i++)
> +			if (mktme_map->serial[i] == 0)
> +				return i;
> +	}
> +	return 0;
> +}
> +
>  void prep_encrypted_page(struct page *page, int order, int keyid, bool zero)  {
>  	int i;
> --
> 2.14.1

RE: [RFC 07/12] x86/mm: Add helper functions to track encrypted VMA's

[Huang, Kai]

> -----Original Message-----
> From: keyrings-owner@vger.kernel.org [mailto:keyrings-
> owner@vger.kernel.org] On Behalf Of Alison Schofield
> Sent: Saturday, September 8, 2018 10:37 AM
> To: dhowells@redhat.com; tglx@linutronix.de
> Cc: Huang, Kai <kai.huang@intel.com>; Nakajima, Jun
> <jun.nakajima@intel.com>; Shutemov, Kirill <kirill.shutemov@intel.com>;
> Hansen, Dave <dave.hansen@intel.com>; Sakkinen, Jarkko
> <jarkko.sakkinen@intel.com>; jmorris@namei.org; keyrings@vger.kernel.org;
> linux-security-module@vger.kernel.org; mingo@redhat.com; hpa@zytor.com;
> x86@kernel.org; linux-mm@kvack.org
> Subject: [RFC 07/12] x86/mm: Add helper functions to track encrypted VMA's
> 
> In order to safely manage the usage of memory encryption keys, VMA's using
> each keyid need to be tracked. This tracking allows the Kernel Key Service to
> know when the keyid resource is actually in use, or when it is idle and may be
> considered for reuse.
> 
> Define a global atomic encrypt_count array to track the number of VMA's
> oustanding for each encryption keyid.
> 
> Implement helper functions to manipulate this encrypt_count array.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>  arch/x86/include/asm/mktme.h |  7 +++++++
>  arch/x86/mm/mktme.c          | 39
> +++++++++++++++++++++++++++++++++++++++
>  include/linux/mm.h           |  2 ++
>  3 files changed, 48 insertions(+)
> 
> diff --git a/arch/x86/include/asm/mktme.h b/arch/x86/include/asm/mktme.h
> index b707f800b68f..5f3fa0c39c1c 100644
> --- a/arch/x86/include/asm/mktme.h
> +++ b/arch/x86/include/asm/mktme.h
> @@ -16,6 +16,13 @@ extern int mktme_keyid_shift;
>  /* Set the encryption keyid bits in a VMA */  extern void
> mprotect_set_encrypt(struct vm_area_struct *vma, int newkeyid);
> 
> +/* Manage the references to outstanding VMA's per encryption key */
> +extern int vma_alloc_encrypt_array(void); extern void
> +vma_free_encrypt_array(void); extern int vma_read_encrypt_ref(int
> +keyid); extern void vma_get_encrypt_ref(struct vm_area_struct *vma);
> +extern void vma_put_encrypt_ref(struct vm_area_struct *vma);
> +
>  /* Manage mappings between hardware keyids and userspace keys */  extern
> int mktme_map_alloc(void);  extern void mktme_map_free(void); diff --git
> a/arch/x86/mm/mktme.c b/arch/x86/mm/mktme.c index
> 5ee7f37e9cd0..5690ef51a79a 100644
> --- a/arch/x86/mm/mktme.c
> +++ b/arch/x86/mm/mktme.c
> @@ -163,6 +163,45 @@ int mktme_map_get_free_keyid(void)
>  	return 0;
>  }
> 
> +/*
> + *  Helper functions manage the encrypt_count[] array that tracks the
> + *  VMA's outstanding for each encryption keyid. The gets & puts are
> + *  used in core mm code that allocates and free's VMA's. The alloc,
> + *  free, and read functions are used by the MKTME key service to
> + *  manage key allocation and programming.
> + */
> +atomic_t *encrypt_count;
> +
> +int vma_alloc_encrypt_array(void)
> +{
> +	encrypt_count = kcalloc(mktme_nr_keyids, sizeof(atomic_t),
> GFP_KERNEL);
> +	if (!encrypt_count)
> +		return -ENOMEM;
> +	return 0;
> +}
> +
> +void vma_free_encrypt_array(void)
> +{
> +	kfree(encrypt_count);
> +}
> +
> +int vma_read_encrypt_ref(int keyid)
> +{
> +	return atomic_read(&encrypt_count[keyid]);
> +}

I think it's better to move above to security/keys/mktme_keys.c w/ appropriate renaming.  

Thanks,
-Kai
> +
> +void vma_get_encrypt_ref(struct vm_area_struct *vma) {
> +	if (vma_keyid(vma))
> +		atomic_inc(&encrypt_count[vma_keyid(vma)]);
> +}
> +
> +void vma_put_encrypt_ref(struct vm_area_struct *vma) {
> +	if (vma_keyid(vma))
> +		atomic_dec(&encrypt_count[vma_keyid(vma)]);
> +}
> +
>  void prep_encrypted_page(struct page *page, int order, int keyid, bool zero)  {
>  	int i;
> diff --git a/include/linux/mm.h b/include/linux/mm.h index
> 0f9422c7841e..b217c699dbab 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2803,6 +2803,8 @@ static inline void setup_nr_node_ids(void) {}  #ifndef
> CONFIG_X86_INTEL_MKTME  static inline void mprotect_set_encrypt(struct
> vm_area_struct *vma,
>  					int newkeyid) {}
> +static inline void vma_get_encrypt_ref(struct vm_area_struct *vma) {}
> +static inline void vma_put_encrypt_ref(struct vm_area_struct *vma) {}
>  #endif /* CONFIG_X86_INTEL_MKTME */
>  #endif /* __KERNEL__ */
>  #endif /* _LINUX_MM_H */
> --
> 2.14.1

RE: [RFC 11/12] keys/mktme: Add a new key service type for memory encryption keys

[Huang, Kai]

> -----Original Message-----
> From: keyrings-owner@vger.kernel.org [mailto:keyrings-
> owner@vger.kernel.org] On Behalf Of Alison Schofield
> Sent: Saturday, September 8, 2018 10:39 AM
> To: dhowells@redhat.com; tglx@linutronix.de
> Cc: Huang, Kai <kai.huang@intel.com>; Nakajima, Jun
> <jun.nakajima@intel.com>; Shutemov, Kirill <kirill.shutemov@intel.com>;
> Hansen, Dave <dave.hansen@intel.com>; Sakkinen, Jarkko
> <jarkko.sakkinen@intel.com>; jmorris@namei.org; keyrings@vger.kernel.org;
> linux-security-module@vger.kernel.org; mingo@redhat.com; hpa@zytor.com;
> x86@kernel.org; linux-mm@kvack.org
> Subject: [RFC 11/12] keys/mktme: Add a new key service type for memory
> encryption keys
> 
> MKTME (Multi-Key Total Memory Encryption) is a technology that allows
> transparent memory encryption in upcoming Intel platforms. MKTME will
> support mulitple encryption domains, each having their own key. The main use
> case for the feature is virtual machine isolation. The API needs the flexibility to
> work for a wide range of uses.
> 
> The MKTME key service type manages the addition and removal of the memory
> encryption keys. It maps software keys to hardware keyids and programs the
> hardware with the user requested encryption options.
> 
> The only supported encryption algorithm is AES-XTS 128.
> 
> The MKTME key service is half of the MKTME API level solution. It pairs with a
> new memory encryption system call: encrypt_mprotect() that uses the keys to
> encrypt memory.
> 
> See Documentation/x86/mktme-keys.txt
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>  arch/x86/Kconfig           |   1 +
>  include/keys/mktme-type.h  |  28 +++++
>  security/keys/Kconfig      |  11 ++
>  security/keys/Makefile     |   1 +
>  security/keys/mktme_keys.c | 278
> +++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 319 insertions(+)
>  create mode 100644 include/keys/mktme-type.h  create mode 100644
> security/keys/mktme_keys.c
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index
> 023a22568c06..50d8aa6a58e9 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1527,6 +1527,7 @@ config X86_INTEL_MKTME
>  	bool "Intel Multi-Key Total Memory Encryption"
>  	select DYNAMIC_PHYSICAL_MASK
>  	select PAGE_EXTENSION
> +	select MKTME_KEYS
>  	depends on X86_64 && CPU_SUP_INTEL
>  	---help---
>  	  Say yes to enable support for Multi-Key Total Memory Encryption.
> diff --git a/include/keys/mktme-type.h b/include/keys/mktme-type.h new file
> mode 100644 index 000000000000..bebe74cb2b51
> --- /dev/null
> +++ b/include/keys/mktme-type.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +/*
> + * Key service for Multi-KEY Total Memory Encryption  */
> +
> +#ifndef _KEYS_MKTME_TYPE_H
> +#define _KEYS_MKTME_TYPE_H
> +
> +#include <linux/key.h>
> +
> +/*
> + * The AES-XTS 128 encryption algorithm requires 128 bits for each
> + * user supplied option: userkey=, tweak=, entropy=.
> + */
> +#define MKTME_AES_XTS_SIZE	16
> +
> +enum mktme_alg {
> +	MKTME_ALG_AES_XTS_128,
> +};
> +
> +const char *const mktme_alg_names[] = {
> +	[MKTME_ALG_AES_XTS_128]	= "aes_xts_128",
> +};
> +
> +extern struct key_type key_type_mktme;
> +
> +#endif /* _KEYS_MKTME_TYPE_H */
> diff --git a/security/keys/Kconfig b/security/keys/Kconfig index
> 6462e6654ccf..c36972113e67 100644
> --- a/security/keys/Kconfig
> +++ b/security/keys/Kconfig
> @@ -101,3 +101,14 @@ config KEY_DH_OPERATIONS
>  	 in the kernel.
> 
>  	 If you are unsure as to whether this is required, answer N.
> +
> +config MKTME_KEYS
> +	bool "Multi-Key Total Memory Encryption Keys"
> +	depends on KEYS && X86_INTEL_MKTME
> +	help
> +	  This option provides support for Multi-Key Total Memory
> +	  Encryption (MKTME) on Intel platforms offering the feature.
> +	  MKTME allows userspace to manage the hardware encryption
> +	  keys through the kernel key services.
> +
> +	  If you are unsure as to whether this is required, answer N.
> diff --git a/security/keys/Makefile b/security/keys/Makefile index
> ef1581b337a3..2d9f9a82cb8a 100644
> --- a/security/keys/Makefile
> +++ b/security/keys/Makefile
> @@ -29,3 +29,4 @@ obj-$(CONFIG_KEY_DH_OPERATIONS) += dh.o
>  obj-$(CONFIG_BIG_KEYS) += big_key.o
>  obj-$(CONFIG_TRUSTED_KEYS) += trusted.o
>  obj-$(CONFIG_ENCRYPTED_KEYS) += encrypted-keys/
> +obj-$(CONFIG_MKTME_KEYS) += mktme_keys.o
> diff --git a/security/keys/mktme_keys.c b/security/keys/mktme_keys.c new file
> mode 100644 index 000000000000..dcbce7194647
> --- /dev/null
> +++ b/security/keys/mktme_keys.c
> @@ -0,0 +1,278 @@
> +// SPDX-License-Identifier: GPL-3.0
> +
> +/* Documentation/x86/mktme-keys.txt */
> +
> +#include <linux/cred.h>
> +#include <linux/cpu.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/key.h>
> +#include <linux/key-type.h>
> +#include <linux/init.h>
> +#include <linux/parser.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> +#include <asm/intel_pconfig.h>
> +#include <asm/mktme.h>
> +#include <keys/mktme-type.h>
> +#include <keys/user-type.h>
> +
> +#include "internal.h"
> +
> +struct kmem_cache *mktme_prog_cache;	/* hardware programming
> struct */
> +cpumask_var_t mktme_cpumask;		/* one cpu per pkg to program
> keys */

Oh the 'mktme_cpumask' is here. Sorry I didn't notice when replying to your patch 10. :)

But I think you can just move what you did in patch 10 here and leave intel_pconfig.h unchanged. It's much clearer. 

> +
> +static const char * const mktme_program_err[] = {
> +	"KeyID was successfully programmed",	/* 0 */
> +	"Invalid KeyID programming command",	/* 1 */
> +	"Insufficient entropy",			/* 2 */
> +	"KeyID not valid",			/* 3 */
> +	"Invalid encryption algorithm chosen",	/* 4 */
> +	"Failure to access key table",		/* 5 */
> +};
> +
> +/* If a key is available, program and add the key to the software map.
> +*/ static int mktme_program_key(key_serial_t serial,
> +			     struct mktme_key_program *kprog) {
> +	int keyid, ret;
> +
> +	keyid = mktme_map_get_free_keyid();
> +	if (keyid == 0)
> +		return -EDQUOT;
> +
> +	kprog->keyid = keyid;
> +	ret = mktme_key_program(kprog, mktme_cpumask);
> +	if (ret == MKTME_PROG_SUCCESS)
> +		mktme_map_set_keyid(keyid, serial);
> +	else
> +		pr_debug("mktme: %s [%d]\n", mktme_program_err[ret], ret);
> +
> +	return ret;
> +}
> +
> +enum mktme_opt_id	{
> +	OPT_ERROR = -1,
> +	OPT_USERKEY,
> +	OPT_TWEAK,
> +	OPT_ENTROPY,
> +	OPT_ALGORITHM,
> +};
> +
> +static const match_table_t mktme_token = {
> +	{OPT_USERKEY, "userkey=%s"},
> +	{OPT_TWEAK, "tweak=%s"},
> +	{OPT_ENTROPY, "entropy=%s"},
> +	{OPT_ALGORITHM, "algorithm=%s"},
> +	{OPT_ERROR, NULL}
> +
> +};
> +
> +/*
> + * Algorithm AES-XTS 128 is the only supported encryption algorithm.
> + * CPU Generated Key: requires user supplied entropy and accepts no
> + *		      other options.
> + * User Supplied Key: requires user supplied tweak key and accepts
> + *		      no other options.
> + */
> +static int mktme_check_options(struct mktme_key_program *kprog,
> +			       unsigned long token_mask)
> +{
> +	if (!token_mask)
> +		return -EINVAL;
> +
> +	kprog->keyid_ctrl |= MKTME_AES_XTS_128;
> +
> +	if (!test_bit(OPT_USERKEY, &token_mask)) {
> +		if ((!test_bit(OPT_ENTROPY, &token_mask)) ||
> +		    (test_bit(OPT_TWEAK, &token_mask)))
> +			return -EINVAL;
> +
> +		kprog->keyid_ctrl |= MKTME_KEYID_SET_KEY_RANDOM;
> +	}
> +	if (test_bit(OPT_USERKEY, &token_mask)) {
> +		if ((test_bit(OPT_ENTROPY, &token_mask)) ||
> +		    (!test_bit(OPT_TWEAK, &token_mask)))
> +			return -EINVAL;
> +
> +		kprog->keyid_ctrl |= MKTME_KEYID_SET_KEY_DIRECT;
> +	}
> +	return 0;
> +}
> +
> +/*
> + * Parse the options and begin to fill in the key programming struct kprog.
> + * Check the lengths of incoming data and push data directly into kprog fields.
> + */
> +static int mktme_get_options(char *options, struct mktme_key_program
> +*kprog) {
> +	int len = MKTME_AES_XTS_SIZE / 2;
> +	substring_t args[MAX_OPT_ARGS];
> +	unsigned long token_mask = 0;
> +	enum mktme_alg alg;
> +	char *p = options;
> +	int ret, token;
> +
> +	while ((p = strsep(&options, " \t"))) {
> +		if (*p == '\0' || *p == ' ' || *p == '\t')
> +			continue;
> +		token = match_token(p, mktme_token, args);
> +		if (test_and_set_bit(token, &token_mask))
> +			return -EINVAL;
> +
> +		switch (token) {
> +		case OPT_USERKEY:
> +			if (strlen(args[0].from) != MKTME_AES_XTS_SIZE)
> +				return -EINVAL;
> +			ret = hex2bin(kprog->key_field_1, args[0].from, len);
> +			if (ret < 0)
> +				return -EINVAL;
> +			break;
> +
> +		case OPT_TWEAK:
> +			if (strlen(args[0].from) != MKTME_AES_XTS_SIZE)
> +				return -EINVAL;
> +			ret = hex2bin(kprog->key_field_2, args[0].from, len);
> +			if (ret < 0)
> +				return -EINVAL;
> +			break;
> +
> +		case OPT_ENTROPY:
> +			if (strlen(args[0].from) != MKTME_AES_XTS_SIZE)
> +				return -EINVAL;
> +			/* Applied to both CPU-generated data and tweak keys
> */
> +			ret = hex2bin(kprog->key_field_1, args[0].from, len);
> +			ret = hex2bin(kprog->key_field_2, args[0].from, len);
> +			if (ret < 0)
> +				return -EINVAL;
> +			break;

I replied w/ some comments in patch 1 (Document part). Do you have any particular reason to introduce OPT_ENTROPY, while we can simply use OPT_USERKEY and OPT_TWEAK?

Actually I think it might be better that we disallow or ignore OPT_USERKEY, OPT_TWEAK (or OPT_ENTROPY in your patch). Please see my reply to your patch 1.

> +
> +		case OPT_ALGORITHM:
> +			alg = match_string(mktme_alg_names,
> +					   ARRAY_SIZE(mktme_alg_names),
> +					   args[0].from);
> +			if (alg != MKTME_ALG_AES_XTS_128)
> +				return -EINVAL;
> +			break;
> +
> +		default:
> +			return -EINVAL;
> +		}
> +	}
> +	return mktme_check_options(kprog, token_mask); }
> +
> +/* Key Service Command: Creates a software key and programs hardware */
> +int mktme_instantiate(struct key *key, struct key_preparsed_payload
> +*prep) {
> +	struct mktme_key_program *kprog = NULL;
> +	size_t datalen = prep->datalen;
> +	char *options;
> +	int ret = 0;
> +
> +	if (!capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
> +		return -EACCES;
> +
> +	if (datalen <= 0 || datalen > 1024 || !prep->data)
> +		return -EINVAL;
> +
> +	options = kmemdup(prep->data, datalen + 1, GFP_KERNEL);
> +	if (!options)
> +		return -ENOMEM;
> +
> +	options[datalen] = '\0';
> +
> +	kprog = kmem_cache_zalloc(mktme_prog_cache, GFP_KERNEL);
> +	if (!kprog) {
> +		kzfree(options);
> +		return -ENOMEM;
> +	}
> +	ret = mktme_get_options(options, kprog);
> +	if (ret < 0)
> +		goto out;
> +
> +	mktme_map_lock();
> +	ret = mktme_program_key(key->serial, kprog);
> +	mktme_map_unlock();
> +out:
> +	kzfree(options);
> +	kmem_cache_free(mktme_prog_cache, kprog);
> +	return ret;
> +}
> +
> +struct key_type key_type_mktme = {
> +	.name = "mktme",
> +	.instantiate = mktme_instantiate,
> +	.describe = user_describe,
> +};
> +
> +/*
> + * Build mktme_cpumask to include one cpu per physical package.
> + * The mask is used in mktme_key_program() when the hardware key
> + * table is programmed on a per package basis.
> + */
> +static int mktme_build_cpumask(void)
> +{
> +	int online_cpu, mktme_cpu;
> +	int online_pkgid, mktme_pkgid = -1;
> +
> +	if (!zalloc_cpumask_var(&mktme_cpumask, GFP_KERNEL))
> +		return -ENOMEM;
> +
> +	for_each_online_cpu(online_cpu) {
> +		online_pkgid = topology_physical_package_id(online_cpu);
> +
> +		for_each_cpu(mktme_cpu, mktme_cpumask) {
> +			mktme_pkgid =
> topology_physical_package_id(mktme_cpu);
> +			if (mktme_pkgid == online_pkgid)
> +				break;
> +		}
> +		if (mktme_pkgid != online_pkgid)
> +			cpumask_set_cpu(online_cpu, mktme_cpumask);
> +	}

Could we use 'for_each_online_node', 'cpumask_first/next', etc to simplify the logic?

> +	return 0;
> +}
> +
> +/*
> + * Allocate the global key map structure based on the available keyids
> + * at boot time. Create a cache and a cpu_mask to use for programming
> + * the hardware. Initialize the encrypt_count array to track VMA's per
> + * keyid. Once all that succeeds, register the 'mktme' key type.
> + */
> +static int __init init_mktme(void)
> +{
> +	int ret;
> +
> +	/* Verify keys are present */
> +	if (!(mktme_nr_keyids > 0))
> +		return -EINVAL;
> +
> +	if (!mktme_map_alloc())
> +		return -ENOMEM;
> +
> +	mktme_prog_cache = KMEM_CACHE(mktme_key_program,
> SLAB_PANIC);
> +	if (!mktme_prog_cache)
> +		goto free_map;
> +
> +	if (vma_alloc_encrypt_array() < 0)
> +		goto free_cache;

I think it's better to move 'vma_alloc_encrypt_array' part to this patch. Please see my reply to your patch 7.

I also think we should avoid adding new staff to arch/x86/include/asm/mktme.h since it is included by some basic page table manipulation header files. Adding mm related structure to asm/mktme.h sometimes may fail to compile kernel in my experience.

Thanks,
-Kai
> +
> +	if (mktme_build_cpumask() < 0)
> +		goto free_array;
> +
> +	ret = register_key_type(&key_type_mktme);
> +	if (!ret)
> +		return ret;
> +
> +	free_cpumask_var(mktme_cpumask);
> +free_array:
> +	vma_free_encrypt_array();
> +free_cache:
> +	kmem_cache_destroy(mktme_prog_cache);
> +free_map:
> +	mktme_map_free();
> +
> +	return -ENOMEM;
> +}
> +
> +late_initcall(init_mktme);
> --
> 2.14.1

Re: [RFC 02/12] mm: Generalize the mprotect implementation to support extensions

[Jarkko Sakkinen]

On Fri, 2018-09-07 at 15:34 -0700, Alison Schofield wrote:
> Today mprotect is implemented to support legacy mprotect behavior
> plus an extension for memory protection keys. Make it more generic
> so that it can support additional extensions in the future.
> 
> This is done is preparation for adding a new system call for memory
> encyption keys. The intent is that the new encrypted mprotect will be
> another extension to legacy mprotect.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>  mm/mprotect.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/mprotect.c b/mm/mprotect.c
> index 68dc476310c0..56e64ef7931e 100644
> --- a/mm/mprotect.c
> +++ b/mm/mprotect.c
> @@ -35,6 +35,8 @@
>  
>  #include "internal.h"
>  
> +#define NO_PKEY  -1

This commit does not make anything more generic but it does take
away a magic number. The code change is senseful. The commit
message is nonsense.

PS. Please use @linux.intel.com for LKML.

/Jarkko

Re: [RFC 00/12] Multi-Key Total Memory Encryption API (MKTME)

[Sakkinen, Jarkko]

On Fri, 2018-09-07 at 15:23 -0700, Alison Schofield wrote:
> Seeking comments on the APIs supporting MKTME on future Intel platforms.
> 
> MKTME (Multi-Key Total Memory Encryption) is a technology supporting
> memory encryption on upcoming Intel platforms. Whereas TME allows
> encryption of the entire system memory using a single key, MKTME
> allows mulitple encryption domains, each having their own key. While 
> the main use case for the feature is virtual machine isolation, the
> API needs the flexibility to work for a wide range of use cases.

Is it a common knowledge what TME is? I kind of dropped out of the wagon
in the 2nd sentence of this paragraph.

/Jarkko

Re: [RFC 01/12] docs/x86: Document the Multi-Key Total Memory Encryption API

[Sakkinen, Jarkko]

On Fri, 2018-09-07 at 15:34 -0700, Alison Schofield wrote:
> Document the API's used for MKTME on Intel platforms.
> MKTME: Multi-KEY Total Memory Encryption
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>  Documentation/x86/mktme-keys.txt | 153
> +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 153 insertions(+)
>  create mode 100644 Documentation/x86/mktme-keys.txt
> 
> diff --git a/Documentation/x86/mktme-keys.txt b/Documentation/x86/mktme-
> keys.txt
> new file mode 100644
> index 000000000000..2dea7acd2a17
> --- /dev/null
> +++ b/Documentation/x86/mktme-keys.txt
> @@ -0,0 +1,153 @@
> +MKTME (Multi-Key Total Memory Encryption) is a technology that allows
> +memory encryption on Intel platforms. Whereas TME (Total Memory Encryption)
> +allows encryption of the entire system memory using a single key, MKTME
> +allows multiple encryption domains, each having their own key. The main use
> +case for the feature is virtual machine isolation. The API's introduced here
> +are intended to offer flexibility to work in a wide range of uses.
> +
> +The externally available Intel Architecture Spec:
> +https://software.intel.com/sites/default/files/managed/a5/16/Multi-Key-Total-
> Memory-Encryption-Spec.pdf
> +
> +============================  API Overview  ============================
> +
> +There are 2 MKTME specific API's that enable userspace to create and use
> +the memory encryption keys:

This is like saying that they are different APIs to do semantically the
same exact thing. Is that so?

/Jarkko

Re: [RFC 04/12] x86/mm: Add helper functions to manage memory encryption keys

[Sakkinen, Jarkko]

On Fri, 2018-09-07 at 15:36 -0700, Alison Schofield wrote:
> Define a global mapping structure to track the mapping of userspace
> keys to hardware keyids in MKTME (Multi-Key Total Memory Encryption).
> This data will be used for the memory encryption system call and the
> kernel key service API.

This is also for earlier patches. I kind of feel uncomfortable because
I can neither TME nor MKTME how they reside in the chipset/board. Are
they inside the CPU package? Or is it perhaps a separate chip outside
the CPU? I would guess not but the problem is that I have to guess in
the first place.

> Implement helper functions to access this mapping structure and make
> them visible to the MKTME Kernel Key Service: security/keys/mktme_keys
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>  arch/x86/include/asm/mktme.h | 11 ++++++
>  arch/x86/mm/mktme.c          | 85
> ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 96 insertions(+)
> 
> diff --git a/arch/x86/include/asm/mktme.h b/arch/x86/include/asm/mktme.h
> index dbfbd955da98..f6acd551457f 100644
> --- a/arch/x86/include/asm/mktme.h
> +++ b/arch/x86/include/asm/mktme.h
> @@ -13,6 +13,17 @@ extern phys_addr_t mktme_keyid_mask;
>  extern int mktme_nr_keyids;
>  extern int mktme_keyid_shift;
>  
> +/* Manage mappings between hardware keyids and userspace keys */
> +extern int mktme_map_alloc(void);
> +extern void mktme_map_free(void);
> +extern void mktme_map_lock(void);
> +extern void mktme_map_unlock(void);
> +extern int mktme_map_get_free_keyid(void);
> +extern void mktme_map_clear_keyid(int keyid);
> +extern void mktme_map_set_keyid(int keyid, unsigned int serial);
> +extern int mktme_map_keyid_from_serial(unsigned int serial);
> +extern unsigned int mktme_map_serial_from_keyid(int keyid);
> +
>  extern struct page_ext_operations page_mktme_ops;
>  
>  #define page_keyid page_keyid
> diff --git a/arch/x86/mm/mktme.c b/arch/x86/mm/mktme.c
> index 660caf6a5ce1..5246d8323359 100644
> --- a/arch/x86/mm/mktme.c
> +++ b/arch/x86/mm/mktme.c
> @@ -63,6 +63,91 @@ int vma_keyid(struct vm_area_struct *vma)
>  	return (prot & mktme_keyid_mask) >> mktme_keyid_shift;
>  }
>  
> +/*
> + * struct mktme_mapping and the mktme_map_* functions manage the mapping
> + * of userspace keys to hardware keyids in MKTME. They are used by the
> + * the encrypt_mprotect system call and the MKTME Key Service API.
> + */
> +struct mktme_mapping {
> +	struct mutex	lock;		/* protect this map & HW
> state */
> +	unsigned int	mapped_keyids;
> +	unsigned int	serial[];
> +};
> +
> +struct mktme_mapping *mktme_map;
> +
> +static inline long mktme_map_size(void)
> +{
> +	long size = 0;
> +
> +	size += sizeof(mktme_map);
> +	size += sizeof(mktme_map->serial[0]) * mktme_nr_keyids;
> +	return size;
> +}
> +
> +int mktme_map_alloc(void)
> +{
> +	mktme_map = kzalloc(mktme_map_size(), GFP_KERNEL);
> +	if (!mktme_map)
> +		return 0;
> +	mutex_init(&mktme_map->lock);
> +	return 1;
> +}
> +
> +void mktme_map_free(void)
> +{
> +	kfree(mktme_map);
> +}
> +
> +void mktme_map_lock(void)
> +{
> +	mutex_lock(&mktme_map->lock);
> +}
> +
> +void mktme_map_unlock(void)
> +{
> +	mutex_unlock(&mktme_map->lock);
> +}
> +
> +void mktme_map_set_keyid(int keyid, unsigned int serial)
> +{
> +	mktme_map->serial[keyid] = serial;
> +	mktme_map->mapped_keyids++;
> +}
> +
> +void mktme_map_clear_keyid(int keyid)
> +{
> +	mktme_map->serial[keyid] = 0;
> +	mktme_map->mapped_keyids--;
> +}
> +
> +unsigned int mktme_map_serial_from_keyid(int keyid)
> +{
> +	return mktme_map->serial[keyid];
> +}
> +
> +int mktme_map_keyid_from_serial(unsigned int serial)
> +{
> +	int i;
> +
> +	for (i = 1; i < mktme_nr_keyids; i++)
> +		if (mktme_map->serial[i] == serial)
> +			return i;
> +	return 0;
> +}
> +
> +int mktme_map_get_free_keyid(void)
> +{
> +	int i;
> +
> +	if (mktme_map->mapped_keyids < mktme_nr_keyids) {
> +		for (i = 1; i < mktme_nr_keyids; i++)
> +			if (mktme_map->serial[i] == 0)
> +				return i;
> +	}
> +	return 0;
> +}


The kdoc header is missing from all above.

/Jarkko

Re: [RFC 05/12] x86/mm: Add a helper function to set keyid bits in encrypted VMA's

[Sakkinen, Jarkko]

On Fri, 2018-09-07 at 15:36 -0700, Alison Schofield wrote:
> Store the memory encryption keyid in the upper bits of vm_page_prot
> that match position of keyid, bits 51:46, in a PTE.

Would not do bad to explain the context a bit here. At least I do not
know why you ended up to this bit range.

/Jarkko

Re: [RFC 06/12] mm: Add the encrypt_mprotect() system call

[Jarkko Sakkinen]

On Fri, 2018-09-07 at 15:36 -0700, Alison Schofield wrote:
> Implement memory encryption with a new system call that is an
> extension of the legacy mprotect() system call.
> 
> In encrypt_mprotect the caller must pass a handle to a previously
> allocated and programmed encryption key. Validate the key and store
> the keyid bits in the vm_page_prot for each VMA in the protection
> range.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>  fs/exec.c           |  4 ++--
>  include/linux/key.h |  2 ++
>  include/linux/mm.h  |  3 ++-
>  mm/mprotect.c       | 67 ++++++++++++++++++++++++++++++++++++++++++++++++--
> ---
>  4 files changed, 67 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/exec.c b/fs/exec.c
> index a1a246062561..b681a413db9c 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -754,8 +754,8 @@ int setup_arg_pages(struct linux_binprm *bprm,
>  	vm_flags |= mm->def_flags;
>  	vm_flags |= VM_STACK_INCOMPLETE_SETUP;
>  
> -	ret = mprotect_fixup(vma, &prev, vma->vm_start, vma->vm_end,
> -			vm_flags);
> +	ret = mprotect_fixup(vma, &prev, vma->vm_start, vma->vm_end,
> vm_flags,
> +			     -1);

Why you pass a magic number here when you went the trouble having
a named constant?

>  	if (ret)
>  		goto out_unlock;
>  	BUG_ON(prev != vma);
> diff --git a/include/linux/key.h b/include/linux/key.h
> index e58ee10f6e58..fb8a7d5f6149 100644
> --- a/include/linux/key.h
> +++ b/include/linux/key.h
> @@ -346,6 +346,8 @@ static inline key_serial_t key_serial(const struct key
> *key)
>  
>  extern void key_set_timeout(struct key *, unsigned);
>  
> +extern key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
> +				 key_perm_t perm);
>  /*
>   * The permissions required on a key that we're looking up.
>   */
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index ac85c0805761..0f9422c7841e 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1579,7 +1579,8 @@ extern unsigned long change_protection(struct
> vm_area_struct *vma, unsigned long
>  			      int dirty_accountable, int prot_numa);
>  extern int mprotect_fixup(struct vm_area_struct *vma,
>  			  struct vm_area_struct **pprev, unsigned long start,
> -			  unsigned long end, unsigned long newflags);
> +			  unsigned long end, unsigned long newflags,
> +			  int newkeyid);
>  
>  /*
>   * doesn't attempt to fault and will return short.
> diff --git a/mm/mprotect.c b/mm/mprotect.c
> index 56e64ef7931e..6c2e1106525c 100644
> --- a/mm/mprotect.c
> +++ b/mm/mprotect.c
> @@ -28,14 +28,17 @@
>  #include <linux/ksm.h>
>  #include <linux/uaccess.h>
>  #include <linux/mm_inline.h>
> +#include <linux/key.h>
>  #include <asm/pgtable.h>
>  #include <asm/cacheflush.h>
>  #include <asm/mmu_context.h>
>  #include <asm/tlbflush.h>
> +#include <asm/mktme.h>
>  
>  #include "internal.h"
>  
>  #define NO_PKEY  -1
> +#define NO_KEYID -1

Should have only single named constant IMHO. This ambiguity
is worse than some reasonable constant name for both cases.
Maybe NO_KEYID would be adequate?

>  
>  static unsigned long change_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
>  		unsigned long addr, unsigned long end, pgprot_t newprot,
> @@ -310,7 +313,8 @@ unsigned long change_protection(struct vm_area_struct
> *vma, unsigned long start,
>  
>  int
>  mprotect_fixup(struct vm_area_struct *vma, struct vm_area_struct **pprev,
> -	unsigned long start, unsigned long end, unsigned long newflags)
> +	       unsigned long start, unsigned long end, unsigned long
> newflags,
> +	       int newkeyid)
>  {
>  	struct mm_struct *mm = vma->vm_mm;
>  	unsigned long oldflags = vma->vm_flags;
> @@ -320,10 +324,24 @@ mprotect_fixup(struct vm_area_struct *vma, struct
> vm_area_struct **pprev,
>  	int error;
>  	int dirty_accountable = 0;
>  
> +	/*
> +	 * Flags match and Keyids match or we have NO_KEYID.
> +	 * This _fixup is usually called from do_mprotect_ext() except
> +	 * for one special case: caller fs/exec.c/setup_arg_pages()
> +	 * In that case, newkeyid is passed as -1 (NO_KEYID).
> +	 */
> +	if (newflags == oldflags &&
> +	    (newkeyid == vma_keyid(vma) || newkeyid == NO_KEYID)) {
> +		*pprev = vma;
> +		return 0;
> +	}
> +	/* Flags match and Keyid changes */
>  	if (newflags == oldflags) {
> +		mprotect_set_encrypt(vma, newkeyid);
>  		*pprev = vma;
>  		return 0;
>  	}
> +	/* Flags and Keyids both change, continue. */
>  
>  	/*
>  	 * If we make a private mapping writable we increase our commit;
> @@ -373,6 +391,8 @@ mprotect_fixup(struct vm_area_struct *vma, struct
> vm_area_struct **pprev,
>  	}
>  
>  success:
> +	if (newkeyid != NO_KEYID)
> +		mprotect_set_encrypt(vma, newkeyid);
>  	/*
>  	 * vm_flags and vm_page_prot are protected by the mmap_sem
>  	 * held in write mode.
> @@ -404,10 +424,15 @@ mprotect_fixup(struct vm_area_struct *vma, struct
> vm_area_struct **pprev,
>  }
>  
>  /*
> - * When pkey==NO_PKEY we get legacy mprotect behavior here.
> + * do_mprotect_ext() supports the legacy mprotect behavior plus extensions
> + * for protection keys and memory encryption keys. These extensions are
> + * mutually exclusive and the behavior is:
> + *	(pkey==NO_PKEY && keyid==NO_KEYID) ==> legacy mprotect
> + *	(pkey is valid)  ==> legacy mprotect plus protection key extensions
> + *	(keyid is valid) ==> legacy mprotect plus encryption key extensions
>   */

The header does not follow

https://www.kernel.org/doc/Documentation/kernel-doc-nano-HOWTO.txt

>  static int do_mprotect_ext(unsigned long start, size_t len,
> -		unsigned long prot, int pkey)
> +			   unsigned long prot, int pkey, int keyid)
>  {
>  	unsigned long nstart, end, tmp, reqprot;
>  	struct vm_area_struct *vma, *prev;
> @@ -505,7 +530,8 @@ static int do_mprotect_ext(unsigned long start, size_t
> len,
>  		tmp = vma->vm_end;
>  		if (tmp > end)
>  			tmp = end;
> -		error = mprotect_fixup(vma, &prev, nstart, tmp, newflags);
> +		error = mprotect_fixup(vma, &prev, nstart, tmp, newflags,
> +				       keyid);
>  		if (error)
>  			goto out;
>  		nstart = tmp;
> @@ -530,7 +556,7 @@ static int do_mprotect_ext(unsigned long start, size_t
> len,
>  SYSCALL_DEFINE3(mprotect, unsigned long, start, size_t, len,
>  		unsigned long, prot)
>  {
> -	return do_mprotect_ext(start, len, prot, NO_PKEY);
> +	return do_mprotect_ext(start, len, prot, NO_PKEY, NO_KEYID);
>  }
>  
>  #ifdef CONFIG_ARCH_HAS_PKEYS
> @@ -538,7 +564,7 @@ SYSCALL_DEFINE3(mprotect, unsigned long, start, size_t,
> len,
>  SYSCALL_DEFINE4(pkey_mprotect, unsigned long, start, size_t, len,
>  		unsigned long, prot, int, pkey)
>  {
> -	return do_mprotect_ext(start, len, prot, pkey);
> +	return do_mprotect_ext(start, len, prot, pkey, NO_KEYID);
>  }
>  
>  SYSCALL_DEFINE2(pkey_alloc, unsigned long, flags, unsigned long, init_val)
> @@ -587,3 +613,32 @@ SYSCALL_DEFINE1(pkey_free, int, pkey)
>  }
>  
>  #endif /* CONFIG_ARCH_HAS_PKEYS */
> +
> +#ifdef CONFIG_X86_INTEL_MKTME
> +
> +SYSCALL_DEFINE4(encrypt_mprotect, unsigned long, start, size_t, len,
> +		unsigned long, prot, key_serial_t, serial)
> +{
> +	key_ref_t key_ref;
> +	int ret, keyid;
> +
> +	/* TODO MKTME key service must be initialized */
> +
> +	key_ref = lookup_user_key(serial, 0, KEY_NEED_VIEW);
> +	if (IS_ERR(key_ref))
> +		return PTR_ERR(key_ref);
> +
> +	mktme_map_lock();
> +	keyid = mktme_map_keyid_from_serial(serial);
> +	if (!keyid) {
> +		mktme_map_unlock();
> +		key_ref_put(key_ref);
> +		return -EINVAL;
> +	}
> +	ret = do_mprotect_ext(start, len, prot, NO_PKEY, keyid);
> +	mktme_map_unlock();
> +	key_ref_put(key_ref);
> +	return ret;
> +}
> +
> +#endif /* CONFIG_X86_INTEL_MKTME */

/Jarkko

Re: [RFC 08/12] mm: Track VMA's in use for each memory encryption keyid

[Jarkko Sakkinen]

On Fri, 2018-09-07 at 15:37 -0700, Alison Schofield wrote:
> Keep track of the VMA's oustanding for each memory encryption keyid.
> The count is used by the MKTME (Multi-Key Total Memory Encryption)
> Key Service to determine when it is safe to reprogram a hardware
> encryption key.

Maybe a stupid question but why they are tracked and what do you 
mean by tracking?

/Jarkko

> Approach here is to do gets and puts on the encryption reference
> wherever kmem_cache_alloc/free's of vma_area_cachep's are executed.
> A couple of these locations will not be hit until cgroup support is
> added. One of these locations should never hit, so use a VM_WARN_ON.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>  arch/x86/mm/mktme.c |  2 ++
>  kernel/fork.c       |  2 ++
>  mm/mmap.c           | 12 ++++++++++++
>  mm/nommu.c          |  4 ++++
>  4 files changed, 20 insertions(+)
> 
> diff --git a/arch/x86/mm/mktme.c b/arch/x86/mm/mktme.c
> index 5690ef51a79a..8a7c326d4546 100644
> --- a/arch/x86/mm/mktme.c
> +++ b/arch/x86/mm/mktme.c
> @@ -72,10 +72,12 @@ void mprotect_set_encrypt(struct vm_area_struct *vma, int
> newkeyid)
>  	if (newkeyid == oldkeyid)
>  		return;
>  
> +	vma_put_encrypt_ref(vma);
>  	newprot = pgprot_val(vma->vm_page_prot);
>  	newprot &= ~mktme_keyid_mask;
>  	newprot |= (unsigned long)newkeyid << mktme_keyid_shift;
>  	vma->vm_page_prot = __pgprot(newprot);
> +	vma_get_encrypt_ref(vma);
>  }
>  
>  /*
> diff --git a/kernel/fork.c b/kernel/fork.c
> index e5e7a220a124..2d0e507bde7c 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -459,6 +459,7 @@ static __latent_entropy int dup_mmap(struct mm_struct *mm,
>  		if (!tmp)
>  			goto fail_nomem;
>  		*tmp = *mpnt;
> +		vma_get_encrypt_ref(tmp);	/* Track encrypted vma's */
>  		INIT_LIST_HEAD(&tmp->anon_vma_chain);
>  		retval = vma_dup_policy(mpnt, tmp);
>  		if (retval)
> @@ -539,6 +540,7 @@ static __latent_entropy int dup_mmap(struct mm_struct *mm,
>  fail_nomem_anon_vma_fork:
>  	mpol_put(vma_policy(tmp));
>  fail_nomem_policy:
> +	vma_put_encrypt_ref(tmp);		/* Track encrypted vma's */
>  	kmem_cache_free(vm_area_cachep, tmp);
>  fail_nomem:
>  	retval = -ENOMEM;
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 4c604eb644b4..7390b8b69fd6 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -182,6 +182,7 @@ static struct vm_area_struct *remove_vma(struct
> vm_area_struct *vma)
>  	if (vma->vm_file)
>  		fput(vma->vm_file);
>  	mpol_put(vma_policy(vma));
> +	vma_put_encrypt_ref(vma);
>  	kmem_cache_free(vm_area_cachep, vma);
>  	return next;
>  }
> @@ -913,6 +914,7 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long
> start,
>  			anon_vma_merge(vma, next);
>  		mm->map_count--;
>  		mpol_put(vma_policy(next));
> +		vma_put_encrypt_ref(next);
>  		kmem_cache_free(vm_area_cachep, next);
>  		/*
>  		 * In mprotect's case 6 (see comments on vma_merge),
> @@ -1744,6 +1746,7 @@ unsigned long mmap_region(struct file *file, unsigned
> long addr,
>  		goto unacct_error;
>  	}
>  
> +	vma_get_encrypt_ref(vma);
>  	vma->vm_mm = mm;
>  	vma->vm_start = addr;
>  	vma->vm_end = addr + len;
> @@ -1839,6 +1842,7 @@ unsigned long mmap_region(struct file *file, unsigned
> long addr,
>  unmap_and_free_vma:
>  	vma->vm_file = NULL;
>  	fput(file);
> +	vma_put_encrypt_ref(vma);
>  
>  	/* Undo any partial mapping done by a device driver. */
>  	unmap_region(mm, vma, prev, vma->vm_start, vma->vm_end);
> @@ -2653,6 +2657,7 @@ int __split_vma(struct mm_struct *mm, struct
> vm_area_struct *vma,
>  		new->vm_pgoff += ((addr - vma->vm_start) >> PAGE_SHIFT);
>  	}
>  
> +	vma_get_encrypt_ref(new);
>  	err = vma_dup_policy(vma, new);
>  	if (err)
>  		goto out_free_vma;
> @@ -2686,6 +2691,7 @@ int __split_vma(struct mm_struct *mm, struct
> vm_area_struct *vma,
>   out_free_mpol:
>  	mpol_put(vma_policy(new));
>   out_free_vma:
> +	vma_put_encrypt_ref(new);
>  	kmem_cache_free(vm_area_cachep, new);
>  	return err;
>  }
> @@ -3007,6 +3013,7 @@ static int do_brk_flags(unsigned long addr, unsigned
> long len, unsigned long fla
>  		return -ENOMEM;
>  	}
>  
> +	vma_get_encrypt_ref(vma);
>  	INIT_LIST_HEAD(&vma->anon_vma_chain);
>  	vma->vm_mm = mm;
>  	vma->vm_ops = &anon_vm_ops;
> @@ -3229,6 +3236,7 @@ struct vm_area_struct *copy_vma(struct vm_area_struct
> **vmap,
>  		new_vma->vm_pgoff = pgoff;
>  		if (vma_dup_policy(vma, new_vma))
>  			goto out_free_vma;
> +		vma_get_encrypt_ref(new_vma);
>  		INIT_LIST_HEAD(&new_vma->anon_vma_chain);
>  		if (anon_vma_clone(new_vma, vma))
>  			goto out_free_mempol;
> @@ -3243,6 +3251,7 @@ struct vm_area_struct *copy_vma(struct vm_area_struct
> **vmap,
>  
>  out_free_mempol:
>  	mpol_put(vma_policy(new_vma));
> +	vma_put_encrypt_ref(new_vma);
>  out_free_vma:
>  	kmem_cache_free(vm_area_cachep, new_vma);
>  out:
> @@ -3372,6 +3381,9 @@ static struct vm_area_struct *__install_special_mapping(
>  	if (unlikely(vma == NULL))
>  		return ERR_PTR(-ENOMEM);
>  
> +	/* Do not expect a memory encrypted vma here */
> +	VM_WARN_ON(vma_keyid(vma));
> +
>  	INIT_LIST_HEAD(&vma->anon_vma_chain);
>  	vma->vm_mm = mm;
>  	vma->vm_start = addr;
> diff --git a/mm/nommu.c b/mm/nommu.c
> index 73f66e81cfb0..85f04c174638 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -769,6 +769,7 @@ static void delete_vma(struct mm_struct *mm, struct
> vm_area_struct *vma)
>  	if (vma->vm_file)
>  		fput(vma->vm_file);
>  	put_nommu_region(vma->vm_region);
> +	vma_put_encrypt_ref(vma);
>  	kmem_cache_free(vm_area_cachep, vma);
>  }
>  
> @@ -1215,6 +1216,7 @@ unsigned long do_mmap(struct file *file,
>  	if (!vma)
>  		goto error_getting_vma;
>  
> +	vma_get_encrypt_ref(vma);
>  	region->vm_usage = 1;
>  	region->vm_flags = vm_flags;
>  	region->vm_pgoff = pgoff;
> @@ -1375,6 +1377,7 @@ unsigned long do_mmap(struct file *file,
>  	kmem_cache_free(vm_region_jar, region);
>  	if (vma->vm_file)
>  		fput(vma->vm_file);
> +	vma_put_encrypt_ref(vma);
>  	kmem_cache_free(vm_area_cachep, vma);
>  	return ret;
>  
> @@ -1486,6 +1489,7 @@ int split_vma(struct mm_struct *mm, struct
> vm_area_struct *vma,
>  	*new = *vma;
>  	*region = *vma->vm_region;
>  	new->vm_region = region;
> +	vma_get_encrypt_ref(new);
>  
>  	npages = (addr - vma->vm_start) >> PAGE_SHIFT;
>  

Re: [RFC 09/12] mm: Restrict memory encryption to anonymous VMA's

[Sakkinen, Jarkko]

On Fri, 2018-09-07 at 15:37 -0700, Alison Schofield wrote:
> Memory encryption is only supported for mappings that are ANONYMOUS.
> Test the entire range of VMA's in an encrypt_mprotect() request to
> make sure they all meet that requirement before encrypting any.
> 
> The encrypt_mprotect syscall will return -EINVAL and will not encrypt
> any VMA's if this check fails.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>  mm/mprotect.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/mm/mprotect.c b/mm/mprotect.c
> index 6c2e1106525c..3384b755aad1 100644
> --- a/mm/mprotect.c
> +++ b/mm/mprotect.c
> @@ -311,6 +311,24 @@ unsigned long change_protection(struct vm_area_struct
> *vma, unsigned long start,
>  	return pages;
>  }
>  
> +/*
> + * Encrypted mprotect is only supported on anonymous mappings.
> + * All VMA's in the requested range must be anonymous. If this
> + * test fails on any single VMA, the entire mprotect request fails.
> + */

kdoc

> +bool mem_supports_encryption(struct vm_area_struct *vma, unsigned long end)
> +{
> +	struct vm_area_struct *test_vma = vma;
> +
> +	do {
> +		if (!vma_is_anonymous(test_vma))
> +			return false;
> +
> +		test_vma = test_vma->vm_next;
> +	} while (test_vma && test_vma->vm_start < end);
> +	return true;
> +}
> +
>  int
>  mprotect_fixup(struct vm_area_struct *vma, struct vm_area_struct **pprev,
>  	       unsigned long start, unsigned long end, unsigned long
> newflags,
> @@ -491,6 +509,10 @@ static int do_mprotect_ext(unsigned long start, size_t
> len,
>  				goto out;
>  		}
>  	}
> +	if (keyid > 0 && !mem_supports_encryption(vma, end)) {
> +		error = -EINVAL;
> +		goto out;
> +	}
>  	if (start > vma->vm_start)
>  		prev = vma;
>  

/Jarkko

Re: [RFC 10/12] x86/pconfig: Program memory encryption keys on a system-wide basis

[Sakkinen, Jarkko]

On Fri, 2018-09-07 at 15:38 -0700, Alison Schofield wrote:
> The kernel manages the MKTME (Multi-Key Total Memory Encryption) Keys
> as a system wide single pool of keys. The hardware, however, manages
> the keys on a per physical package basis. Each physical package
> maintains a key table that all CPU's in that package share.
> 
> In order to maintain the consistent, system wide view that the kernel
> requires, program all physical packages during a key program request.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>

Just kind of checking that are you talking about multiple cores in
a single package or really multiple packages?

/Jarkko

Re: [RFC 09/12] mm: Restrict memory encryption to anonymous VMA's

[Dave Hansen]

On 09/10/2018 11:21 AM, Sakkinen, Jarkko wrote:
>> +/*
>> + * Encrypted mprotect is only supported on anonymous mappings.
>> + * All VMA's in the requested range must be anonymous. If this
>> + * test fails on any single VMA, the entire mprotect request fails.
>> + */
> kdoc

kdoc what?  You want this comment in kdoc format?  Why?

Re: [RFC 00/12] Multi-Key Total Memory Encryption API (MKTME)

[Alison Schofield]

On Sun, Sep 09, 2018 at 06:10:19PM -0700, Huang, Kai wrote:
> 
> > -----Original Message-----
> > From: keyrings-owner@vger.kernel.org [mailto:keyrings-
> > owner@vger.kernel.org] On Behalf Of Alison Schofield
> > Sent: Saturday, September 8, 2018 10:23 AM
> > To: dhowells@redhat.com; tglx@linutronix.de
> > Cc: Huang, Kai <kai.huang@intel.com>; Nakajima, Jun
> > <jun.nakajima@intel.com>; Shutemov, Kirill <kirill.shutemov@intel.com>;
> > Hansen, Dave <dave.hansen@intel.com>; Sakkinen, Jarkko
> > <jarkko.sakkinen@intel.com>; jmorris@namei.org; keyrings@vger.kernel.org;
> > linux-security-module@vger.kernel.org; mingo@redhat.com; hpa@zytor.com;
> > x86@kernel.org; linux-mm@kvack.org
> > Subject: [RFC 00/12] Multi-Key Total Memory Encryption API (MKTME)
> > 
> > Seeking comments on the APIs supporting MKTME on future Intel platforms.
> > 
> > MKTME (Multi-Key Total Memory Encryption) is a technology supporting
> > memory encryption on upcoming Intel platforms. Whereas TME allows
> > encryption of the entire system memory using a single key, MKTME allows
> > mulitple encryption domains, each having their own key. While the main use
> > case for the feature is virtual machine isolation, the API needs the flexibility to
> > work for a wide range of use cases.
> > 
> > This RFC presents the 2 API additions that enable userspace to:
> >  1) Create Encryption Keys: Kernel Key Service type "mktme"
> >  2) Use the Encryption Keys: system call encrypt_mprotect()
> > 
> > In order to share between: the Kernel Key Service, the new system call, and the
> > existing mm code, helper functions were created in arch/x86/mktme
> 
> IMHO, we can separate this series into 2 parts, as you did above, and send out them separately. The reason is, in general I think adding new MKTME type to key retention services is not that related to memory management code, namely the encrypt_mprotect() API part.
> 
> So if we split the two parts and send them out separately, the first part can be reviewed by keyring and security guys, without involving mm guys, and the encrypt_mprotect() part can be more reviewed more by mm guys. 
> 

Kai,

That was the direction I had in mind at the onset: the MKTME key service
would be one patch(set) and the MKTME encrypt_mprotect() system call would
be delivered in another patch(set).

That separation falls apart when the shared structures and functions are
introduced. That 'mktme_map' (maps userspace keys to hardware keyid slots),
and the 'encrypt_count' array (counts vma's outstanding for each key) need
to be shared by both pieces. These mktme special shared structures and the
functions that operate on them are all defined in arch/x86/mm/mktme.h,.c.
>From there they can be shared with the security/keys/mktme_keys.c

Once I made that separation, I stuck with it. Those structures, and any
functions that manipulate those structures live in arch/x86/mm/mktme.h,c

You noted that some of the functions that operate on the encrypt_count
might not need to be over in arch/x86/mm/mktme.c because they are not used
in the mm code. That is true.  But, then I'd be splitting up the definition
of the struct and the funcs that operate on it. So, I stuck with keeping it
all together in the arch specific mktme files.

Having said all the above, I do welcome other ideas on how to better organize
the code. 

Back to your request- to split it into smaller patchsets might look something
like:
1) the MKTME API helpers
2) the MKTME Key Service
3) the MKTME syscall encrypt_mprotect()

I'm not clear that would make anyones review life easier, than picking
the same pieces out of the greater patchset.

Suggestions welcome,
Alison

> And since encrypt_mprotect() is a new syscall, you may need to add more lists for the review, ie, linux-api, and maybe linux-kernel as well.

Got it. Will include theses in v1.

> 
> Thanks,
> -Kai
> 
> > 
> > This patchset is built upon Kirill Shutemov's patchset for the core MKTME
> > support. You can find that here:
> > git://git.kernel.org/pub/scm/linux/kernel/git/kas/linux.git mktme/wip
> > 
> > 
> > Alison Schofield (12):
> >   docs/x86: Document the Multi-Key Total Memory Encryption API
> >   mm: Generalize the mprotect implementation to support extensions
> >   syscall/x86: Wire up a new system call for memory encryption keys
> >   x86/mm: Add helper functions to manage memory encryption keys
> >   x86/mm: Add a helper function to set keyid bits in encrypted VMA's
> >   mm: Add the encrypt_mprotect() system call
> >   x86/mm: Add helper functions to track encrypted VMA's
> >   mm: Track VMA's in use for each memory encryption keyid
> >   mm: Restrict memory encryption to anonymous VMA's
> >   x86/pconfig: Program memory encryption keys on a system-wide basis
> >   keys/mktme: Add a new key service type for memory encryption keys
> >   keys/mktme: Do not revoke in use memory encryption keys
> > 
> >  Documentation/x86/mktme-keys.txt       | 153 ++++++++++++++++
> >  arch/x86/Kconfig                       |   1 +
> >  arch/x86/entry/syscalls/syscall_32.tbl |   1 +
> >  arch/x86/entry/syscalls/syscall_64.tbl |   1 +
> >  arch/x86/include/asm/intel_pconfig.h   |  42 ++++-
> >  arch/x86/include/asm/mktme.h           |  21 +++
> >  arch/x86/mm/mktme.c                    | 141 ++++++++++++++
> >  fs/exec.c                              |   4 +-
> >  include/keys/mktme-type.h              |  28 +++
> >  include/linux/key.h                    |   2 +
> >  include/linux/mm.h                     |   9 +-
> >  include/linux/syscalls.h               |   2 +
> >  include/uapi/asm-generic/unistd.h      |   4 +-
> >  kernel/fork.c                          |   2 +
> >  kernel/sys_ni.c                        |   2 +
> >  mm/mmap.c                              |  12 ++
> >  mm/mprotect.c                          |  93 +++++++++-
> >  mm/nommu.c                             |   4 +
> >  security/keys/Kconfig                  |  11 ++
> >  security/keys/Makefile                 |   1 +
> >  security/keys/internal.h               |   6 +
> >  security/keys/keyctl.c                 |   7 +
> >  security/keys/mktme_keys.c             | 325
> > +++++++++++++++++++++++++++++++++
> >  23 files changed, 855 insertions(+), 17 deletions(-)  create mode 100644
> > Documentation/x86/mktme-keys.txt  create mode 100644 include/keys/mktme-
> > type.h  create mode 100644 security/keys/mktme_keys.c
> > 
> > --
> > 2.14.1
> 

Re: [RFC 09/12] mm: Restrict memory encryption to anonymous VMA's

[Jarkko Sakkinen]

On Mon, Sep 10, 2018 at 11:57:49AM -0700, Dave Hansen wrote:
> On 09/10/2018 11:21 AM, Sakkinen, Jarkko wrote:
> >> +/*
> >> + * Encrypted mprotect is only supported on anonymous mappings.
> >> + * All VMA's in the requested range must be anonymous. If this
> >> + * test fails on any single VMA, the entire mprotect request fails.
> >> + */
> > kdoc
> 
> kdoc what?  You want this comment in kdoc format?  Why?

If there is a header comment for a function anyway, why wouldn't you
put it to kdoc-format?

/Jarkko

Re: [RFC 09/12] mm: Restrict memory encryption to anonymous VMA's

[Dave Hansen]

On 09/10/2018 02:07 PM, Jarkko Sakkinen wrote:
> On Mon, Sep 10, 2018 at 11:57:49AM -0700, Dave Hansen wrote:
>> On 09/10/2018 11:21 AM, Sakkinen, Jarkko wrote:
>>>> +/*
>>>> + * Encrypted mprotect is only supported on anonymous mappings.
>>>> + * All VMA's in the requested range must be anonymous. If this
>>>> + * test fails on any single VMA, the entire mprotect request fails.
>>>> + */
>>> kdoc
>> kdoc what?  You want this comment in kdoc format?  Why?
> If there is a header comment for a function anyway, why wouldn't you
> put it to kdoc-format?

Because this is a shorter and more concise way to document the function.
 kdoc isn't universally the best thing to do for function comments.

Re: [RFC 11/12] keys/mktme: Add a new key service type for memory encryption keys

[Alison Schofield]

On Sun, Sep 09, 2018 at 08:29:29PM -0700, Huang, Kai wrote:
> > -----Original Message-----
> > From: keyrings-owner@vger.kernel.org [mailto:keyrings-
> > owner@vger.kernel.org] On Behalf Of Alison Schofield
> > Sent: Saturday, September 8, 2018 10:39 AM
> > To: dhowells@redhat.com; tglx@linutronix.de
> > Cc: Huang, Kai <kai.huang@intel.com>; Nakajima, Jun
> > <jun.nakajima@intel.com>; Shutemov, Kirill <kirill.shutemov@intel.com>;
> > Hansen, Dave <dave.hansen@intel.com>; Sakkinen, Jarkko
> > <jarkko.sakkinen@intel.com>; jmorris@namei.org; keyrings@vger.kernel.org;
> > linux-security-module@vger.kernel.org; mingo@redhat.com; hpa@zytor.com;
> > x86@kernel.org; linux-mm@kvack.org
> > Subject: [RFC 11/12] keys/mktme: Add a new key service type for memory
> > encryption keys
> > 
> > MKTME (Multi-Key Total Memory Encryption) is a technology that allows
> > transparent memory encryption in upcoming Intel platforms. MKTME will
> > support mulitple encryption domains, each having their own key. The main use
> > case for the feature is virtual machine isolation. The API needs the flexibility to
> > work for a wide range of uses.
> > 
> > The MKTME key service type manages the addition and removal of the memory
> > encryption keys. It maps software keys to hardware keyids and programs the
> > hardware with the user requested encryption options.
> > 
> > The only supported encryption algorithm is AES-XTS 128.
> > 
> > The MKTME key service is half of the MKTME API level solution. It pairs with a
> > new memory encryption system call: encrypt_mprotect() that uses the keys to
> > encrypt memory.
> > 


Kai -
Splitting out responses by subject...

> > +cpumask_var_t mktme_cpumask;		/* one cpu per pkg to program
> > keys */
> 
> Oh the 'mktme_cpumask' is here. Sorry I didn't notice when replying to your patch 10. :)
> 
> But I think you can just move what you did in patch 10 here and leave intel_pconfig.h unchanged. It's much clearer. 

I'll try that out and see how it works.

> > +
> > +	for_each_online_cpu(online_cpu) {
> > +		online_pkgid = topology_physical_package_id(online_cpu);
> > +
> > +		for_each_cpu(mktme_cpu, mktme_cpumask) {
> > +			mktme_pkgid =
> > topology_physical_package_id(mktme_cpu);
> > +			if (mktme_pkgid == online_pkgid)
> > +				break;
> > +		}
> > +		if (mktme_pkgid != online_pkgid)
> > +			cpumask_set_cpu(online_cpu, mktme_cpumask);
> > +	}
> 
> Could we use 'for_each_online_node', 'cpumask_first/next', etc to simplify the logic?

Sure - I'll look at those. 

Thanks!
Alison

RE: [RFC 04/12] x86/mm: Add helper functions to manage memory encryption keys

[Huang, Kai]

> -----Original Message-----
> From: owner-linux-security-module@vger.kernel.org [mailto:owner-linux-
> security-module@vger.kernel.org] On Behalf Of Huang, Kai
> Sent: Monday, September 10, 2018 2:57 PM
> To: Schofield, Alison <alison.schofield@intel.com>; dhowells@redhat.com;
> tglx@linutronix.de
> Cc: Nakajima, Jun <jun.nakajima@intel.com>; Shutemov, Kirill
> <kirill.shutemov@intel.com>; Hansen, Dave <dave.hansen@intel.com>;
> Sakkinen, Jarkko <jarkko.sakkinen@intel.com>; jmorris@namei.org;
> keyrings@vger.kernel.org; linux-security-module@vger.kernel.org;
> mingo@redhat.com; hpa@zytor.com; x86@kernel.org; linux-mm@kvack.org
> Subject: RE: [RFC 04/12] x86/mm: Add helper functions to manage memory
> encryption keys
> 
> 
> > -----Original Message-----
> > From: owner-linux-security-module@vger.kernel.org [mailto:owner-linux-
> > security-module@vger.kernel.org] On Behalf Of Alison Schofield
> > Sent: Saturday, September 8, 2018 10:36 AM
> > To: dhowells@redhat.com; tglx@linutronix.de
> > Cc: Huang, Kai <kai.huang@intel.com>; Nakajima, Jun
> > <jun.nakajima@intel.com>; Shutemov, Kirill
> > <kirill.shutemov@intel.com>; Hansen, Dave <dave.hansen@intel.com>;
> > Sakkinen, Jarkko <jarkko.sakkinen@intel.com>; jmorris@namei.org;
> > keyrings@vger.kernel.org; linux-security-module@vger.kernel.org;
> > mingo@redhat.com; hpa@zytor.com; x86@kernel.org; linux-mm@kvack.org
> > Subject: [RFC 04/12] x86/mm: Add helper functions to manage memory
> > encryption keys
> >
> > Define a global mapping structure to track the mapping of userspace
> > keys to hardware keyids in MKTME (Multi-Key Total Memory Encryption).
> > This data will be used for the memory encryption system call and the
> > kernel key service API.
> >
> > Implement helper functions to access this mapping structure and make
> > them visible to the MKTME Kernel Key Service: security/keys/mktme_keys
> >
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > ---
> >  arch/x86/include/asm/mktme.h | 11 ++++++
> >  arch/x86/mm/mktme.c          | 85
> > ++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 96 insertions(+)
> 
> Maybe it's better to put those changes to include/keys/mktme-type.h, and
> security/keys/mktme_key.c? It seems you don't have to involve linux-mm and
> x86 guys by doing so?
> 
> Thanks,
> -Kai
> >
> > diff --git a/arch/x86/include/asm/mktme.h
> > b/arch/x86/include/asm/mktme.h index dbfbd955da98..f6acd551457f 100644
> > --- a/arch/x86/include/asm/mktme.h
> > +++ b/arch/x86/include/asm/mktme.h
> > @@ -13,6 +13,17 @@ extern phys_addr_t mktme_keyid_mask;  extern int
> > mktme_nr_keyids;  extern int mktme_keyid_shift;
> >
> > +/* Manage mappings between hardware keyids and userspace keys */
> > +extern int mktme_map_alloc(void); extern void mktme_map_free(void);
> > +extern void mktme_map_lock(void); extern void mktme_map_unlock(void);
> > +extern int mktme_map_get_free_keyid(void); extern void
> > +mktme_map_clear_keyid(int keyid); extern void mktme_map_set_keyid(int
> > +keyid, unsigned int serial); extern int
> > +mktme_map_keyid_from_serial(unsigned int serial); extern unsigned int
> > +mktme_map_serial_from_keyid(int keyid);
> > +
> >  extern struct page_ext_operations page_mktme_ops;
> >
> >  #define page_keyid page_keyid
> > diff --git a/arch/x86/mm/mktme.c b/arch/x86/mm/mktme.c index
> > 660caf6a5ce1..5246d8323359 100644
> > --- a/arch/x86/mm/mktme.c
> > +++ b/arch/x86/mm/mktme.c
> > @@ -63,6 +63,91 @@ int vma_keyid(struct vm_area_struct *vma)
> >  	return (prot & mktme_keyid_mask) >> mktme_keyid_shift;  }
> >
> > +/*
> > + * struct mktme_mapping and the mktme_map_* functions manage the
> > +mapping
> > + * of userspace keys to hardware keyids in MKTME. They are used by
> > +the
> > + * the encrypt_mprotect system call and the MKTME Key Service API.
> > + */
> > +struct mktme_mapping {
> > +	struct mutex	lock;		/* protect this map & HW state */
> > +	unsigned int	mapped_keyids;
> > +	unsigned int	serial[];
> > +};

Sorry one more comment that I missed yesterday:

I think 'key_serial_t' should be used  as type of serial throughout this patch, but not 'unsigned int'. 

Thanks,
-Kai

> > +
> > +struct mktme_mapping *mktme_map;
> > +
> > +static inline long mktme_map_size(void) {
> > +	long size = 0;
> > +
> > +	size += sizeof(mktme_map);
> > +	size += sizeof(mktme_map->serial[0]) * mktme_nr_keyids;
> > +	return size;
> > +}
> > +
> > +int mktme_map_alloc(void)
> > +{
> > +	mktme_map = kzalloc(mktme_map_size(), GFP_KERNEL);
> > +	if (!mktme_map)
> > +		return 0;
> > +	mutex_init(&mktme_map->lock);
> > +	return 1;
> > +}
> > +
> > +void mktme_map_free(void)
> > +{
> > +	kfree(mktme_map);
> > +}
> > +
> > +void mktme_map_lock(void)
> > +{
> > +	mutex_lock(&mktme_map->lock);
> > +}
> > +
> > +void mktme_map_unlock(void)
> > +{
> > +	mutex_unlock(&mktme_map->lock);
> > +}
> > +
> > +void mktme_map_set_keyid(int keyid, unsigned int serial) {
> > +	mktme_map->serial[keyid] = serial;
> > +	mktme_map->mapped_keyids++;
> > +}
> > +
> > +void mktme_map_clear_keyid(int keyid) {
> > +	mktme_map->serial[keyid] = 0;
> > +	mktme_map->mapped_keyids--;
> > +}
> > +
> > +unsigned int mktme_map_serial_from_keyid(int keyid) {
> > +	return mktme_map->serial[keyid];
> > +}
> > +
> > +int mktme_map_keyid_from_serial(unsigned int serial) {
> > +	int i;
> > +
> > +	for (i = 1; i < mktme_nr_keyids; i++)
> > +		if (mktme_map->serial[i] == serial)
> > +			return i;
> > +	return 0;
> > +}
> > +
> > +int mktme_map_get_free_keyid(void)
> > +{
> > +	int i;
> > +
> > +	if (mktme_map->mapped_keyids < mktme_nr_keyids) {
> > +		for (i = 1; i < mktme_nr_keyids; i++)
> > +			if (mktme_map->serial[i] == 0)
> > +				return i;
> > +	}
> > +	return 0;
> > +}
> > +
> >  void prep_encrypted_page(struct page *page, int order, int keyid, bool zero)  {
> >  	int i;
> > --
> > 2.14.1

Re: [RFC 04/12] x86/mm: Add helper functions to manage memory encryption keys

[Alison Schofield]

On Mon, Sep 10, 2018 at 04:37:01PM -0700, Huang, Kai wrote:
> > -----Original Message-----
> > From: owner-linux-security-module@vger.kernel.org [mailto:owner-linux-
> > security-module@vger.kernel.org] On Behalf Of Huang, Kai
> > Sent: Monday, September 10, 2018 2:57 PM
> > To: Schofield, Alison <alison.schofield@intel.com>; dhowells@redhat.com;
> > tglx@linutronix.de
> > Cc: Nakajima, Jun <jun.nakajima@intel.com>; Shutemov, Kirill
> > <kirill.shutemov@intel.com>; Hansen, Dave <dave.hansen@intel.com>;
> > Sakkinen, Jarkko <jarkko.sakkinen@intel.com>; jmorris@namei.org;
> > keyrings@vger.kernel.org; linux-security-module@vger.kernel.org;
> > mingo@redhat.com; hpa@zytor.com; x86@kernel.org; linux-mm@kvack.org
> > Subject: RE: [RFC 04/12] x86/mm: Add helper functions to manage memory
> > encryption keys
> > 
> > 
> > > -----Original Message-----
> > > From: owner-linux-security-module@vger.kernel.org [mailto:owner-linux-
> > > security-module@vger.kernel.org] On Behalf Of Alison Schofield
> > > Sent: Saturday, September 8, 2018 10:36 AM
> > > To: dhowells@redhat.com; tglx@linutronix.de
> > > Cc: Huang, Kai <kai.huang@intel.com>; Nakajima, Jun
> > > <jun.nakajima@intel.com>; Shutemov, Kirill
> > > <kirill.shutemov@intel.com>; Hansen, Dave <dave.hansen@intel.com>;
> > > Sakkinen, Jarkko <jarkko.sakkinen@intel.com>; jmorris@namei.org;
> > > keyrings@vger.kernel.org; linux-security-module@vger.kernel.org;
> > > mingo@redhat.com; hpa@zytor.com; x86@kernel.org; linux-mm@kvack.org
> > > Subject: [RFC 04/12] x86/mm: Add helper functions to manage memory
> > > encryption keys
> > >
> > > Define a global mapping structure to track the mapping of userspace
> > > keys to hardware keyids in MKTME (Multi-Key Total Memory Encryption).
> > > This data will be used for the memory encryption system call and the
> > > kernel key service API.
> > >
> > > Implement helper functions to access this mapping structure and make
> > > them visible to the MKTME Kernel Key Service: security/keys/mktme_keys
> > >
> > > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > > ---
> > >  arch/x86/include/asm/mktme.h | 11 ++++++
> > >  arch/x86/mm/mktme.c          | 85
> > > ++++++++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 96 insertions(+)
> > 
> > Maybe it's better to put those changes to include/keys/mktme-type.h, and
> > security/keys/mktme_key.c? It seems you don't have to involve linux-mm and
> > x86 guys by doing so?
> > 
> > Thanks,
> > -Kai
> > >
> > > diff --git a/arch/x86/include/asm/mktme.h
> > > b/arch/x86/include/asm/mktme.h index dbfbd955da98..f6acd551457f 100644
> > > --- a/arch/x86/include/asm/mktme.h
> > > +++ b/arch/x86/include/asm/mktme.h
> > > @@ -13,6 +13,17 @@ extern phys_addr_t mktme_keyid_mask;  extern int
> > > mktme_nr_keyids;  extern int mktme_keyid_shift;
> > >
> > > +/* Manage mappings between hardware keyids and userspace keys */
> > > +extern int mktme_map_alloc(void); extern void mktme_map_free(void);
> > > +extern void mktme_map_lock(void); extern void mktme_map_unlock(void);
> > > +extern int mktme_map_get_free_keyid(void); extern void
> > > +mktme_map_clear_keyid(int keyid); extern void mktme_map_set_keyid(int
> > > +keyid, unsigned int serial); extern int
> > > +mktme_map_keyid_from_serial(unsigned int serial); extern unsigned int
> > > +mktme_map_serial_from_keyid(int keyid);
> > > +
> > >  extern struct page_ext_operations page_mktme_ops;
> > >
> > >  #define page_keyid page_keyid
> > > diff --git a/arch/x86/mm/mktme.c b/arch/x86/mm/mktme.c index
> > > 660caf6a5ce1..5246d8323359 100644
> > > --- a/arch/x86/mm/mktme.c
> > > +++ b/arch/x86/mm/mktme.c
> > > @@ -63,6 +63,91 @@ int vma_keyid(struct vm_area_struct *vma)
> > >  	return (prot & mktme_keyid_mask) >> mktme_keyid_shift;  }
> > >
> > > +/*
> > > + * struct mktme_mapping and the mktme_map_* functions manage the
> > > +mapping
> > > + * of userspace keys to hardware keyids in MKTME. They are used by
> > > +the
> > > + * the encrypt_mprotect system call and the MKTME Key Service API.
> > > + */
> > > +struct mktme_mapping {
> > > +	struct mutex	lock;		/* protect this map & HW state */
> > > +	unsigned int	mapped_keyids;
> > > +	unsigned int	serial[];
> > > +};
> 
> Sorry one more comment that I missed yesterday:
> 
> I think 'key_serial_t' should be used  as type of serial throughout this patch, but not 'unsigned int'. 
> 
> Thanks,
> -Kai

I agree! It's not an oversight, but rather a header file include nightmare.
I can look at it again.

Re: [RFC 01/12] docs/x86: Document the Multi-Key Total Memory Encryption API

[Alison Schofield]

On Sun, Sep 09, 2018 at 06:28:28PM -0700, Huang, Kai wrote:
> 
> > -----Original Message-----
> > From: owner-linux-mm@kvack.org [mailto:owner-linux-mm@kvack.org] On
> > Behalf Of Alison Schofield
> > Sent: Saturday, September 8, 2018 10:34 AM
> > To: dhowells@redhat.com; tglx@linutronix.de
> > Cc: Huang, Kai <kai.huang@intel.com>; Nakajima, Jun
> > <jun.nakajima@intel.com>; Shutemov, Kirill <kirill.shutemov@intel.com>;
> > Hansen, Dave <dave.hansen@intel.com>; Sakkinen, Jarkko
> > <jarkko.sakkinen@intel.com>; jmorris@namei.org; keyrings@vger.kernel.org;
> > linux-security-module@vger.kernel.org; mingo@redhat.com; hpa@zytor.com;
> > x86@kernel.org; linux-mm@kvack.org
> > Subject: [RFC 01/12] docs/x86: Document the Multi-Key Total Memory
> > Encryption API
> > 
> > Document the API's used for MKTME on Intel platforms.
> > MKTME: Multi-KEY Total Memory Encryption
> > 
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > ---
> >  Documentation/x86/mktme-keys.txt | 153
> > +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 153 insertions(+)
> >  create mode 100644 Documentation/x86/mktme-keys.txt
> > 
> > diff --git a/Documentation/x86/mktme-keys.txt b/Documentation/x86/mktme-
> > keys.txt
> > new file mode 100644
> > index 000000000000..2dea7acd2a17
> > --- /dev/null
> > +++ b/Documentation/x86/mktme-keys.txt
> > @@ -0,0 +1,153 @@
> > +MKTME (Multi-Key Total Memory Encryption) is a technology that allows
> > +memory encryption on Intel platforms. Whereas TME (Total Memory
> > +Encryption) allows encryption of the entire system memory using a
> > +single key, MKTME allows multiple encryption domains, each having their
> > +own key. The main use case for the feature is virtual machine
> > +isolation. The API's introduced here are intended to offer flexibility to work in a
> > wide range of uses.
> > +
> > +The externally available Intel Architecture Spec:
> > +https://software.intel.com/sites/default/files/managed/a5/16/Multi-Key-
> > +Total-Memory-Encryption-Spec.pdf
> > +
> > +============================  API Overview
> > +============================
> > +
> > +There are 2 MKTME specific API's that enable userspace to create and
> > +use the memory encryption keys:
> > +
> > +1) Kernel Key Service: MKTME Type
> > +
> > +   MKTME is a new key type added to the existing Kernel Key Services
> > +   to support the memory encryption keys. The MKTME service manages
> > +   the addition and removal of MKTME keys. It maps userspace keys
> > +   to hardware keyids and programs the hardware with user requested
> > +   encryption parameters.
> > +
> > +   o An understanding of the Kernel Key Service is required in order
> > +     to use the MKTME key type as it is a subset of that service.
> > +
> > +   o MKTME keys are a limited resource. There is a single pool of
> > +     MKTME keys for a system and that pool can be from 3 to 63 keys.
> 
> Why 3 to 63 keys? Architecturally we are able to support up to 15-bit keyID, although in the first generation server we only support 6-bit keyID, which is 63 key/keyIDs (excluding keyID 0, which is TME's keyID).

My understanding is that low level SKU's could have as few as 3 bits 
available to hold the keyid, and that the max is 6 bits, hence 64.
I probably don't need to be stating that level of detail here, but
rather just iterate the important point that the resource is limited!

> 
> > +     With that in mind, userspace may take advantage of the kernel
> > +     key services sharing and permissions model for userspace keys.
> > +     One key can be shared as long as each user has the permission
> > +     of "KEY_NEED_VIEW" to use it.
> > +
> > +   o MKTME key type uses capabilities to restrict the allocation
> > +     of keys. It only requires CAP_SYS_RESOURCE, but will accept
> > +     the broader capability of CAP_SYS_ADMIN.  See capabilities(7).
> > +
> > +   o The MKTME key service blocks kernel key service commands that
> > +     could lead to reprogramming of in use keys, or loss of keys from
> > +     the pool. This means MKTME does not allow a key to be invalidated,
> > +     unlinked, or timed out. These operations are blocked by MKTME as
> > +     it creates all keys with the internal flag KEY_FLAG_KEEP.
> > +
> > +   o MKTME does not support the keyctl option of UPDATE. Userspace
> > +     may change the programming of a key by revoking it and adding
> > +     a new key with the updated encryption options (or vice-versa).
> > +
> > +2) System Call: encrypt_mprotect()
> > +
> > +   MKTME encryption is requested by calling encrypt_mprotect(). The
> > +   caller passes the serial number to a previously allocated and
> > +   programmed encryption key. That handle was created with the MKTME
> > +   Key Service.
> > +
> > +   o The caller must have KEY_NEED_VIEW permission on the key
> > +
> > +   o The range of memory that is to be protected must be mapped as
> > +     ANONYMOUS. If it is not, the entire encrypt_mprotect() request
> > +     fails with EINVAL.
> > +
> > +   o As an extension to the existing mprotect() system call,
> > +     encrypt_mprotect() supports the legacy mprotect behavior plus
> > +     the enabling of memory encryption. That means that in addition
> > +     to encrypting the memory, the protection flags will be updated
> > +     as requested in the call.
> > +
> > +   o Additional mprotect() calls to memory already protected with
> > +     MKTME will not alter the MKTME status.
> 
> I think it's better to separate encrypt_mprotect() into another doc so both parts can be reviewed easier.

I can do that.
Also, I do know I need man page for that too.
> 
> > +
> > +======================  Usage: MKTME Key Service
> > +======================
> > +
> > +MKTME is enabled on supported Intel platforms by selecting
> > +CONFIG_X86_INTEL_MKTME which selects CONFIG_MKTME_KEYS.
> > +
> > +Allocating MKTME Keys via command line or system call:
> > +    keyctl add mktme name "[options]" ring
> > +
> > +    key_serial_t add_key(const char *type, const char *description,
> > +                         const void *payload, size_t plen,
> > +                         key_serial_t keyring);
> > +
> > +Revoking MKTME Keys via command line or system call::
> > +   keyctl revoke <key>
> > +
> > +   long keyctl(KEYCTL_REVOKE, key_serial_t key);
> > +
> > +Options Field Definition:
> > +    userkey=      ASCII HEX value encryption key. Defaults to a CPU
> > +		  generated key if a userkey is not defined here.
> > +
> > +    algorithm=    Encryption algorithm name as a string.
> > +		  Valid algorithm: "aes-xts-128"
> > +
> > +    tweak=        ASCII HEX value tweak key. Tweak key will be added to the
> > +                  userkey...  (need to be clear here that this is being sent
> > +                  to the hardware - kernel not messing w it)
> > +
> > +    entropy=      ascii hex value entropy.
> > +                  This entropy will be used to generated the CPU key and
> > +		  the tweak key when CPU generated key is requested.
> > +
> > +Algorithm Dependencies:
> > +    AES-XTS 128 is the only supported algorithm.
> > +    There are only 2 ways that AES-XTS 128 may be used:
> > +
> > +    1) User specified encryption key
> > +	- The user specified encryption key must be exactly
> > +	  16 ASCII Hex bytes (128 bits).
> > +	- A tweak key must be specified and it must be exactly
> > +	  16 ASCII Hex bytes (128 bits).
> > +	- No entropy field is accepted.
> > +
> > +    2) CPU generated encryption key
> > +	- When no user specified encryption key is provided, the
> > +	  default encryption key will be CPU generated.
> > +	- User must specify 16 ASCII Hex bytes of entropy. This
> > +	  entropy will be used by the CPU to generate both the
> > +	  encryption key and the tweak key.
> > +	- No entropy field is accepted.
             ^^^^^^^ should be tweak

> 
> This is not true. The spec says in CPU generated random mode, both 'key' and 'tweak' part are used to generate the final key and tweak respectively.
> 
> Actually, simple 'XOR' is used to generate the final key:
> 
> case KEYID_SET_KEY_RANDOM:
> 	......
> 	(* Mix user supplied entropy to the data key and tweak key *)
> 	TMP_RND_DATA_KEY = TMP_RND_KEY XOR
> 		TMP_KEY_PROGRAM_STRUCT.KEY_FIELD_1.BYTES[15:0];
> 	TMP_RND_TWEAK_KEY = TMP_RND_TWEAK_KEY XOR
> 		TMP_KEY_PROGRAM_STRUCT.KEY_FIELD_2.BYTES[15:0];
> 
> So I think we can either just remove 'entropy' parameter, since we can use both 'userkey' and 'tweak' even for random key mode.
> 
> In fact, which might be better IMHO, we can simply disallow or ignore 'userkey' and 'tweak' part for random key mode, since if we allow user to specify both entropies, and if user passes value with all 1, we are effectively making the key and tweak to be all 1, which is not random anymore.
> 
> Instead, kernel can generate random for both entropies, or we can simply uses 0, ignoring user input.

Kai,
I think my typo above, threw you off. We have the same understanding of
the key fields.

Is this the structure you are suggesting?

	Options

	key_type=	"user" or "CPU"

	key=		If key_type == user
				key= is the data key
			If key_type == CPU
				key= is not required
				if key= is present
					it is entropy to be mixed with
					CPU generated data key

	tweak=		If key_type == user
				tweak= is the tweak key
			If key_type == CPU
				tweak= is not required
				if tweak= is present
					it is entropy to be mixed with
					CPU generated tweak key


Alison
> 
> Thanks,
> -Kai

........snip...........

RE: [RFC 01/12] docs/x86: Document the Multi-Key Total Memory Encryption API

[Huang, Kai]

> -----Original Message-----
> From: owner-linux-mm@kvack.org [mailto:owner-linux-mm@kvack.org] On
> Behalf Of Huang, Kai
> Sent: Monday, September 10, 2018 1:28 PM
> To: Schofield, Alison <alison.schofield@intel.com>; dhowells@redhat.com;
> tglx@linutronix.de
> Cc: Nakajima, Jun <jun.nakajima@intel.com>; Shutemov, Kirill
> <kirill.shutemov@intel.com>; Hansen, Dave <dave.hansen@intel.com>;
> Sakkinen, Jarkko <jarkko.sakkinen@intel.com>; jmorris@namei.org;
> keyrings@vger.kernel.org; linux-security-module@vger.kernel.org;
> mingo@redhat.com; hpa@zytor.com; x86@kernel.org; linux-mm@kvack.org
> Subject: RE: [RFC 01/12] docs/x86: Document the Multi-Key Total Memory
> Encryption API
> 
> 
> > -----Original Message-----
> > From: owner-linux-mm@kvack.org [mailto:owner-linux-mm@kvack.org] On
> > Behalf Of Alison Schofield
> > Sent: Saturday, September 8, 2018 10:34 AM
> > To: dhowells@redhat.com; tglx@linutronix.de
> > Cc: Huang, Kai <kai.huang@intel.com>; Nakajima, Jun
> > <jun.nakajima@intel.com>; Shutemov, Kirill
> > <kirill.shutemov@intel.com>; Hansen, Dave <dave.hansen@intel.com>;
> > Sakkinen, Jarkko <jarkko.sakkinen@intel.com>; jmorris@namei.org;
> > keyrings@vger.kernel.org; linux-security-module@vger.kernel.org;
> > mingo@redhat.com; hpa@zytor.com; x86@kernel.org; linux-mm@kvack.org
> > Subject: [RFC 01/12] docs/x86: Document the Multi-Key Total Memory
> > Encryption API
> >
> > Document the API's used for MKTME on Intel platforms.
> > MKTME: Multi-KEY Total Memory Encryption
> >
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > ---
> >  Documentation/x86/mktme-keys.txt | 153
> > +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 153 insertions(+)
> >  create mode 100644 Documentation/x86/mktme-keys.txt
> >
> > diff --git a/Documentation/x86/mktme-keys.txt
> > b/Documentation/x86/mktme- keys.txt new file mode 100644 index
> > 000000000000..2dea7acd2a17
> > --- /dev/null
> > +++ b/Documentation/x86/mktme-keys.txt
> > @@ -0,0 +1,153 @@
> > +MKTME (Multi-Key Total Memory Encryption) is a technology that allows
> > +memory encryption on Intel platforms. Whereas TME (Total Memory
> > +Encryption) allows encryption of the entire system memory using a
> > +single key, MKTME allows multiple encryption domains, each having
> > +their own key. The main use case for the feature is virtual machine
> > +isolation. The API's introduced here are intended to offer
> > +flexibility to work in a
> > wide range of uses.
> > +
> > +The externally available Intel Architecture Spec:
> > +https://software.intel.com/sites/default/files/managed/a5/16/Multi-Ke
> > +y-
> > +Total-Memory-Encryption-Spec.pdf
> > +
> > +============================  API Overview
> > +============================
> > +
> > +There are 2 MKTME specific API's that enable userspace to create and
> > +use the memory encryption keys:
> > +
> > +1) Kernel Key Service: MKTME Type
> > +
> > +   MKTME is a new key type added to the existing Kernel Key Services
> > +   to support the memory encryption keys. The MKTME service manages
> > +   the addition and removal of MKTME keys. It maps userspace keys
> > +   to hardware keyids and programs the hardware with user requested
> > +   encryption parameters.
> > +
> > +   o An understanding of the Kernel Key Service is required in order
> > +     to use the MKTME key type as it is a subset of that service.
> > +
> > +   o MKTME keys are a limited resource. There is a single pool of
> > +     MKTME keys for a system and that pool can be from 3 to 63 keys.
> 
> Why 3 to 63 keys? Architecturally we are able to support up to 15-bit keyID,
> although in the first generation server we only support 6-bit keyID, which is 63
> key/keyIDs (excluding keyID 0, which is TME's keyID).
> 
> > +     With that in mind, userspace may take advantage of the kernel
> > +     key services sharing and permissions model for userspace keys.
> > +     One key can be shared as long as each user has the permission
> > +     of "KEY_NEED_VIEW" to use it.
> > +
> > +   o MKTME key type uses capabilities to restrict the allocation
> > +     of keys. It only requires CAP_SYS_RESOURCE, but will accept
> > +     the broader capability of CAP_SYS_ADMIN.  See capabilities(7).
> > +
> > +   o The MKTME key service blocks kernel key service commands that
> > +     could lead to reprogramming of in use keys, or loss of keys from
> > +     the pool. This means MKTME does not allow a key to be invalidated,
> > +     unlinked, or timed out. These operations are blocked by MKTME as
> > +     it creates all keys with the internal flag KEY_FLAG_KEEP.
> > +
> > +   o MKTME does not support the keyctl option of UPDATE. Userspace
> > +     may change the programming of a key by revoking it and adding
> > +     a new key with the updated encryption options (or vice-versa).
> > +
> > +2) System Call: encrypt_mprotect()
> > +
> > +   MKTME encryption is requested by calling encrypt_mprotect(). The
> > +   caller passes the serial number to a previously allocated and
> > +   programmed encryption key. That handle was created with the MKTME
> > +   Key Service.
> > +
> > +   o The caller must have KEY_NEED_VIEW permission on the key
> > +
> > +   o The range of memory that is to be protected must be mapped as
> > +     ANONYMOUS. If it is not, the entire encrypt_mprotect() request
> > +     fails with EINVAL.
> > +
> > +   o As an extension to the existing mprotect() system call,
> > +     encrypt_mprotect() supports the legacy mprotect behavior plus
> > +     the enabling of memory encryption. That means that in addition
> > +     to encrypting the memory, the protection flags will be updated
> > +     as requested in the call.
> > +
> > +   o Additional mprotect() calls to memory already protected with
> > +     MKTME will not alter the MKTME status.
> 
> I think it's better to separate encrypt_mprotect() into another doc so both parts
> can be reviewed easier.
> 
> > +
> > +======================  Usage: MKTME Key Service
> > +======================
> > +
> > +MKTME is enabled on supported Intel platforms by selecting
> > +CONFIG_X86_INTEL_MKTME which selects CONFIG_MKTME_KEYS.
> > +
> > +Allocating MKTME Keys via command line or system call:
> > +    keyctl add mktme name "[options]" ring
> > +
> > +    key_serial_t add_key(const char *type, const char *description,
> > +                         const void *payload, size_t plen,
> > +                         key_serial_t keyring);
> > +
> > +Revoking MKTME Keys via command line or system call::
> > +   keyctl revoke <key>
> > +
> > +   long keyctl(KEYCTL_REVOKE, key_serial_t key);
> > +
> > +Options Field Definition:
> > +    userkey=      ASCII HEX value encryption key. Defaults to a CPU
> > +		  generated key if a userkey is not defined here.
> > +
> > +    algorithm=    Encryption algorithm name as a string.
> > +		  Valid algorithm: "aes-xts-128"
> > +
> > +    tweak=        ASCII HEX value tweak key. Tweak key will be added to the
> > +                  userkey...  (need to be clear here that this is being sent
> > +                  to the hardware - kernel not messing w it)
> > +
> > +    entropy=      ascii hex value entropy.
> > +                  This entropy will be used to generated the CPU key and
> > +		  the tweak key when CPU generated key is requested.
> > +
> > +Algorithm Dependencies:
> > +    AES-XTS 128 is the only supported algorithm.
> > +    There are only 2 ways that AES-XTS 128 may be used:
> > +
> > +    1) User specified encryption key
> > +	- The user specified encryption key must be exactly
> > +	  16 ASCII Hex bytes (128 bits).
> > +	- A tweak key must be specified and it must be exactly
> > +	  16 ASCII Hex bytes (128 bits).
> > +	- No entropy field is accepted.
> > +
> > +    2) CPU generated encryption key
> > +	- When no user specified encryption key is provided, the
> > +	  default encryption key will be CPU generated.
> > +	- User must specify 16 ASCII Hex bytes of entropy. This
> > +	  entropy will be used by the CPU to generate both the
> > +	  encryption key and the tweak key.
> > +	- No entropy field is accepted.
> 
> This is not true. The spec says in CPU generated random mode, both 'key' and
> 'tweak' part are used to generate the final key and tweak respectively.
> 
> Actually, simple 'XOR' is used to generate the final key:
> 
> case KEYID_SET_KEY_RANDOM:
> 	......
> 	(* Mix user supplied entropy to the data key and tweak key *)
> 	TMP_RND_DATA_KEY = TMP_RND_KEY XOR
> 		TMP_KEY_PROGRAM_STRUCT.KEY_FIELD_1.BYTES[15:0];
> 	TMP_RND_TWEAK_KEY = TMP_RND_TWEAK_KEY XOR
> 		TMP_KEY_PROGRAM_STRUCT.KEY_FIELD_2.BYTES[15:0];
> 
> So I think we can either just remove 'entropy' parameter, since we can use both
> 'userkey' and 'tweak' even for random key mode.
> 
> In fact, which might be better IMHO, we can simply disallow or ignore 'userkey'
> and 'tweak' part for random key mode, since if we allow user to specify both
> entropies, and if user passes value with all 1, we are effectively making the key
> and tweak to be all 1, which is not random anymore.
> 
> Instead, kernel can generate random for both entropies, or we can simply uses 0,
> ignoring user input.

Hmm.. I just realized that I confused 'OR' and 'XOR', so the last two paragraphs doesn't make sense. :)

But still I think we can simply use "userkey" and "tweak". "entropy" is not needed IMO.

Thanks,
-Kai
> 
> Thanks,
> -Kai
> > +
> > +======================  Usage: encrypt_mprotect()
> > +======================
> > +
> > +System Call encrypt_mprotect()::
> > +
> > +    This system call is an extension of the existing mprotect() system
> > +    call. It requires the same parameters as legary mprotect() plus
> > +    one additional parameter, the keyid. Userspace must provide the
> > +    key serial number assigned through the kernel key service.
> > +
> > +    int encrypt_mprotect(void *addr, size_t len, int prot, int
> > + keyid);
> > +
> > +======================  Usage: Sample Roundtrip
> > ======================
> > +
> > +Sample usage of MKTME Key Service API with encrypt_mprotect() API:
> > +
> > +  Add a key:
> > +        key = add_key(mktme, name, options, strlen(option), keyring);
> > +
> > +  Map memory:
> > +        ptr = mmap(NULL, size, prot, MAP_ANONYMOUS|MAP_PRIVATE, -1,
> > + 0);
> > +
> > +  Protect memory:
> > +        ret = syscall(sys_encrypt_mprotect, ptr, size, prot, keyid);
> > +
> > +  Use protected memory:
> > +        ................
> > +
> > +  Free memory:
> > +        ret = munmap(ptr, size);
> > +
> > +  Revoke key:
> > +        ret = keyctl(KEYCTL_REVOKE, key);
> > +
> > --
> > 2.14.1

Re: [RFC 01/12] docs/x86: Document the Multi-Key Total Memory Encryption API

[Alison Schofield]

On Mon, Sep 10, 2018 at 10:32:20AM -0700, Sakkinen, Jarkko wrote:
> On Fri, 2018-09-07 at 15:34 -0700, Alison Schofield wrote:
> > Document the API's used for MKTME on Intel platforms.
> > MKTME: Multi-KEY Total Memory Encryption
> > 
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > ---
> >  Documentation/x86/mktme-keys.txt | 153
> > +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 153 insertions(+)
> >  create mode 100644 Documentation/x86/mktme-keys.txt
> > 
> > diff --git a/Documentation/x86/mktme-keys.txt b/Documentation/x86/mktme-
> > keys.txt
> > new file mode 100644
> > index 000000000000..2dea7acd2a17
> > --- /dev/null
> > +++ b/Documentation/x86/mktme-keys.txt
> > @@ -0,0 +1,153 @@
> > +MKTME (Multi-Key Total Memory Encryption) is a technology that allows
> > +memory encryption on Intel platforms. Whereas TME (Total Memory Encryption)
> > +allows encryption of the entire system memory using a single key, MKTME
> > +allows multiple encryption domains, each having their own key. The main use
> > +case for the feature is virtual machine isolation. The API's introduced here
> > +are intended to offer flexibility to work in a wide range of uses.
> > +
> > +The externally available Intel Architecture Spec:
> > +https://software.intel.com/sites/default/files/managed/a5/16/Multi-Key-Total-
> > Memory-Encryption-Spec.pdf
> > +
> > +============================  API Overview  ============================
> > +
> > +There are 2 MKTME specific API's that enable userspace to create and use
> > +the memory encryption keys:
> 
> This is like saying that they are different APIs to do semantically the
> same exact thing. Is that so?

No. 
The API's used to create and use memory encryption keys are described below:

> 
> /Jarkko

RE: [RFC 01/12] docs/x86: Document the Multi-Key Total Memory Encryption API

[Huang, Kai]

> -----Original Message-----
> From: owner-linux-mm@kvack.org [mailto:owner-linux-mm@kvack.org] On
> Behalf Of Alison Schofield
> Sent: Tuesday, September 11, 2018 12:13 PM
> To: Huang, Kai <kai.huang@intel.com>
> Cc: dhowells@redhat.com; tglx@linutronix.de; Nakajima, Jun
> <jun.nakajima@intel.com>; Shutemov, Kirill <kirill.shutemov@intel.com>;
> Hansen, Dave <dave.hansen@intel.com>; Sakkinen, Jarkko
> <jarkko.sakkinen@intel.com>; jmorris@namei.org; keyrings@vger.kernel.org;
> linux-security-module@vger.kernel.org; mingo@redhat.com; hpa@zytor.com;
> x86@kernel.org; linux-mm@kvack.org
> Subject: Re: [RFC 01/12] docs/x86: Document the Multi-Key Total Memory
> Encryption API
> 
> On Sun, Sep 09, 2018 at 06:28:28PM -0700, Huang, Kai wrote:
> >
> > > -----Original Message-----
> > > From: owner-linux-mm@kvack.org [mailto:owner-linux-mm@kvack.org] On
> > > Behalf Of Alison Schofield
> > > Sent: Saturday, September 8, 2018 10:34 AM
> > > To: dhowells@redhat.com; tglx@linutronix.de
> > > Cc: Huang, Kai <kai.huang@intel.com>; Nakajima, Jun
> > > <jun.nakajima@intel.com>; Shutemov, Kirill
> > > <kirill.shutemov@intel.com>; Hansen, Dave <dave.hansen@intel.com>;
> > > Sakkinen, Jarkko <jarkko.sakkinen@intel.com>; jmorris@namei.org;
> > > keyrings@vger.kernel.org; linux-security-module@vger.kernel.org;
> > > mingo@redhat.com; hpa@zytor.com; x86@kernel.org; linux-
> mm@kvack.org
> > > Subject: [RFC 01/12] docs/x86: Document the Multi-Key Total Memory
> > > Encryption API
> > >
> > > Document the API's used for MKTME on Intel platforms.
> > > MKTME: Multi-KEY Total Memory Encryption
> > >
> > > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > > ---
> > >  Documentation/x86/mktme-keys.txt | 153
> > > +++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 153 insertions(+)
> > >  create mode 100644 Documentation/x86/mktme-keys.txt
> > >
> > > diff --git a/Documentation/x86/mktme-keys.txt
> > > b/Documentation/x86/mktme- keys.txt new file mode 100644 index
> > > 000000000000..2dea7acd2a17
> > > --- /dev/null
> > > +++ b/Documentation/x86/mktme-keys.txt
> > > @@ -0,0 +1,153 @@
> > > +MKTME (Multi-Key Total Memory Encryption) is a technology that
> > > +allows memory encryption on Intel platforms. Whereas TME (Total
> > > +Memory
> > > +Encryption) allows encryption of the entire system memory using a
> > > +single key, MKTME allows multiple encryption domains, each having
> > > +their own key. The main use case for the feature is virtual machine
> > > +isolation. The API's introduced here are intended to offer
> > > +flexibility to work in a
> > > wide range of uses.
> > > +
> > > +The externally available Intel Architecture Spec:
> > > +https://software.intel.com/sites/default/files/managed/a5/16/Multi-
> > > +Key-
> > > +Total-Memory-Encryption-Spec.pdf
> > > +
> > > +============================  API Overview
> > > +============================
> > > +
> > > +There are 2 MKTME specific API's that enable userspace to create
> > > +and use the memory encryption keys:
> > > +
> > > +1) Kernel Key Service: MKTME Type
> > > +
> > > +   MKTME is a new key type added to the existing Kernel Key Services
> > > +   to support the memory encryption keys. The MKTME service manages
> > > +   the addition and removal of MKTME keys. It maps userspace keys
> > > +   to hardware keyids and programs the hardware with user requested
> > > +   encryption parameters.
> > > +
> > > +   o An understanding of the Kernel Key Service is required in order
> > > +     to use the MKTME key type as it is a subset of that service.
> > > +
> > > +   o MKTME keys are a limited resource. There is a single pool of
> > > +     MKTME keys for a system and that pool can be from 3 to 63 keys.
> >
> > Why 3 to 63 keys? Architecturally we are able to support up to 15-bit keyID,
> although in the first generation server we only support 6-bit keyID, which is 63
> key/keyIDs (excluding keyID 0, which is TME's keyID).
> 
> My understanding is that low level SKU's could have as few as 3 bits available to
> hold the keyid, and that the max is 6 bits, hence 64.
> I probably don't need to be stating that level of detail here, but rather just
> iterate the important point that the resource is limited!
> 
> >
> > > +     With that in mind, userspace may take advantage of the kernel
> > > +     key services sharing and permissions model for userspace keys.
> > > +     One key can be shared as long as each user has the permission
> > > +     of "KEY_NEED_VIEW" to use it.
> > > +
> > > +   o MKTME key type uses capabilities to restrict the allocation
> > > +     of keys. It only requires CAP_SYS_RESOURCE, but will accept
> > > +     the broader capability of CAP_SYS_ADMIN.  See capabilities(7).
> > > +
> > > +   o The MKTME key service blocks kernel key service commands that
> > > +     could lead to reprogramming of in use keys, or loss of keys from
> > > +     the pool. This means MKTME does not allow a key to be invalidated,
> > > +     unlinked, or timed out. These operations are blocked by MKTME as
> > > +     it creates all keys with the internal flag KEY_FLAG_KEEP.
> > > +
> > > +   o MKTME does not support the keyctl option of UPDATE. Userspace
> > > +     may change the programming of a key by revoking it and adding
> > > +     a new key with the updated encryption options (or vice-versa).
> > > +
> > > +2) System Call: encrypt_mprotect()
> > > +
> > > +   MKTME encryption is requested by calling encrypt_mprotect(). The
> > > +   caller passes the serial number to a previously allocated and
> > > +   programmed encryption key. That handle was created with the MKTME
> > > +   Key Service.
> > > +
> > > +   o The caller must have KEY_NEED_VIEW permission on the key
> > > +
> > > +   o The range of memory that is to be protected must be mapped as
> > > +     ANONYMOUS. If it is not, the entire encrypt_mprotect() request
> > > +     fails with EINVAL.
> > > +
> > > +   o As an extension to the existing mprotect() system call,
> > > +     encrypt_mprotect() supports the legacy mprotect behavior plus
> > > +     the enabling of memory encryption. That means that in addition
> > > +     to encrypting the memory, the protection flags will be updated
> > > +     as requested in the call.
> > > +
> > > +   o Additional mprotect() calls to memory already protected with
> > > +     MKTME will not alter the MKTME status.
> >
> > I think it's better to separate encrypt_mprotect() into another doc so both
> parts can be reviewed easier.
> 
> I can do that.
> Also, I do know I need man page for that too.
> >
> > > +
> > > +======================  Usage: MKTME Key Service
> > > +======================
> > > +
> > > +MKTME is enabled on supported Intel platforms by selecting
> > > +CONFIG_X86_INTEL_MKTME which selects CONFIG_MKTME_KEYS.
> > > +
> > > +Allocating MKTME Keys via command line or system call:
> > > +    keyctl add mktme name "[options]" ring
> > > +
> > > +    key_serial_t add_key(const char *type, const char *description,
> > > +                         const void *payload, size_t plen,
> > > +                         key_serial_t keyring);
> > > +
> > > +Revoking MKTME Keys via command line or system call::
> > > +   keyctl revoke <key>
> > > +
> > > +   long keyctl(KEYCTL_REVOKE, key_serial_t key);
> > > +
> > > +Options Field Definition:
> > > +    userkey=      ASCII HEX value encryption key. Defaults to a CPU
> > > +		  generated key if a userkey is not defined here.
> > > +
> > > +    algorithm=    Encryption algorithm name as a string.
> > > +		  Valid algorithm: "aes-xts-128"
> > > +
> > > +    tweak=        ASCII HEX value tweak key. Tweak key will be added to the
> > > +                  userkey...  (need to be clear here that this is being sent
> > > +                  to the hardware - kernel not messing w it)
> > > +
> > > +    entropy=      ascii hex value entropy.
> > > +                  This entropy will be used to generated the CPU key and
> > > +		  the tweak key when CPU generated key is requested.
> > > +
> > > +Algorithm Dependencies:
> > > +    AES-XTS 128 is the only supported algorithm.
> > > +    There are only 2 ways that AES-XTS 128 may be used:
> > > +
> > > +    1) User specified encryption key
> > > +	- The user specified encryption key must be exactly
> > > +	  16 ASCII Hex bytes (128 bits).
> > > +	- A tweak key must be specified and it must be exactly
> > > +	  16 ASCII Hex bytes (128 bits).
> > > +	- No entropy field is accepted.
> > > +
> > > +    2) CPU generated encryption key
> > > +	- When no user specified encryption key is provided, the
> > > +	  default encryption key will be CPU generated.
> > > +	- User must specify 16 ASCII Hex bytes of entropy. This
> > > +	  entropy will be used by the CPU to generate both the
> > > +	  encryption key and the tweak key.
> > > +	- No entropy field is accepted.
>              ^^^^^^^ should be tweak
> 
> >
> > This is not true. The spec says in CPU generated random mode, both 'key' and
> 'tweak' part are used to generate the final key and tweak respectively.
> >
> > Actually, simple 'XOR' is used to generate the final key:
> >
> > case KEYID_SET_KEY_RANDOM:
> > 	......
> > 	(* Mix user supplied entropy to the data key and tweak key *)
> > 	TMP_RND_DATA_KEY = TMP_RND_KEY XOR
> > 		TMP_KEY_PROGRAM_STRUCT.KEY_FIELD_1.BYTES[15:0];
> > 	TMP_RND_TWEAK_KEY = TMP_RND_TWEAK_KEY XOR
> > 		TMP_KEY_PROGRAM_STRUCT.KEY_FIELD_2.BYTES[15:0];
> >
> > So I think we can either just remove 'entropy' parameter, since we can use
> both 'userkey' and 'tweak' even for random key mode.
> >
> > In fact, which might be better IMHO, we can simply disallow or ignore
> 'userkey' and 'tweak' part for random key mode, since if we allow user to specify
> both entropies, and if user passes value with all 1, we are effectively making the
> key and tweak to be all 1, which is not random anymore.
> >
> > Instead, kernel can generate random for both entropies, or we can simply uses
> 0, ignoring user input.
> 
> Kai,
> I think my typo above, threw you off. We have the same understanding of the
> key fields.
> 
> Is this the structure you are suggesting?
> 
> 	Options
> 
> 	key_type=	"user" or "CPU"
> 
> 	key=		If key_type == user
> 				key= is the data key
> 			If key_type == CPU
> 				key= is not required
> 				if key= is present
> 					it is entropy to be mixed with
> 					CPU generated data key
> 
> 	tweak=		If key_type == user
> 				tweak= is the tweak key
> 			If key_type == CPU
> 				tweak= is not required
> 				if tweak= is present
> 					it is entropy to be mixed with
> 					CPU generated tweak key

Exactly.

Although I am not sure whether we should support other 2 modes: Clear key  and  no encryption;

Thanks,
-Kai
> 
> 
> Alison
> >
> > Thanks,
> > -Kai
> 
> ........snip...........

Re: [RFC 02/12] mm: Generalize the mprotect implementation to support extensions

[Alison Schofield]

On Mon, Sep 10, 2018 at 01:12:31PM +0300, Jarkko Sakkinen wrote:
> On Fri, 2018-09-07 at 15:34 -0700, Alison Schofield wrote:
> > Today mprotect is implemented to support legacy mprotect behavior
> > plus an extension for memory protection keys. Make it more generic
> > so that it can support additional extensions in the future.
> > 
> > This is done is preparation for adding a new system call for memory
> > encyption keys. The intent is that the new encrypted mprotect will be
> > another extension to legacy mprotect.
> > 
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > ---
> >  mm/mprotect.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/mm/mprotect.c b/mm/mprotect.c
> > index 68dc476310c0..56e64ef7931e 100644
> > --- a/mm/mprotect.c
> > +++ b/mm/mprotect.c
> > @@ -35,6 +35,8 @@
> >  
> >  #include "internal.h"
> >  
> > +#define NO_PKEY  -1
> 
> This commit does not make anything more generic but it does take
> away a magic number. The code change is senseful. The commit
> message is nonsense.

do_mprotect_ext() is intended to be the generic replacement for
do_mprotect_pkey() which was added for protection keys.

> 
> PS. Please use @linux.intel.com for LKML.
Is this a request to use your @linux.intel.com email address when I'm
posting to LKML's?

> 
> /Jarkko

Re: [RFC 01/12] docs/x86: Document the Multi-Key Total Memory Encryption API

[Alison Schofield]

On Mon, Sep 10, 2018 at 05:33:33PM -0700, Huang, Kai wrote:
> > -----Original Message-----
> > From: owner-linux-mm@kvack.org [mailto:owner-linux-mm@kvack.org] On
> > Behalf Of Alison Schofield
> > Sent: Tuesday, September 11, 2018 12:13 PM
> > To: Huang, Kai <kai.huang@intel.com>
> > Cc: dhowells@redhat.com; tglx@linutronix.de; Nakajima, Jun
> > <jun.nakajima@intel.com>; Shutemov, Kirill <kirill.shutemov@intel.com>;
> > Hansen, Dave <dave.hansen@intel.com>; Sakkinen, Jarkko
> > <jarkko.sakkinen@intel.com>; jmorris@namei.org; keyrings@vger.kernel.org;
> > linux-security-module@vger.kernel.org; mingo@redhat.com; hpa@zytor.com;
> > x86@kernel.org; linux-mm@kvack.org
> > Subject: Re: [RFC 01/12] docs/x86: Document the Multi-Key Total Memory
> > Encryption API
> > 
> > On Sun, Sep 09, 2018 at 06:28:28PM -0700, Huang, Kai wrote:
> > >
> > > > -----Original Message-----
> > > > From: owner-linux-mm@kvack.org [mailto:owner-linux-mm@kvack.org] On
> > > > Behalf Of Alison Schofield
> > > > Sent: Saturday, September 8, 2018 10:34 AM
> > > > To: dhowells@redhat.com; tglx@linutronix.de
> > > > Cc: Huang, Kai <kai.huang@intel.com>; Nakajima, Jun
> > > > <jun.nakajima@intel.com>; Shutemov, Kirill
> > > > <kirill.shutemov@intel.com>; Hansen, Dave <dave.hansen@intel.com>;
> > > > Sakkinen, Jarkko <jarkko.sakkinen@intel.com>; jmorris@namei.org;
> > > > keyrings@vger.kernel.org; linux-security-module@vger.kernel.org;
> > > > mingo@redhat.com; hpa@zytor.com; x86@kernel.org; linux-
> > mm@kvack.org
> > > > Subject: [RFC 01/12] docs/x86: Document the Multi-Key Total Memory
> > > > Encryption API
> > > >
> > > > Document the API's used for MKTME on Intel platforms.
> > > > MKTME: Multi-KEY Total Memory Encryption
> > > >
> > > > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > > > ---
> > > >  Documentation/x86/mktme-keys.txt | 153
> > > > +++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 153 insertions(+)
> > > >  create mode 100644 Documentation/x86/mktme-keys.txt
> > > >
> > > > diff --git a/Documentation/x86/mktme-keys.txt
> > > > b/Documentation/x86/mktme- keys.txt new file mode 100644 index
> > > > 000000000000..2dea7acd2a17
> > > > --- /dev/null
> > > > +++ b/Documentation/x86/mktme-keys.txt
> > > > @@ -0,0 +1,153 @@
> > > > +MKTME (Multi-Key Total Memory Encryption) is a technology that
> > > > +allows memory encryption on Intel platforms. Whereas TME (Total
> > > > +Memory
> > > > +Encryption) allows encryption of the entire system memory using a
> > > > +single key, MKTME allows multiple encryption domains, each having
> > > > +their own key. The main use case for the feature is virtual machine
> > > > +isolation. The API's introduced here are intended to offer
> > > > +flexibility to work in a
> > > > wide range of uses.
> > > > +
> > > > +The externally available Intel Architecture Spec:
> > > > +https://software.intel.com/sites/default/files/managed/a5/16/Multi-
> > > > +Key-
> > > > +Total-Memory-Encryption-Spec.pdf
> > > > +
> > > > +============================  API Overview
> > > > +============================
> > > > +
> > > > +There are 2 MKTME specific API's that enable userspace to create
> > > > +and use the memory encryption keys:
> > > > +
> > > > +1) Kernel Key Service: MKTME Type
> > > > +
> > > > +   MKTME is a new key type added to the existing Kernel Key Services
> > > > +   to support the memory encryption keys. The MKTME service manages
> > > > +   the addition and removal of MKTME keys. It maps userspace keys
> > > > +   to hardware keyids and programs the hardware with user requested
> > > > +   encryption parameters.
> > > > +
> > > > +   o An understanding of the Kernel Key Service is required in order
> > > > +     to use the MKTME key type as it is a subset of that service.
> > > > +
> > > > +   o MKTME keys are a limited resource. There is a single pool of
> > > > +     MKTME keys for a system and that pool can be from 3 to 63 keys.
> > >
> > > Why 3 to 63 keys? Architecturally we are able to support up to 15-bit keyID,
> > although in the first generation server we only support 6-bit keyID, which is 63
> > key/keyIDs (excluding keyID 0, which is TME's keyID).
> > 
> > My understanding is that low level SKU's could have as few as 3 bits available to
> > hold the keyid, and that the max is 6 bits, hence 64.
> > I probably don't need to be stating that level of detail here, but rather just
> > iterate the important point that the resource is limited!
> > 
> > >
> > > > +     With that in mind, userspace may take advantage of the kernel
> > > > +     key services sharing and permissions model for userspace keys.
> > > > +     One key can be shared as long as each user has the permission
> > > > +     of "KEY_NEED_VIEW" to use it.
> > > > +
> > > > +   o MKTME key type uses capabilities to restrict the allocation
> > > > +     of keys. It only requires CAP_SYS_RESOURCE, but will accept
> > > > +     the broader capability of CAP_SYS_ADMIN.  See capabilities(7).
> > > > +
> > > > +   o The MKTME key service blocks kernel key service commands that
> > > > +     could lead to reprogramming of in use keys, or loss of keys from
> > > > +     the pool. This means MKTME does not allow a key to be invalidated,
> > > > +     unlinked, or timed out. These operations are blocked by MKTME as
> > > > +     it creates all keys with the internal flag KEY_FLAG_KEEP.
> > > > +
> > > > +   o MKTME does not support the keyctl option of UPDATE. Userspace
> > > > +     may change the programming of a key by revoking it and adding
> > > > +     a new key with the updated encryption options (or vice-versa).
> > > > +
> > > > +2) System Call: encrypt_mprotect()
> > > > +
> > > > +   MKTME encryption is requested by calling encrypt_mprotect(). The
> > > > +   caller passes the serial number to a previously allocated and
> > > > +   programmed encryption key. That handle was created with the MKTME
> > > > +   Key Service.
> > > > +
> > > > +   o The caller must have KEY_NEED_VIEW permission on the key
> > > > +
> > > > +   o The range of memory that is to be protected must be mapped as
> > > > +     ANONYMOUS. If it is not, the entire encrypt_mprotect() request
> > > > +     fails with EINVAL.
> > > > +
> > > > +   o As an extension to the existing mprotect() system call,
> > > > +     encrypt_mprotect() supports the legacy mprotect behavior plus
> > > > +     the enabling of memory encryption. That means that in addition
> > > > +     to encrypting the memory, the protection flags will be updated
> > > > +     as requested in the call.
> > > > +
> > > > +   o Additional mprotect() calls to memory already protected with
> > > > +     MKTME will not alter the MKTME status.
> > >
> > > I think it's better to separate encrypt_mprotect() into another doc so both
> > parts can be reviewed easier.
> > 
> > I can do that.
> > Also, I do know I need man page for that too.
> > >
> > > > +
> > > > +======================  Usage: MKTME Key Service
> > > > +======================
> > > > +
> > > > +MKTME is enabled on supported Intel platforms by selecting
> > > > +CONFIG_X86_INTEL_MKTME which selects CONFIG_MKTME_KEYS.
> > > > +
> > > > +Allocating MKTME Keys via command line or system call:
> > > > +    keyctl add mktme name "[options]" ring
> > > > +
> > > > +    key_serial_t add_key(const char *type, const char *description,
> > > > +                         const void *payload, size_t plen,
> > > > +                         key_serial_t keyring);
> > > > +
> > > > +Revoking MKTME Keys via command line or system call::
> > > > +   keyctl revoke <key>
> > > > +
> > > > +   long keyctl(KEYCTL_REVOKE, key_serial_t key);
> > > > +
> > > > +Options Field Definition:
> > > > +    userkey=      ASCII HEX value encryption key. Defaults to a CPU
> > > > +		  generated key if a userkey is not defined here.
> > > > +
> > > > +    algorithm=    Encryption algorithm name as a string.
> > > > +		  Valid algorithm: "aes-xts-128"
> > > > +
> > > > +    tweak=        ASCII HEX value tweak key. Tweak key will be added to the
> > > > +                  userkey...  (need to be clear here that this is being sent
> > > > +                  to the hardware - kernel not messing w it)
> > > > +
> > > > +    entropy=      ascii hex value entropy.
> > > > +                  This entropy will be used to generated the CPU key and
> > > > +		  the tweak key when CPU generated key is requested.
> > > > +
> > > > +Algorithm Dependencies:
> > > > +    AES-XTS 128 is the only supported algorithm.
> > > > +    There are only 2 ways that AES-XTS 128 may be used:
> > > > +
> > > > +    1) User specified encryption key
> > > > +	- The user specified encryption key must be exactly
> > > > +	  16 ASCII Hex bytes (128 bits).
> > > > +	- A tweak key must be specified and it must be exactly
> > > > +	  16 ASCII Hex bytes (128 bits).
> > > > +	- No entropy field is accepted.
> > > > +
> > > > +    2) CPU generated encryption key
> > > > +	- When no user specified encryption key is provided, the
> > > > +	  default encryption key will be CPU generated.
> > > > +	- User must specify 16 ASCII Hex bytes of entropy. This
> > > > +	  entropy will be used by the CPU to generate both the
> > > > +	  encryption key and the tweak key.
> > > > +	- No entropy field is accepted.
> >              ^^^^^^^ should be tweak
> > 
> > >
> > > This is not true. The spec says in CPU generated random mode, both 'key' and
> > 'tweak' part are used to generate the final key and tweak respectively.
> > >
> > > Actually, simple 'XOR' is used to generate the final key:
> > >
> > > case KEYID_SET_KEY_RANDOM:
> > > 	......
> > > 	(* Mix user supplied entropy to the data key and tweak key *)
> > > 	TMP_RND_DATA_KEY = TMP_RND_KEY XOR
> > > 		TMP_KEY_PROGRAM_STRUCT.KEY_FIELD_1.BYTES[15:0];
> > > 	TMP_RND_TWEAK_KEY = TMP_RND_TWEAK_KEY XOR
> > > 		TMP_KEY_PROGRAM_STRUCT.KEY_FIELD_2.BYTES[15:0];
> > >
> > > So I think we can either just remove 'entropy' parameter, since we can use
> > both 'userkey' and 'tweak' even for random key mode.
> > >
> > > In fact, which might be better IMHO, we can simply disallow or ignore
> > 'userkey' and 'tweak' part for random key mode, since if we allow user to specify
> > both entropies, and if user passes value with all 1, we are effectively making the
> > key and tweak to be all 1, which is not random anymore.
> > >
> > > Instead, kernel can generate random for both entropies, or we can simply uses
> > 0, ignoring user input.
> > 
> > Kai,
> > I think my typo above, threw you off. We have the same understanding of the
> > key fields.
> > 
> > Is this the structure you are suggesting?
> > 
> > 	Options
> > 
> > 	key_type=	"user" or "CPU"
> > 
> > 	key=		If key_type == user
> > 				key= is the data key
> > 			If key_type == CPU
> > 				key= is not required
> > 				if key= is present
> > 					it is entropy to be mixed with
> > 					CPU generated data key
> > 
> > 	tweak=		If key_type == user
> > 				tweak= is the tweak key
> > 			If key_type == CPU
> > 				tweak= is not required
> > 				if tweak= is present
> > 					it is entropy to be mixed with
> > 					CPU generated tweak key
> 
> Exactly.
> 
> Although I am not sure whether we should support other 2 modes: Clear key  and  no encryption;

A hardware key does get CLEAR'ed when the userspace key is revoked.
I don't think we identified any other user directed need to clear a key.

The no encryption option is currently considered not a requirement.
That means, although you see it in the Intel HW Spec, we don't have
use case that is driving us to implement it.

For other's info - no encryption would be an option where the key
tells the hardware not to do any encryption at all on this piece of memory.
All of memory not encrypted with these MKTME keys, is by default, encrypted
with the system level TME, Total Memory Encryption algorithm. (OK - not
really *all*, there is also a BIOS settable exclusion zone for TME)

> 
> Thanks,
> -Kai
> > 
> > 
> > Alison
> > >
> > > Thanks,
> > > -Kai
> > 
> > ........snip...........
> 

Re: [RFC 01/12] docs/x86: Document the Multi-Key Total Memory Encryption API

[Huang, Kai]

On Mon, 2018-09-10 at 17:45 -0700, Alison Schofield wrote:
> On Mon, Sep 10, 2018 at 05:33:33PM -0700, Huang, Kai wrote:
> > > -----Original Message-----
> > > From: owner-linux-mm@kvack.org [mailto:owner-linux-mm@kvack.org]
> > > On
> > > Behalf Of Alison Schofield
> > > Sent: Tuesday, September 11, 2018 12:13 PM
> > > To: Huang, Kai <kai.huang@intel.com>
> > > Cc: dhowells@redhat.com; tglx@linutronix.de; Nakajima, Jun
> > > <jun.nakajima@intel.com>; Shutemov, Kirill <kirill.shutemov@intel
> > > .com>;
> > > Hansen, Dave <dave.hansen@intel.com>; Sakkinen, Jarkko
> > > <jarkko.sakkinen@intel.com>; jmorris@namei.org; keyrings@vger.ker
> > > nel.org;
> > > linux-security-module@vger.kernel.org; mingo@redhat.com; hpa@zyto
> > > r.com;
> > > x86@kernel.org; linux-mm@kvack.org
> > > Subject: Re: [RFC 01/12] docs/x86: Document the Multi-Key Total
> > > Memory
> > > Encryption API
> > > 
> > > On Sun, Sep 09, 2018 at 06:28:28PM -0700, Huang, Kai wrote:
> > > > 
> > > > > -----Original Message-----
> > > > > From: owner-linux-mm@kvack.org [mailto:owner-linux-mm@kvack.o
> > > > > rg] On
> > > > > Behalf Of Alison Schofield
> > > > > Sent: Saturday, September 8, 2018 10:34 AM
> > > > > To: dhowells@redhat.com; tglx@linutronix.de
> > > > > Cc: Huang, Kai <kai.huang@intel.com>; Nakajima, Jun
> > > > > <jun.nakajima@intel.com>; Shutemov, Kirill
> > > > > <kirill.shutemov@intel.com>; Hansen, Dave <dave.hansen@intel.
> > > > > com>;
> > > > > Sakkinen, Jarkko <jarkko.sakkinen@intel.com>; jmorris@namei.o
> > > > > rg;
> > > > > keyrings@vger.kernel.org; linux-security-module@vger.kernel.o
> > > > > rg;
> > > > > mingo@redhat.com; hpa@zytor.com; x86@kernel.org; linux-
> > > 
> > > mm@kvack.org
> > > > > Subject: [RFC 01/12] docs/x86: Document the Multi-Key Total
> > > > > Memory
> > > > > Encryption API
> > > > > 
> > > > > Document the API's used for MKTME on Intel platforms.
> > > > > MKTME: Multi-KEY Total Memory Encryption
> > > > > 
> > > > > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > > > > ---
> > > > >  Documentation/x86/mktme-keys.txt | 153
> > > > > +++++++++++++++++++++++++++++++++++++++
> > > > >  1 file changed, 153 insertions(+)
> > > > >  create mode 100644 Documentation/x86/mktme-keys.txt
> > > > > 
> > > > > diff --git a/Documentation/x86/mktme-keys.txt
> > > > > b/Documentation/x86/mktme- keys.txt new file mode 100644
> > > > > index
> > > > > 000000000000..2dea7acd2a17
> > > > > --- /dev/null
> > > > > +++ b/Documentation/x86/mktme-keys.txt
> > > > > @@ -0,0 +1,153 @@
> > > > > +MKTME (Multi-Key Total Memory Encryption) is a technology
> > > > > that
> > > > > +allows memory encryption on Intel platforms. Whereas TME
> > > > > (Total
> > > > > +Memory
> > > > > +Encryption) allows encryption of the entire system memory
> > > > > using a
> > > > > +single key, MKTME allows multiple encryption domains, each
> > > > > having
> > > > > +their own key. The main use case for the feature is virtual
> > > > > machine
> > > > > +isolation. The API's introduced here are intended to offer
> > > > > +flexibility to work in a
> > > > > wide range of uses.
> > > > > +
> > > > > +The externally available Intel Architecture Spec:
> > > > > +https://software.intel.com/sites/default/files/managed/a5/16
> > > > > /Multi-
> > > > > +Key-
> > > > > +Total-Memory-Encryption-Spec.pdf
> > > > > +
> > > > > +============================  API Overview
> > > > > +============================
> > > > > +
> > > > > +There are 2 MKTME specific API's that enable userspace to
> > > > > create
> > > > > +and use the memory encryption keys:
> > > > > +
> > > > > +1) Kernel Key Service: MKTME Type
> > > > > +
> > > > > +   MKTME is a new key type added to the existing Kernel Key
> > > > > Services
> > > > > +   to support the memory encryption keys. The MKTME service
> > > > > manages
> > > > > +   the addition and removal of MKTME keys. It maps userspace
> > > > > keys
> > > > > +   to hardware keyids and programs the hardware with user
> > > > > requested
> > > > > +   encryption parameters.
> > > > > +
> > > > > +   o An understanding of the Kernel Key Service is required
> > > > > in order
> > > > > +     to use the MKTME key type as it is a subset of that
> > > > > service.
> > > > > +
> > > > > +   o MKTME keys are a limited resource. There is a single
> > > > > pool of
> > > > > +     MKTME keys for a system and that pool can be from 3 to
> > > > > 63 keys.
> > > > 
> > > > Why 3 to 63 keys? Architecturally we are able to support up to
> > > > 15-bit keyID,
> > > 
> > > although in the first generation server we only support 6-bit
> > > keyID, which is 63
> > > key/keyIDs (excluding keyID 0, which is TME's keyID).
> > > 
> > > My understanding is that low level SKU's could have as few as 3
> > > bits available to
> > > hold the keyid, and that the max is 6 bits, hence 64.
> > > I probably don't need to be stating that level of detail here,
> > > but rather just
> > > iterate the important point that the resource is limited!
> > > 
> > > > 
> > > > > +     With that in mind, userspace may take advantage of the
> > > > > kernel
> > > > > +     key services sharing and permissions model for
> > > > > userspace keys.
> > > > > +     One key can be shared as long as each user has the
> > > > > permission
> > > > > +     of "KEY_NEED_VIEW" to use it.
> > > > > +
> > > > > +   o MKTME key type uses capabilities to restrict the
> > > > > allocation
> > > > > +     of keys. It only requires CAP_SYS_RESOURCE, but will
> > > > > accept
> > > > > +     the broader capability of CAP_SYS_ADMIN.  See
> > > > > capabilities(7).
> > > > > +
> > > > > +   o The MKTME key service blocks kernel key service
> > > > > commands that
> > > > > +     could lead to reprogramming of in use keys, or loss of
> > > > > keys from
> > > > > +     the pool. This means MKTME does not allow a key to be
> > > > > invalidated,
> > > > > +     unlinked, or timed out. These operations are blocked by
> > > > > MKTME as
> > > > > +     it creates all keys with the internal flag
> > > > > KEY_FLAG_KEEP.
> > > > > +
> > > > > +   o MKTME does not support the keyctl option of UPDATE.
> > > > > Userspace
> > > > > +     may change the programming of a key by revoking it and
> > > > > adding
> > > > > +     a new key with the updated encryption options (or vice-
> > > > > versa).
> > > > > +
> > > > > +2) System Call: encrypt_mprotect()
> > > > > +
> > > > > +   MKTME encryption is requested by calling
> > > > > encrypt_mprotect(). The
> > > > > +   caller passes the serial number to a previously allocated
> > > > > and
> > > > > +   programmed encryption key. That handle was created with
> > > > > the MKTME
> > > > > +   Key Service.
> > > > > +
> > > > > +   o The caller must have KEY_NEED_VIEW permission on the
> > > > > key
> > > > > +
> > > > > +   o The range of memory that is to be protected must be
> > > > > mapped as
> > > > > +     ANONYMOUS. If it is not, the entire encrypt_mprotect()
> > > > > request
> > > > > +     fails with EINVAL.
> > > > > +
> > > > > +   o As an extension to the existing mprotect() system call,
> > > > > +     encrypt_mprotect() supports the legacy mprotect
> > > > > behavior plus
> > > > > +     the enabling of memory encryption. That means that in
> > > > > addition
> > > > > +     to encrypting the memory, the protection flags will be
> > > > > updated
> > > > > +     as requested in the call.
> > > > > +
> > > > > +   o Additional mprotect() calls to memory already protected
> > > > > with
> > > > > +     MKTME will not alter the MKTME status.
> > > > 
> > > > I think it's better to separate encrypt_mprotect() into another
> > > > doc so both
> > > 
> > > parts can be reviewed easier.
> > > 
> > > I can do that.
> > > Also, I do know I need man page for that too.
> > > > 
> > > > > +
> > > > > +======================  Usage: MKTME Key Service
> > > > > +======================
> > > > > +
> > > > > +MKTME is enabled on supported Intel platforms by selecting
> > > > > +CONFIG_X86_INTEL_MKTME which selects CONFIG_MKTME_KEYS.
> > > > > +
> > > > > +Allocating MKTME Keys via command line or system call:
> > > > > +    keyctl add mktme name "[options]" ring
> > > > > +
> > > > > +    key_serial_t add_key(const char *type, const char
> > > > > *description,
> > > > > +                         const void *payload, size_t plen,
> > > > > +                         key_serial_t keyring);
> > > > > +
> > > > > +Revoking MKTME Keys via command line or system call::
> > > > > +   keyctl revoke <key>
> > > > > +
> > > > > +   long keyctl(KEYCTL_REVOKE, key_serial_t key);
> > > > > +
> > > > > +Options Field Definition:
> > > > > +    userkey=      ASCII HEX value encryption key. Defaults
> > > > > to a CPU
> > > > > +		  generated key if a userkey is not defined
> > > > > here.
> > > > > +
> > > > > +    algorithm=    Encryption algorithm name as a string.
> > > > > +		  Valid algorithm: "aes-xts-128"
> > > > > +
> > > > > +    tweak=        ASCII HEX value tweak key. Tweak key will
> > > > > be added to the
> > > > > +                  userkey...  (need to be clear here that
> > > > > this is being sent
> > > > > +                  to the hardware - kernel not messing w it)
> > > > > +
> > > > > +    entropy=      ascii hex value entropy.
> > > > > +                  This entropy will be used to generated the
> > > > > CPU key and
> > > > > +		  the tweak key when CPU generated key is
> > > > > requested.
> > > > > +
> > > > > +Algorithm Dependencies:
> > > > > +    AES-XTS 128 is the only supported algorithm.
> > > > > +    There are only 2 ways that AES-XTS 128 may be used:
> > > > > +
> > > > > +    1) User specified encryption key
> > > > > +	- The user specified encryption key must be exactly
> > > > > +	  16 ASCII Hex bytes (128 bits).
> > > > > +	- A tweak key must be specified and it must be
> > > > > exactly
> > > > > +	  16 ASCII Hex bytes (128 bits).
> > > > > +	- No entropy field is accepted.
> > > > > +
> > > > > +    2) CPU generated encryption key
> > > > > +	- When no user specified encryption key is provided,
> > > > > the
> > > > > +	  default encryption key will be CPU generated.
> > > > > +	- User must specify 16 ASCII Hex bytes of entropy.
> > > > > This
> > > > > +	  entropy will be used by the CPU to generate both
> > > > > the
> > > > > +	  encryption key and the tweak key.
> > > > > +	- No entropy field is accepted.
> > > 
> > >              ^^^^^^^ should be tweak
> > > 
> > > > 
> > > > This is not true. The spec says in CPU generated random mode,
> > > > both 'key' and
> > > 
> > > 'tweak' part are used to generate the final key and tweak
> > > respectively.
> > > > 
> > > > Actually, simple 'XOR' is used to generate the final key:
> > > > 
> > > > case KEYID_SET_KEY_RANDOM:
> > > > 	......
> > > > 	(* Mix user supplied entropy to the data key and tweak
> > > > key *)
> > > > 	TMP_RND_DATA_KEY = TMP_RND_KEY XOR
> > > > 		TMP_KEY_PROGRAM_STRUCT.KEY_FIELD_1.BYTES[15:0];
> > > > 	TMP_RND_TWEAK_KEY = TMP_RND_TWEAK_KEY XOR
> > > > 		TMP_KEY_PROGRAM_STRUCT.KEY_FIELD_2.BYTES[15:0];
> > > > 
> > > > So I think we can either just remove 'entropy' parameter, since
> > > > we can use
> > > 
> > > both 'userkey' and 'tweak' even for random key mode.
> > > > 
> > > > In fact, which might be better IMHO, we can simply disallow or
> > > > ignore
> > > 
> > > 'userkey' and 'tweak' part for random key mode, since if we allow
> > > user to specify
> > > both entropies, and if user passes value with all 1, we are
> > > effectively making the
> > > key and tweak to be all 1, which is not random anymore.
> > > > 
> > > > Instead, kernel can generate random for both entropies, or we
> > > > can simply uses
> > > 
> > > 0, ignoring user input.
> > > 
> > > Kai,
> > > I think my typo above, threw you off. We have the same
> > > understanding of the
> > > key fields.
> > > 
> > > Is this the structure you are suggesting?
> > > 
> > > 	Options
> > > 
> > > 	key_type=	"user" or "CPU"
> > > 
> > > 	key=		If key_type == user
> > > 				key= is the data key
> > > 			If key_type == CPU
> > > 				key= is not required
> > > 				if key= is present
> > > 					it is entropy to be mixed with
> > > 					CPU generated data key
> > > 
> > > 	tweak=		If key_type == user
> > > 				tweak= is the tweak key
> > > 			If key_type == CPU
> > > 				tweak= is not required
> > > 				if tweak= is present
> > > 					it is entropy to be mixed with
> > > 					CPU generated tweak key
> > 
> > Exactly.
> > 
> > Although I am not sure whether we should support other 2 modes:
> > Clear key  and  no encryption;
> 
> A hardware key does get CLEAR'ed when the userspace key is revoked.
> I don't think we identified any other user directed need to clear a
> key.
> 
> The no encryption option is currently considered not a requirement.
> That means, although you see it in the Intel HW Spec, we don't have
> use case that is driving us to implement it.
> 
> For other's info - no encryption would be an option where the key
> tells the hardware not to do any encryption at all on this piece of
> memory.
> All of memory not encrypted with these MKTME keys, is by default,
> encrypted
> with the system level TME, Total Memory Encryption algorithm. (OK -
> not
> really *all*, there is also a BIOS settable exclusion zone for TME)

Agreed. Let's focus on user mode and CPU mode for now.

Thanks,
-Kai
> 
> > 
> > Thanks,
> > -Kai
> > > 
> > > 
> > > Alison
> > > > 
> > > > Thanks,
> > > > -Kai
> > > 
> > > ........snip...........

Re: [RFC 06/12] mm: Add the encrypt_mprotect() system call

[Alison Schofield]

On Mon, Sep 10, 2018 at 09:02:43PM +0300, Jarkko Sakkinen wrote:
> On Fri, 2018-09-07 at 15:36 -0700, Alison Schofield wrote:
> > Implement memory encryption with a new system call that is an
> > extension of the legacy mprotect() system call.
> > 
> > In encrypt_mprotect the caller must pass a handle to a previously
> > allocated and programmed encryption key. Validate the key and store
> > the keyid bits in the vm_page_prot for each VMA in the protection
> > range.
> > 
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > ---
> >  fs/exec.c           |  4 ++--
> >  include/linux/key.h |  2 ++
> >  include/linux/mm.h  |  3 ++-
> >  mm/mprotect.c       | 67 ++++++++++++++++++++++++++++++++++++++++++++++++--
> > ---
> >  4 files changed, 67 insertions(+), 9 deletions(-)
> > 
> > diff --git a/fs/exec.c b/fs/exec.c
> > index a1a246062561..b681a413db9c 100644
> > --- a/fs/exec.c
> > +++ b/fs/exec.c
> > @@ -754,8 +754,8 @@ int setup_arg_pages(struct linux_binprm *bprm,
> >  	vm_flags |= mm->def_flags;
> >  	vm_flags |= VM_STACK_INCOMPLETE_SETUP;
> >  
> > -	ret = mprotect_fixup(vma, &prev, vma->vm_start, vma->vm_end,
> > -			vm_flags);
> > +	ret = mprotect_fixup(vma, &prev, vma->vm_start, vma->vm_end,
> > vm_flags,
> > +			     -1);
> 
> Why you pass a magic number here when you went the trouble having
> a named constant?

The named constant isn't available over here in fs/exec.c.
The special case is documented further down ... (scroll down)
That caller has no knowledge of proctection or encryption keys.

> 
> >  	if (ret)
> >  		goto out_unlock;
> >  	BUG_ON(prev != vma);
> > diff --git a/include/linux/key.h b/include/linux/key.h
> > index e58ee10f6e58..fb8a7d5f6149 100644
> > --- a/include/linux/key.h
> > +++ b/include/linux/key.h
> > @@ -346,6 +346,8 @@ static inline key_serial_t key_serial(const struct key
> > *key)
> >  
> >  extern void key_set_timeout(struct key *, unsigned);
> >  
> > +extern key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
> > +				 key_perm_t perm);
> >  /*
> >   * The permissions required on a key that we're looking up.
> >   */
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index ac85c0805761..0f9422c7841e 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -1579,7 +1579,8 @@ extern unsigned long change_protection(struct
> > vm_area_struct *vma, unsigned long
> >  			      int dirty_accountable, int prot_numa);
> >  extern int mprotect_fixup(struct vm_area_struct *vma,
> >  			  struct vm_area_struct **pprev, unsigned long start,
> > -			  unsigned long end, unsigned long newflags);
> > +			  unsigned long end, unsigned long newflags,
> > +			  int newkeyid);
> >  
> >  /*
> >   * doesn't attempt to fault and will return short.
> > diff --git a/mm/mprotect.c b/mm/mprotect.c
> > index 56e64ef7931e..6c2e1106525c 100644
> > --- a/mm/mprotect.c
> > +++ b/mm/mprotect.c
> > @@ -28,14 +28,17 @@
> >  #include <linux/ksm.h>
> >  #include <linux/uaccess.h>
> >  #include <linux/mm_inline.h>
> > +#include <linux/key.h>
> >  #include <asm/pgtable.h>
> >  #include <asm/cacheflush.h>
> >  #include <asm/mmu_context.h>
> >  #include <asm/tlbflush.h>
> > +#include <asm/mktme.h>
> >  
> >  #include "internal.h"
> >  
> >  #define NO_PKEY  -1
> > +#define NO_KEYID -1
> 
> Should have only single named constant IMHO. This ambiguity
> is worse than some reasonable constant name for both cases.
> Maybe NO_KEYID would be adequate?

Yes, this could be NO_KEY for both. I was worried about readability,
but most of the usages compare it to a well-named variable, as in 
(pkey == NO_KEY) or (keyid == NO_KEY) so it seems to work. 
Will do!

> 
> >  
> >  static unsigned long change_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
> >  		unsigned long addr, unsigned long end, pgprot_t newprot,
> > @@ -310,7 +313,8 @@ unsigned long change_protection(struct vm_area_struct
> > *vma, unsigned long start,
> >  
> >  int
> >  mprotect_fixup(struct vm_area_struct *vma, struct vm_area_struct **pprev,
> > -	unsigned long start, unsigned long end, unsigned long newflags)
> > +	       unsigned long start, unsigned long end, unsigned long
> > newflags,
> > +	       int newkeyid)
> >  {
> >  	struct mm_struct *mm = vma->vm_mm;
> >  	unsigned long oldflags = vma->vm_flags;
> > @@ -320,10 +324,24 @@ mprotect_fixup(struct vm_area_struct *vma, struct
> > vm_area_struct **pprev,
> >  	int error;
> >  	int dirty_accountable = 0;
> >  
> > +	/*
> > +	 * Flags match and Keyids match or we have NO_KEYID.
> > +	 * This _fixup is usually called from do_mprotect_ext() except
> > +	 * for one special case: caller fs/exec.c/setup_arg_pages()
> > +	 * In that case, newkeyid is passed as -1 (NO_KEYID).
> > +	 */

Above is that special case explanation.

> > +	if (newflags == oldflags &&
> > +	    (newkeyid == vma_keyid(vma) || newkeyid == NO_KEYID)) {
> > +		*pprev = vma;
> > +		return 0;
> > +	}
> > +	/* Flags match and Keyid changes */
> >  	if (newflags == oldflags) {
> > +		mprotect_set_encrypt(vma, newkeyid);
> >  		*pprev = vma;
> >  		return 0;
> >  	}
> > +	/* Flags and Keyids both change, continue. */
> >  
> >  	/*
> >  	 * If we make a private mapping writable we increase our commit;
> > @@ -373,6 +391,8 @@ mprotect_fixup(struct vm_area_struct *vma, struct
> > vm_area_struct **pprev,
> >  	}
> >  
> >  success:
> > +	if (newkeyid != NO_KEYID)
> > +		mprotect_set_encrypt(vma, newkeyid);
> >  	/*
> >  	 * vm_flags and vm_page_prot are protected by the mmap_sem
> >  	 * held in write mode.
> > @@ -404,10 +424,15 @@ mprotect_fixup(struct vm_area_struct *vma, struct
> > vm_area_struct **pprev,
> >  }
> >  
> >  /*
> > - * When pkey==NO_PKEY we get legacy mprotect behavior here.
> > + * do_mprotect_ext() supports the legacy mprotect behavior plus extensions
> > + * for protection keys and memory encryption keys. These extensions are
> > + * mutually exclusive and the behavior is:
> > + *	(pkey==NO_PKEY && keyid==NO_KEYID) ==> legacy mprotect
> > + *	(pkey is valid)  ==> legacy mprotect plus protection key extensions
> > + *	(keyid is valid) ==> legacy mprotect plus encryption key extensions
> >   */
> 
> The header does not follow
> 
> https://www.kernel.org/doc/Documentation/kernel-doc-nano-HOWTO.txt

Jarkko,

I see the "how to" on kernel doc formatting, but I'm not getting the
when? When should it be done? I can imagine it for new code, like
the mktme_keys.c where new code is being added and the kernel docs
could give a complete narrative of the module.

Here, in mprotect.c, there are no kernel doc comments. I realize
sometime we just need to start somewhere ;), but I find it odd to
pluck out one function header to be extracted for kernel docs.

Alison

> 
> >  static int do_mprotect_ext(unsigned long start, size_t len,
> > -		unsigned long prot, int pkey)
> > +			   unsigned long prot, int pkey, int keyid)
> >  {
> >  	unsigned long nstart, end, tmp, reqprot;
> >  	struct vm_area_struct *vma, *prev;
> > @@ -505,7 +530,8 @@ static int do_mprotect_ext(unsigned long start, size_t
> > len,
> >  		tmp = vma->vm_end;
> >  		if (tmp > end)
> >  			tmp = end;
> > -		error = mprotect_fixup(vma, &prev, nstart, tmp, newflags);
> > +		error = mprotect_fixup(vma, &prev, nstart, tmp, newflags,
> > +				       keyid);
> >  		if (error)
> >  			goto out;
> >  		nstart = tmp;
> > @@ -530,7 +556,7 @@ static int do_mprotect_ext(unsigned long start, size_t
> > len,
> >  SYSCALL_DEFINE3(mprotect, unsigned long, start, size_t, len,
> >  		unsigned long, prot)
> >  {
> > -	return do_mprotect_ext(start, len, prot, NO_PKEY);
> > +	return do_mprotect_ext(start, len, prot, NO_PKEY, NO_KEYID);
> >  }
> >  
> >  #ifdef CONFIG_ARCH_HAS_PKEYS
> > @@ -538,7 +564,7 @@ SYSCALL_DEFINE3(mprotect, unsigned long, start, size_t,
> > len,
> >  SYSCALL_DEFINE4(pkey_mprotect, unsigned long, start, size_t, len,
> >  		unsigned long, prot, int, pkey)
> >  {
> > -	return do_mprotect_ext(start, len, prot, pkey);
> > +	return do_mprotect_ext(start, len, prot, pkey, NO_KEYID);
> >  }
> >  
> >  SYSCALL_DEFINE2(pkey_alloc, unsigned long, flags, unsigned long, init_val)
> > @@ -587,3 +613,32 @@ SYSCALL_DEFINE1(pkey_free, int, pkey)
> >  }
> >  
> >  #endif /* CONFIG_ARCH_HAS_PKEYS */
> > +
> > +#ifdef CONFIG_X86_INTEL_MKTME
> > +
> > +SYSCALL_DEFINE4(encrypt_mprotect, unsigned long, start, size_t, len,
> > +		unsigned long, prot, key_serial_t, serial)
> > +{
> > +	key_ref_t key_ref;
> > +	int ret, keyid;
> > +
> > +	/* TODO MKTME key service must be initialized */
> > +
> > +	key_ref = lookup_user_key(serial, 0, KEY_NEED_VIEW);
> > +	if (IS_ERR(key_ref))
> > +		return PTR_ERR(key_ref);
> > +
> > +	mktme_map_lock();
> > +	keyid = mktme_map_keyid_from_serial(serial);
> > +	if (!keyid) {
> > +		mktme_map_unlock();
> > +		key_ref_put(key_ref);
> > +		return -EINVAL;
> > +	}
> > +	ret = do_mprotect_ext(start, len, prot, NO_PKEY, keyid);
> > +	mktme_map_unlock();
> > +	key_ref_put(key_ref);
> > +	return ret;
> > +}
> > +
> > +#endif /* CONFIG_X86_INTEL_MKTME */
> 
> /Jarkko

Re: [RFC 08/12] mm: Track VMA's in use for each memory encryption keyid

[Alison Schofield]

On Mon, Sep 10, 2018 at 09:20:45PM +0300, Jarkko Sakkinen wrote:
> On Fri, 2018-09-07 at 15:37 -0700, Alison Schofield wrote:
> > Keep track of the VMA's oustanding for each memory encryption keyid.
> > The count is used by the MKTME (Multi-Key Total Memory Encryption)
> > Key Service to determine when it is safe to reprogram a hardware
> > encryption key.
> 
> Maybe a stupid question but why they are tracked and what do you 
> mean by tracking?
> 
> /Jarkko

Perhaps 'Keep a count of' instead of 'Keep track of' will be clearer.

Counting VMA's using each keyid prevents in use keys from being cleared
and reused. The counting is done here, and the mtkme key service checks
these counts to decide if it is OK to allow a userspace key to be revoked.
A successful userspace key revoke will clear the hardware keyid slot and
leave the key available to be reprogrammed.

> 
> > Approach here is to do gets and puts on the encryption reference
> > wherever kmem_cache_alloc/free's of vma_area_cachep's are executed.
> > A couple of these locations will not be hit until cgroup support is
> > added. One of these locations should never hit, so use a VM_WARN_ON.
> > 
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > ---
> >  arch/x86/mm/mktme.c |  2 ++
> >  kernel/fork.c       |  2 ++
> >  mm/mmap.c           | 12 ++++++++++++
> >  mm/nommu.c          |  4 ++++
> >  4 files changed, 20 insertions(+)
> > 

.... snip ....

Re: [RFC 10/12] x86/pconfig: Program memory encryption keys on a system-wide basis

[Alison Schofield]

On Mon, Sep 10, 2018 at 11:24:20AM -0700, Sakkinen, Jarkko wrote:
> On Fri, 2018-09-07 at 15:38 -0700, Alison Schofield wrote:
> > The kernel manages the MKTME (Multi-Key Total Memory Encryption) Keys
> > as a system wide single pool of keys. The hardware, however, manages
> > the keys on a per physical package basis. Each physical package
> > maintains a key table that all CPU's in that package share.
> > 
> > In order to maintain the consistent, system wide view that the kernel
> > requires, program all physical packages during a key program request.
> > 
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> 
> Just kind of checking that are you talking about multiple cores in
> a single package or really multiple packages?

System wide pool.
System has multiple packages.  
Packages have multiple CPU's.

The hardware KEY TABLE is per package. I need that per package KEY TABLE
to be the same in every package across the system. So, I pick one 'lead'
CPU in each package to program that packages KEY TABLE.

(BTW - I'm going to look into Kai's suggestion to move the system wide view
of this key programming into the key service. Not sure if that's a go.)

> 
> /Jarkko

Re: [RFC 00/12] Multi-Key Total Memory Encryption API (MKTME)

[Huang, Kai]

On Mon, 2018-09-10 at 12:10 -0700, Alison Schofield wrote:
> On Sun, Sep 09, 2018 at 06:10:19PM -0700, Huang, Kai wrote:
> > 
> > > -----Original Message-----
> > > From: keyrings-owner@vger.kernel.org [mailto:keyrings-
> > > owner@vger.kernel.org] On Behalf Of Alison Schofield
> > > Sent: Saturday, September 8, 2018 10:23 AM
> > > To: dhowells@redhat.com; tglx@linutronix.de
> > > Cc: Huang, Kai <kai.huang@intel.com>; Nakajima, Jun
> > > <jun.nakajima@intel.com>; Shutemov, Kirill <kirill.shutemov@intel
> > > .com>;
> > > Hansen, Dave <dave.hansen@intel.com>; Sakkinen, Jarkko
> > > <jarkko.sakkinen@intel.com>; jmorris@namei.org; keyrings@vger.ker
> > > nel.org;
> > > linux-security-module@vger.kernel.org; mingo@redhat.com; hpa@zyto
> > > r.com;
> > > x86@kernel.org; linux-mm@kvack.org
> > > Subject: [RFC 00/12] Multi-Key Total Memory Encryption API
> > > (MKTME)
> > > 
> > > Seeking comments on the APIs supporting MKTME on future Intel
> > > platforms.
> > > 
> > > MKTME (Multi-Key Total Memory Encryption) is a technology
> > > supporting
> > > memory encryption on upcoming Intel platforms. Whereas TME allows
> > > encryption of the entire system memory using a single key, MKTME
> > > allows
> > > mulitple encryption domains, each having their own key. While the
> > > main use
> > > case for the feature is virtual machine isolation, the API needs
> > > the flexibility to
> > > work for a wide range of use cases.
> > > 
> > > This RFC presents the 2 API additions that enable userspace to:
> > >  1) Create Encryption Keys: Kernel Key Service type "mktme"
> > >  2) Use the Encryption Keys: system call encrypt_mprotect()
> > > 
> > > In order to share between: the Kernel Key Service, the new system
> > > call, and the
> > > existing mm code, helper functions were created in arch/x86/mktme
> > 
> > IMHO, we can separate this series into 2 parts, as you did above,
> > and send out them separately. The reason is, in general I think
> > adding new MKTME type to key retention services is not that related
> > to memory management code, namely the encrypt_mprotect() API part.
> > 
> > So if we split the two parts and send them out separately, the
> > first part can be reviewed by keyring and security guys, without
> > involving mm guys, and the encrypt_mprotect() part can be more
> > reviewed more by mm guys. 
> > 
> 
> Kai,
> 
> That was the direction I had in mind at the onset: the MKTME key
> service
> would be one patch(set) and the MKTME encrypt_mprotect() system call
> would
> be delivered in another patch(set).
> 
> That separation falls apart when the shared structures and functions
> are
> introduced. That 'mktme_map' (maps userspace keys to hardware keyid
> slots),
> and the 'encrypt_count' array (counts vma's outstanding for each key)
> need
> to be shared by both pieces. These mktme special shared structures
> and the
> functions that operate on them are all defined in
> arch/x86/mm/mktme.h,.c.
> From there they can be shared with the security/keys/mktme_keys.c
> 
> Once I made that separation, I stuck with it. Those structures, and
> any
> functions that manipulate those structures live in
> arch/x86/mm/mktme.h,c
> 
> You noted that some of the functions that operate on the
> encrypt_count
> might not need to be over in arch/x86/mm/mktme.c because they are not
> used
> in the mm code. That is true.  

Yes IMO this is better.

> But, then I'd be splitting up the definition
> of the struct and the funcs that operate on it. So, I stuck with
> keeping it
> all together in the arch specific mktme files.

Definition can also be in include/keys/mktme-type.h, right?

> 
> Having said all the above, I do welcome other ideas on how to better
> organize
> the code. 
> 
> Back to your request- to split it into smaller patchsets might look
> something
> like:
> 1) the MKTME API helpers

Exactly what helpers?

> 2) the MKTME Key Service
> 3) the MKTME syscall encrypt_mprotect()
> 
> I'm not clear that would make anyones review life easier, than
> picking
> the same pieces out of the greater patchset.

Well, the reason I suggested we should split key retention services
from other staff and get it reviewed first is I think the
functionalities of MKTME key type has nothing to do w/ MM code. IMHO
what MKTME key type should provide includes:

1) Support add_key, keyctl (revoke, etc) obviously. THis also implies
keyID allocation, so you will need some info from MM or x86 code to get
 info such as number of keyIDs, etc. In fact I think number of keyIDs
is  the only info you need (0 keyIDs means MKTME is disabled).

2) You need to provide some APIs to munipulate key reference count.
Those functions can be declared in mktme-type.h, and implemented in
mktme_keys.c. I think this is more reasonable, as logically reference
count is part of MKTME key service.

I don't think you need to do anything else in MKTME key type?

I think you can even start upstreaming MKTME key type now w/o Kirill's
core-MKTME support, since 1) adding MKTME key type doesn't do any harm
w/o kirill's core-MKTME code; 2) logically MKTME key type has no true
dependency on core-MKTME code.

So IMHO, a better structure should be:

1) Current upstream code (maybe with some early detction code change,
ie, to disable MKTME for some SW reason, but I am not sure whether this
is truely needed, since when MKTME key type gets initialized, that part
should been done already).
2) MKTME key type
3) Kirill's core-MKTME
4) new syscall

And I think CONFIG_MKTME_KEYS doesn't really need to depend on
CONFIG_INTEL_MKTME at this point, although we can add such if needed
when kirill's core-MKTME code gets upstreamed.

Thanks,
-Kai
> 
> Suggestions welcome,
> Alison
> 
> > And since encrypt_mprotect() is a new syscall, you may need to add
> > more lists for the review, ie, linux-api, and maybe linux-kernel as
> > well.
> 
> Got it. Will include theses in v1.
> 
> > 
> > Thanks,
> > -Kai
> > 
> > > 
> > > This patchset is built upon Kirill Shutemov's patchset for the
> > > core MKTME
> > > support. You can find that here:
> > > git://git.kernel.org/pub/scm/linux/kernel/git/kas/linux.git
> > > mktme/wip
> > > 
> > > 
> > > Alison Schofield (12):
> > >   docs/x86: Document the Multi-Key Total Memory Encryption API
> > >   mm: Generalize the mprotect implementation to support
> > > extensions
> > >   syscall/x86: Wire up a new system call for memory encryption
> > > keys
> > >   x86/mm: Add helper functions to manage memory encryption keys
> > >   x86/mm: Add a helper function to set keyid bits in encrypted
> > > VMA's
> > >   mm: Add the encrypt_mprotect() system call
> > >   x86/mm: Add helper functions to track encrypted VMA's
> > >   mm: Track VMA's in use for each memory encryption keyid
> > >   mm: Restrict memory encryption to anonymous VMA's
> > >   x86/pconfig: Program memory encryption keys on a system-wide
> > > basis
> > >   keys/mktme: Add a new key service type for memory encryption
> > > keys
> > >   keys/mktme: Do not revoke in use memory encryption keys
> > > 
> > >  Documentation/x86/mktme-keys.txt       | 153 ++++++++++++++++
> > >  arch/x86/Kconfig                       |   1 +
> > >  arch/x86/entry/syscalls/syscall_32.tbl |   1 +
> > >  arch/x86/entry/syscalls/syscall_64.tbl |   1 +
> > >  arch/x86/include/asm/intel_pconfig.h   |  42 ++++-
> > >  arch/x86/include/asm/mktme.h           |  21 +++
> > >  arch/x86/mm/mktme.c                    | 141 ++++++++++++++
> > >  fs/exec.c                              |   4 +-
> > >  include/keys/mktme-type.h              |  28 +++
> > >  include/linux/key.h                    |   2 +
> > >  include/linux/mm.h                     |   9 +-
> > >  include/linux/syscalls.h               |   2 +
> > >  include/uapi/asm-generic/unistd.h      |   4 +-
> > >  kernel/fork.c                          |   2 +
> > >  kernel/sys_ni.c                        |   2 +
> > >  mm/mmap.c                              |  12 ++
> > >  mm/mprotect.c                          |  93 +++++++++-
> > >  mm/nommu.c                             |   4 +
> > >  security/keys/Kconfig                  |  11 ++
> > >  security/keys/Makefile                 |   1 +
> > >  security/keys/internal.h               |   6 +
> > >  security/keys/keyctl.c                 |   7 +
> > >  security/keys/mktme_keys.c             | 325
> > > +++++++++++++++++++++++++++++++++
> > >  23 files changed, 855 insertions(+), 17 deletions(-)  create
> > > mode 100644
> > > Documentation/x86/mktme-keys.txt  create mode 100644
> > > include/keys/mktme-
> > > type.h  create mode 100644 security/keys/mktme_keys.c
> > > 
> > > --
> > > 2.14.1

Re: [RFC 10/12] x86/pconfig: Program memory encryption keys on a system-wide basis

[Jarkko Sakkinen]

On Mon, Sep 10, 2018 at 07:46:57PM -0700, Alison Schofield wrote:
> On Mon, Sep 10, 2018 at 11:24:20AM -0700, Sakkinen, Jarkko wrote:
> > On Fri, 2018-09-07 at 15:38 -0700, Alison Schofield wrote:
> > > The kernel manages the MKTME (Multi-Key Total Memory Encryption) Keys
> > > as a system wide single pool of keys. The hardware, however, manages
> > > the keys on a per physical package basis. Each physical package
> > > maintains a key table that all CPU's in that package share.
> > > 
> > > In order to maintain the consistent, system wide view that the kernel
> > > requires, program all physical packages during a key program request.
> > > 
> > > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > 
> > Just kind of checking that are you talking about multiple cores in
> > a single package or really multiple packages?
> 
> System wide pool.
> System has multiple packages.  
> Packages have multiple CPU's.
> 
> The hardware KEY TABLE is per package. I need that per package KEY TABLE
> to be the same in every package across the system. So, I pick one 'lead'
> CPU in each package to program that packages KEY TABLE.
> 
> (BTW - I'm going to look into Kai's suggestion to move the system wide view
> of this key programming into the key service. Not sure if that's a go.)

Thanks. I think could be perhaps a fair addition to the documentation?

/Jarkko

Re: [RFC 11/12] keys/mktme: Add a new key service type for memory encryption keys

[David Howells]

Alison Schofield <alison.schofield@intel.com> wrote:

> +/* Key Service Command: Creates a software key and programs hardware */
> +int mktme_instantiate(struct key *key, struct key_preparsed_payload *prep)
> +{
> +	struct mktme_key_program *kprog = NULL;
> +	size_t datalen = prep->datalen;
> +	char *options;
> +	int ret = 0;
> +
> +	if (!capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
> +		return -EACCES;
> +
> +	if (datalen <= 0 || datalen > 1024 || !prep->data)
> +		return -EINVAL;
> +
> +	options = kmemdup(prep->data, datalen + 1, GFP_KERNEL);
> +	if (!options)
> +		return -ENOMEM;
> +
> +	options[datalen] = '\0';
> +
> +	kprog = kmem_cache_zalloc(mktme_prog_cache, GFP_KERNEL);
> +	if (!kprog) {
> +		kzfree(options);
> +		return -ENOMEM;
> +	}
> +	ret = mktme_get_options(options, kprog);
> +	if (ret < 0)
> +		goto out;

Everything prior to here looks like it should be in the ->preparse() routine.
I really should get round to making that mandatory.

> +
> +	mktme_map_lock();
> +	ret = mktme_program_key(key->serial, kprog);
> +	mktme_map_unlock();
> +out:
> +	kzfree(options);
> +	kmem_cache_free(mktme_prog_cache, kprog);
> +	return ret;
> +}

David

Re: [RFC 11/12] keys/mktme: Add a new key service type for memory encryption keys

[Alison Schofield]

On Tue, Sep 11, 2018 at 11:03:15PM +0100, David Howells wrote:
> Alison Schofield <alison.schofield@intel.com> wrote:
> 
> > +/* Key Service Command: Creates a software key and programs hardware */
> > +int mktme_instantiate(struct key *key, struct key_preparsed_payload *prep)
> > +{
> > +	struct mktme_key_program *kprog = NULL;
> > +	size_t datalen = prep->datalen;
> > +	char *options;
> > +	int ret = 0;
> > +
> > +	if (!capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
> > +		return -EACCES;
> > +
> > +	if (datalen <= 0 || datalen > 1024 || !prep->data)
> > +		return -EINVAL;
> > +
> > +	options = kmemdup(prep->data, datalen + 1, GFP_KERNEL);
> > +	if (!options)
> > +		return -ENOMEM;
> > +
> > +	options[datalen] = '\0';
> > +
> > +	kprog = kmem_cache_zalloc(mktme_prog_cache, GFP_KERNEL);
> > +	if (!kprog) {
> > +		kzfree(options);
> > +		return -ENOMEM;
> > +	}
> > +	ret = mktme_get_options(options, kprog);
> > +	if (ret < 0)
> > +		goto out;
> 
> Everything prior to here looks like it should be in the ->preparse() routine.
> I really should get round to making that mandatory.

Hi Dave,

If a preparse routine handles all the above, then if any of the
above failures occur, the key service has less backing out to do.
Is that the point?

How do I make the connection between the preparse and the instantiate? 
Do I just put what I need to remember about this key request in the
payload.data during preparse, so I can examine it again during
instantiate?

Thanks,
Alison

> 
> > +
> > +	mktme_map_lock();
> > +	ret = mktme_program_key(key->serial, kprog);
> > +	mktme_map_unlock();
> > +out:
> > +	kzfree(options);
> > +	kmem_cache_free(mktme_prog_cache, kprog);
> > +	return ret;
> > +}
> 
> David

Re: [RFC 04/12] x86/mm: Add helper functions to manage memory encryption keys

[David Howells]

Alison Schofield <alison.schofield@intel.com> wrote:

> +void mktme_map_set_keyid(int keyid, unsigned int serial)
> +{
> +	mktme_map->serial[keyid] = serial;
> +	mktme_map->mapped_keyids++;
> +}

It appears that 'serial' should be key_serial_t.

Note that you *aren't* allowed to cache key serial numbers inside the kernel.
You must cache the struct key * instead and hold a reference to the key.  This
will prevent the key from being destroyed whilst it is in use.

David

Re: [RFC 11/12] keys/mktme: Add a new key service type for memory encryption keys

[David Howells]

Alison Schofield <alison.schofield@intel.com> wrote:

> If a preparse routine handles all the above, then if any of the
> above failures occur, the key service has less backing out to do.
> Is that the point?

Yes.  Ideally, ->instantiate() would never fail.

> How do I make the connection between the preparse and the instantiate? 
> Do I just put what I need to remember about this key request in the
> payload.data during preparse, so I can examine it again during
> instantiate?

Have a look at user_preparse().  It attaches the contribution to the supplied
key_preparsed_payload struct, which is then passed to ->instantiate() and
->update() as appropriate.  generic_key_instantiate() is used by the user key
type.

David

Re: [RFC 12/12] keys/mktme: Do not revoke in use memory encryption keys

[David Howells]

Alison Schofield <alison.schofield@intel.com> wrote:

> +
> +	if (strcmp(key->type->name, "mktme") == 0)
> +		mktme_revoke_key(key);
> +

*Please* don't do that.

The core code shouldn't be making references to specific key types in this
way.  The only reason this is necessary for encrypted and trusted keys is
because they misused the ->update() hook and it took a while for this to be
noticed.

> The KEY_FLAG_KEEP bit offers good control. The mktme service uses
> that flag to prevent userspace keys from going away without proper
> synchronization with the mktme service type.

This is not the control you are looking for.  The point of KEY_FLAG_KEEP is to
allow the system to pin a key.  It's not meant to be a flag for the key type
to play with.

You say this:

	One example is that userspace keys should not be revoked while the
	hardware keyid slot is still in use.

but why not?  Revoking it causes accesses to return -EKEYREVOKED; it doesn't
stop the kernel from using the key.

Also, note that you don't *have* to provide a ->revoke() operation

If you really want to suppress revocation, then I would suggest adding another
type operation, say ->may_revoke(), that says whether you're allowed to do
that.

David

Re: [RFC 11/12] keys/mktme: Add a new key service type for memory encryption keys

[Alison Schofield]

On Sun, Sep 09, 2018 at 08:29:29PM -0700, Huang, Kai wrote:
> > + */
> > +static int mktme_build_cpumask(void)
> > +{
> > +	int online_cpu, mktme_cpu;
> > +	int online_pkgid, mktme_pkgid = -1;
> > +
> > +	if (!zalloc_cpumask_var(&mktme_cpumask, GFP_KERNEL))
> > +		return -ENOMEM;
> > +
> > +	for_each_online_cpu(online_cpu) {
> > +		online_pkgid = topology_physical_package_id(online_cpu);
> > +
> > +		for_each_cpu(mktme_cpu, mktme_cpumask) {
> > +			mktme_pkgid =
> > topology_physical_package_id(mktme_cpu);
> > +			if (mktme_pkgid == online_pkgid)
> > +				break;
> > +		}
> > +		if (mktme_pkgid != online_pkgid)
> > +			cpumask_set_cpu(online_cpu, mktme_cpumask);
> > +	}
> 
> Could we use 'for_each_online_node', 'cpumask_first/next', etc to simplify the logic?

Kai, 

I tried to simplify it and came up with code that looked like this:

	int lead_cpu, node;
	for_each_online_node(node) {
		lead_cpu = cpumask_first(cpumask_of_node(node));
		if (lead_cpu < nr_cpu_ids)
			cpumask_set_cpu(lead_cpu, mktme_cpumask_NEW);
	}
When I test it on an SNC (Sub Numa Cluster) system it gives me too many
CPU's. I get a CPU per Node (just like i asked for;) instead of per Socket.
It has 2 sockets and 4 NUMA nodes. 

I kind of remember this when I originally coded it, hence the bottoms up
approach using topology_physical_package_id()

Any ideas?

Alison

RE: [RFC 11/12] keys/mktme: Add a new key service type for memory encryption keys

[Huang, Kai]

> -----Original Message-----
> From: Schofield, Alison
> Sent: Saturday, September 15, 2018 12:07 PM
> To: Huang, Kai <kai.huang@intel.com>
> Cc: dhowells@redhat.com; tglx@linutronix.de; Nakajima, Jun
> <jun.nakajima@intel.com>; Shutemov, Kirill <kirill.shutemov@intel.com>;
> Hansen, Dave <dave.hansen@intel.com>; Sakkinen, Jarkko
> <jarkko.sakkinen@intel.com>; jmorris@namei.org; keyrings@vger.kernel.org;
> linux-security-module@vger.kernel.org; mingo@redhat.com; hpa@zytor.com;
> x86@kernel.org; linux-mm@kvack.org
> Subject: Re: [RFC 11/12] keys/mktme: Add a new key service type for memory
> encryption keys
> 
> On Sun, Sep 09, 2018 at 08:29:29PM -0700, Huang, Kai wrote:
> > > + */
> > > +static int mktme_build_cpumask(void) {
> > > +	int online_cpu, mktme_cpu;
> > > +	int online_pkgid, mktme_pkgid = -1;
> > > +
> > > +	if (!zalloc_cpumask_var(&mktme_cpumask, GFP_KERNEL))
> > > +		return -ENOMEM;
> > > +
> > > +	for_each_online_cpu(online_cpu) {
> > > +		online_pkgid = topology_physical_package_id(online_cpu);
> > > +
> > > +		for_each_cpu(mktme_cpu, mktme_cpumask) {
> > > +			mktme_pkgid =
> > > topology_physical_package_id(mktme_cpu);
> > > +			if (mktme_pkgid == online_pkgid)
> > > +				break;
> > > +		}
> > > +		if (mktme_pkgid != online_pkgid)
> > > +			cpumask_set_cpu(online_cpu, mktme_cpumask);
> > > +	}
> >
> > Could we use 'for_each_online_node', 'cpumask_first/next', etc to simplify the
> logic?
> 
> Kai,
> 
> I tried to simplify it and came up with code that looked like this:
> 
> 	int lead_cpu, node;
> 	for_each_online_node(node) {
> 		lead_cpu = cpumask_first(cpumask_of_node(node));
> 		if (lead_cpu < nr_cpu_ids)
> 			cpumask_set_cpu(lead_cpu, mktme_cpumask_NEW);
> 	}
> When I test it on an SNC (Sub Numa Cluster) system it gives me too many CPU's.
> I get a CPU per Node (just like i asked for;) instead of per Socket.
> It has 2 sockets and 4 NUMA nodes.
> 
> I kind of remember this when I originally coded it, hence the bottoms up
> approach using topology_physical_package_id()
> 
> Any ideas?

Hmm.. I forgot the SNC case, sorry :(

So in case of SNC, is PCONFIG per-package, or per-node? I am not quite sure about this.

If PCONFIG is per-package, I don't have better idea than your original one. :)

Thanks,
-Kai
> 
> Alison
> 

RE: [RFC 11/12] keys/mktme: Add a new key service type for memory encryption keys

[Huang, Kai]

> > On Sun, Sep 09, 2018 at 08:29:29PM -0700, Huang, Kai wrote:
> > > > + */
> > > > +static int mktme_build_cpumask(void) {
> > > > +	int online_cpu, mktme_cpu;
> > > > +	int online_pkgid, mktme_pkgid = -1;
> > > > +
> > > > +	if (!zalloc_cpumask_var(&mktme_cpumask, GFP_KERNEL))
> > > > +		return -ENOMEM;
> > > > +
> > > > +	for_each_online_cpu(online_cpu) {
> > > > +		online_pkgid = topology_physical_package_id(online_cpu);
> > > > +
> > > > +		for_each_cpu(mktme_cpu, mktme_cpumask) {
> > > > +			mktme_pkgid =
> > > > topology_physical_package_id(mktme_cpu);
> > > > +			if (mktme_pkgid == online_pkgid)
> > > > +				break;
> > > > +		}
> > > > +		if (mktme_pkgid != online_pkgid)
> > > > +			cpumask_set_cpu(online_cpu, mktme_cpumask);
> > > > +	}
> > >
> > > Could we use 'for_each_online_node', 'cpumask_first/next', etc to
> > > simplify the
> > logic?
> >
> > Kai,
> >
> > I tried to simplify it and came up with code that looked like this:
> >
> > 	int lead_cpu, node;
> > 	for_each_online_node(node) {
> > 		lead_cpu = cpumask_first(cpumask_of_node(node));
> > 		if (lead_cpu < nr_cpu_ids)
> > 			cpumask_set_cpu(lead_cpu, mktme_cpumask_NEW);
> > 	}
> > When I test it on an SNC (Sub Numa Cluster) system it gives me too many
> CPU's.
> > I get a CPU per Node (just like i asked for;) instead of per Socket.
> > It has 2 sockets and 4 NUMA nodes.
> >
> > I kind of remember this when I originally coded it, hence the bottoms
> > up approach using topology_physical_package_id()
> >
> > Any ideas?
> 
> Hmm.. I forgot the SNC case, sorry :(
> 
> So in case of SNC, is PCONFIG per-package, or per-node? I am not quite sure
> about this.

I have confirmed internally that PCONFIG is per-package even in SNC.

Thanks,
-Kai
> 
> If PCONFIG is per-package, I don't have better idea than your original one. :)
> 
> Thanks,
> -Kai
> >
> > Alison
> >