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

Commit Message

Alison Schofield Sept. 7, 2018, 10:38 p.m. UTC

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 <[e-mail]>
---
 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

Comments

Huang, Kai Sept. 10, 2018, 3:29 a.m. UTC | #1

> -----Original Message-----
> From: [e-mail] [mailto:keyrings-
> [e-mail]] On Behalf Of Alison Schofield
> Sent: Saturday, September 8, 2018 10:39 AM
> To: [e-mail]; [e-mail]
> Cc: Huang, Kai <[e-mail]>; Nakajima, Jun
> <[e-mail]>; Shutemov, Kirill <[e-mail]>;
> Hansen, Dave <[e-mail]>; Sakkinen, Jarkko
> <[e-mail]>; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]
> 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 <[e-mail]>
> ---
>  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

Alison Schofield Sept. 10, 2018, 9:47 p.m. UTC | #2

On Sun, Sep 09, 2018 at 08:29:29PM -0700, Huang, Kai wrote:
> > -----Original Message-----
> > From: [e-mail] [mailto:keyrings-
> > [e-mail]] On Behalf Of Alison Schofield
> > Sent: Saturday, September 8, 2018 10:39 AM
> > To: [e-mail]; [e-mail]
> > Cc: Huang, Kai <[e-mail]>; Nakajima, Jun
> > <[e-mail]>; Shutemov, Kirill <[e-mail]>;
> > Hansen, Dave <[e-mail]>; Sakkinen, Jarkko
> > <[e-mail]>; [e-mail]; [e-mail];
> > [e-mail]; [e-mail]; [e-mail];
> > [e-mail]; [e-mail]
> > 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

David Howells Sept. 11, 2018, 10:03 p.m. UTC | #3

Alison Schofield <[e-mail]> 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

Alison Schofield Sept. 11, 2018, 10:39 p.m. UTC | #4

On Tue, Sep 11, 2018 at 11:03:15PM +0100, David Howells wrote:
> Alison Schofield <[e-mail]> 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

David Howells Sept. 11, 2018, 11:01 p.m. UTC | #5

Alison Schofield <[e-mail]> 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

Alison Schofield Sept. 15, 2018, 12:06 a.m. UTC | #6

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

Huang, Kai Sept. 17, 2018, 10:48 a.m. UTC | #7

> -----Original Message-----
> From: Schofield, Alison
> Sent: Saturday, September 15, 2018 12:07 PM
> To: Huang, Kai <[e-mail]>
> Cc: [e-mail]; [e-mail]; Nakajima, Jun
> <[e-mail]>; Shutemov, Kirill <[e-mail]>;
> Hansen, Dave <[e-mail]>; Sakkinen, Jarkko
> <[e-mail]>; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]
> 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
>

Huang, Kai Sept. 17, 2018, 10:34 p.m. UTC | #8

> > 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
> >

Patch

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);