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

[Huang, Kai]

> -----Original Message-----
> From: owner-linux-security-module at vger.kernel.org [mailto:owner-linux-
> security-module at vger.kernel.org] On Behalf Of Huang, Kai
> Sent: Monday, September 10, 2018 2:57 PM
> To: Schofield, Alison <alison.schofield at intel.com>; dhowells at redhat.com;
> tglx at linutronix.de
> Cc: Nakajima, Jun <jun.nakajima at intel.com>; Shutemov, Kirill
> <kirill.shutemov at intel.com>; Hansen, Dave <dave.hansen at intel.com>;
> Sakkinen, Jarkko <jarkko.sakkinen at intel.com>; jmorris at namei.org;
> keyrings at vger.kernel.org; linux-security-module at vger.kernel.org;
> mingo at redhat.com; hpa at zytor.com; x86 at kernel.org; linux-mm at kvack.org
> Subject: RE: [RFC 04/12] x86/mm: Add helper functions to manage memory
> encryption keys
> 
> 
> > -----Original Message-----
> > From: owner-linux-security-module at vger.kernel.org [mailto:owner-linux-
> > security-module at vger.kernel.org] On Behalf Of Alison Schofield
> > Sent: Saturday, September 8, 2018 10:36 AM
> > To: dhowells at redhat.com; tglx at linutronix.de
> > Cc: Huang, Kai <kai.huang at intel.com>; Nakajima, Jun
> > <jun.nakajima at intel.com>; Shutemov, Kirill
> > <kirill.shutemov at intel.com>; Hansen, Dave <dave.hansen at intel.com>;
> > Sakkinen, Jarkko <jarkko.sakkinen at intel.com>; jmorris at namei.org;
> > keyrings at vger.kernel.org; linux-security-module at vger.kernel.org;
> > mingo at redhat.com; hpa at zytor.com; x86 at kernel.org; linux-mm at 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 at 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