[PATCH 0/6] security/keys/encrypted: Break module dependency chain

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH 0/6] security/keys/encrypted: Break module dependency chain
@ 2019-03-19  6:06 ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-19  6:06 UTC (permalink / raw)
  To: keyrings
  Cc: David Howells, linux-nvdimm, James Bottomley, ecryptfs,
	Roberto Sassu, linux-kernel, Jarkko Sakkinen, Tyler Hicks,
	Mimi Zohar, linux-integrity

With v5.1-rc1 all the nvdimm sub-system regression tests started failing
because the libnvdimm module failed to load in the qemu-kvm test
environment.  Critically that environment does not have a TPM. Commit
240730437deb "KEYS: trusted: explicitly use tpm_chip structure..."
started to require a TPM to be present for the trusted.ko module to load
where there was no requirement for that before.

Rather than undo the "fail if no hardware" behavior James points out
that the module dependencies can be broken by looking up the key-type by
name. Remove the dependencies on the "key_type_trusted" and
"key_type_encrypted" symbol exports, and clean up other boilerplate that
supported those exports in different configurations.

---

Dan Williams (6):
      security/keys/encrypted: Allow operation without trusted.ko
      security/keys/encrypted: Clean up request_trusted_key()
      libnvdimm/security: Drop direct dependency on key_type_encrypted
      security/keys/ecryptfs: Drop direct dependency on key_type_encrypted
      security/integrity/evm: Drop direct dependency on key_type_encrypted
      security/keys/encrypted: Drop export of key_type_encrypted


 drivers/nvdimm/security.c                        |   11 ++++-
 fs/ecryptfs/ecryptfs_kernel.h                    |   22 -----------
 fs/ecryptfs/keystore.c                           |   12 ++++++
 include/keys/encrypted-type.h                    |    2 -
 include/linux/key.h                              |    1 
 security/integrity/evm/evm_crypto.c              |    9 ++++
 security/keys/encrypted-keys/Makefile            |    3 -
 security/keys/encrypted-keys/encrypted.c         |   35 ++++++++++++++++-
 security/keys/encrypted-keys/encrypted.h         |   12 ------
 security/keys/encrypted-keys/masterkey_trusted.c |   46 ----------------------
 security/keys/internal.h                         |    2 -
 security/keys/key.c                              |    1 
 12 files changed, 65 insertions(+), 91 deletions(-)
 delete mode 100644 security/keys/encrypted-keys/masterkey_trusted.c
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 70+ messages in thread

* [PATCH 0/6] security/keys/encrypted: Break module dependency chain
@ 2019-03-19  6:06 ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-19  6:06 UTC (permalink / raw)
  To: keyrings
  Cc: Ira Weiny, Dave Jiang, Tyler Hicks, Keith Busch, Jarkko Sakkinen,
	David Howells, Vishal Verma, James Bottomley, Mimi Zohar,
	linux-integrity, ecryptfs, Roberto Sassu, linux-nvdimm,
	linux-kernel

With v5.1-rc1 all the nvdimm sub-system regression tests started failing
because the libnvdimm module failed to load in the qemu-kvm test
environment.  Critically that environment does not have a TPM. Commit
240730437deb "KEYS: trusted: explicitly use tpm_chip structure..."
started to require a TPM to be present for the trusted.ko module to load
where there was no requirement for that before.

Rather than undo the "fail if no hardware" behavior James points out
that the module dependencies can be broken by looking up the key-type by
name. Remove the dependencies on the "key_type_trusted" and
"key_type_encrypted" symbol exports, and clean up other boilerplate that
supported those exports in different configurations.

---

Dan Williams (6):
      security/keys/encrypted: Allow operation without trusted.ko
      security/keys/encrypted: Clean up request_trusted_key()
      libnvdimm/security: Drop direct dependency on key_type_encrypted
      security/keys/ecryptfs: Drop direct dependency on key_type_encrypted
      security/integrity/evm: Drop direct dependency on key_type_encrypted
      security/keys/encrypted: Drop export of key_type_encrypted


 drivers/nvdimm/security.c                        |   11 ++++-
 fs/ecryptfs/ecryptfs_kernel.h                    |   22 -----------
 fs/ecryptfs/keystore.c                           |   12 ++++++
 include/keys/encrypted-type.h                    |    2 -
 include/linux/key.h                              |    1 
 security/integrity/evm/evm_crypto.c              |    9 ++++
 security/keys/encrypted-keys/Makefile            |    3 -
 security/keys/encrypted-keys/encrypted.c         |   35 ++++++++++++++++-
 security/keys/encrypted-keys/encrypted.h         |   12 ------
 security/keys/encrypted-keys/masterkey_trusted.c |   46 ----------------------
 security/keys/internal.h                         |    2 -
 security/keys/key.c                              |    1 
 12 files changed, 65 insertions(+), 91 deletions(-)
 delete mode 100644 security/keys/encrypted-keys/masterkey_trusted.c

^ permalink raw reply	[flat|nested] 70+ messages in thread

* [PATCH 0/6] security/keys/encrypted: Break module dependency chain
@ 2019-03-19  6:06 ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-19  6:06 UTC (permalink / raw)
  To: keyrings
  Cc: Ira Weiny, Dave Jiang, Tyler Hicks, Keith Busch, Jarkko Sakkinen,
	David Howells, Vishal Verma, James Bottomley, Mimi Zohar,
	linux-integrity, ecryptfs, Roberto Sassu, linux-nvdimm,
	linux-kernel

With v5.1-rc1 all the nvdimm sub-system regression tests started failing
because the libnvdimm module failed to load in the qemu-kvm test
environment.  Critically that environment does not have a TPM. Commit
240730437deb "KEYS: trusted: explicitly use tpm_chip structure..."
started to require a TPM to be present for the trusted.ko module to load
where there was no requirement for that before.

Rather than undo the "fail if no hardware" behavior James points out
that the module dependencies can be broken by looking up the key-type by
name. Remove the dependencies on the "key_type_trusted" and
"key_type_encrypted" symbol exports, and clean up other boilerplate that
supported those exports in different configurations.

---

Dan Williams (6):
      security/keys/encrypted: Allow operation without trusted.ko
      security/keys/encrypted: Clean up request_trusted_key()
      libnvdimm/security: Drop direct dependency on key_type_encrypted
      security/keys/ecryptfs: Drop direct dependency on key_type_encrypted
      security/integrity/evm: Drop direct dependency on key_type_encrypted
      security/keys/encrypted: Drop export of key_type_encrypted


 drivers/nvdimm/security.c                        |   11 ++++-
 fs/ecryptfs/ecryptfs_kernel.h                    |   22 -----------
 fs/ecryptfs/keystore.c                           |   12 ++++++
 include/keys/encrypted-type.h                    |    2 -
 include/linux/key.h                              |    1 
 security/integrity/evm/evm_crypto.c              |    9 ++++
 security/keys/encrypted-keys/Makefile            |    3 -
 security/keys/encrypted-keys/encrypted.c         |   35 ++++++++++++++++-
 security/keys/encrypted-keys/encrypted.h         |   12 ------
 security/keys/encrypted-keys/masterkey_trusted.c |   46 ----------------------
 security/keys/internal.h                         |    2 -
 security/keys/key.c                              |    1 
 12 files changed, 65 insertions(+), 91 deletions(-)
 delete mode 100644 security/keys/encrypted-keys/masterkey_trusted.c

^ permalink raw reply	[flat|nested] 70+ messages in thread

* [PATCH 1/6] security/keys/encrypted: Allow operation without trusted.ko
  2019-03-19  6:06 ` Dan Williams
  (?)
@ 2019-03-19  6:06   ` Dan Williams
  -1 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-19  6:06 UTC (permalink / raw)
  To: keyrings
  Cc: linux-nvdimm, James Bottomley, Roberto Sassu, linux-kernel,
	Mimi Zohar, David Howells, Jarkko Sakkinen

The trusted.ko module may fail to load. In the common case this failure
is simply due to the platform missing a TPM. Teach the encrypted_keys
implementation to lookup the key type by name rather than having a
module dependency.

Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...")
Suggested-by: James Bottomley <jejb@linux.ibm.com>
Cc: Roberto Sassu <roberto.sassu@huawei.com>
Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: David Howells <dhowells@redhat.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 security/keys/encrypted-keys/masterkey_trusted.c |   11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/security/keys/encrypted-keys/masterkey_trusted.c b/security/keys/encrypted-keys/masterkey_trusted.c
index dc3d18cae642..7560aea6438d 100644
--- a/security/keys/encrypted-keys/masterkey_trusted.c
+++ b/security/keys/encrypted-keys/masterkey_trusted.c
@@ -19,6 +19,7 @@
 #include <keys/trusted-type.h>
 #include <keys/encrypted-type.h>
 #include "encrypted.h"
+#include "../internal.h"
 
 /*
  * request_trusted_key - request the trusted key
@@ -31,9 +32,15 @@ struct key *request_trusted_key(const char *trusted_desc,
 				const u8 **master_key, size_t *master_keylen)
 {
 	struct trusted_key_payload *tpayload;
+	struct key_type *type;
 	struct key *tkey;
 
-	tkey = request_key(&key_type_trusted, trusted_desc, NULL);
+	type = key_type_lookup("trusted");
+	if (IS_ERR(type)) {
+		tkey = (struct key *)type;
+		goto error;
+	}
+	tkey = request_key(type, trusted_desc, NULL);
 	if (IS_ERR(tkey))
 		goto error;
 
@@ -44,3 +51,5 @@ struct key *request_trusted_key(const char *trusted_desc,
 error:
 	return tkey;
 }
+
+MODULE_SOFTDEP("pre: trusted");

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 1/6] security/keys/encrypted: Allow operation without trusted.ko
@ 2019-03-19  6:06   ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-19  6:06 UTC (permalink / raw)
  To: keyrings
  Cc: James Bottomley, Roberto Sassu, Jarkko Sakkinen, Mimi Zohar,
	David Howells, vishal.l.verma, linux-nvdimm, linux-kernel

The trusted.ko module may fail to load. In the common case this failure
is simply due to the platform missing a TPM. Teach the encrypted_keys
implementation to lookup the key type by name rather than having a
module dependency.

Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...")
Suggested-by: James Bottomley <jejb@linux.ibm.com>
Cc: Roberto Sassu <roberto.sassu@huawei.com>
Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: David Howells <dhowells@redhat.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 security/keys/encrypted-keys/masterkey_trusted.c |   11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/security/keys/encrypted-keys/masterkey_trusted.c b/security/keys/encrypted-keys/masterkey_trusted.c
index dc3d18cae642..7560aea6438d 100644
--- a/security/keys/encrypted-keys/masterkey_trusted.c
+++ b/security/keys/encrypted-keys/masterkey_trusted.c
@@ -19,6 +19,7 @@
 #include <keys/trusted-type.h>
 #include <keys/encrypted-type.h>
 #include "encrypted.h"
+#include "../internal.h"
 
 /*
  * request_trusted_key - request the trusted key
@@ -31,9 +32,15 @@ struct key *request_trusted_key(const char *trusted_desc,
 				const u8 **master_key, size_t *master_keylen)
 {
 	struct trusted_key_payload *tpayload;
+	struct key_type *type;
 	struct key *tkey;
 
-	tkey = request_key(&key_type_trusted, trusted_desc, NULL);
+	type = key_type_lookup("trusted");
+	if (IS_ERR(type)) {
+		tkey = (struct key *)type;
+		goto error;
+	}
+	tkey = request_key(type, trusted_desc, NULL);
 	if (IS_ERR(tkey))
 		goto error;
 
@@ -44,3 +51,5 @@ struct key *request_trusted_key(const char *trusted_desc,
 error:
 	return tkey;
 }
+
+MODULE_SOFTDEP("pre: trusted");


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 1/6] security/keys/encrypted: Allow operation without trusted.ko
@ 2019-03-19  6:06   ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-19  6:06 UTC (permalink / raw)
  To: keyrings
  Cc: linux-nvdimm, James Bottomley, Roberto Sassu, linux-kernel,
	Mimi Zohar, David Howells, Jarkko Sakkinen

The trusted.ko module may fail to load. In the common case this failure
is simply due to the platform missing a TPM. Teach the encrypted_keys
implementation to lookup the key type by name rather than having a
module dependency.

Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...")
Suggested-by: James Bottomley <jejb@linux.ibm.com>
Cc: Roberto Sassu <roberto.sassu@huawei.com>
Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: David Howells <dhowells@redhat.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 security/keys/encrypted-keys/masterkey_trusted.c |   11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/security/keys/encrypted-keys/masterkey_trusted.c b/security/keys/encrypted-keys/masterkey_trusted.c
index dc3d18cae642..7560aea6438d 100644
--- a/security/keys/encrypted-keys/masterkey_trusted.c
+++ b/security/keys/encrypted-keys/masterkey_trusted.c
@@ -19,6 +19,7 @@
 #include <keys/trusted-type.h>
 #include <keys/encrypted-type.h>
 #include "encrypted.h"
+#include "../internal.h"
 
 /*
  * request_trusted_key - request the trusted key
@@ -31,9 +32,15 @@ struct key *request_trusted_key(const char *trusted_desc,
 				const u8 **master_key, size_t *master_keylen)
 {
 	struct trusted_key_payload *tpayload;
+	struct key_type *type;
 	struct key *tkey;
 
-	tkey = request_key(&key_type_trusted, trusted_desc, NULL);
+	type = key_type_lookup("trusted");
+	if (IS_ERR(type)) {
+		tkey = (struct key *)type;
+		goto error;
+	}
+	tkey = request_key(type, trusted_desc, NULL);
 	if (IS_ERR(tkey))
 		goto error;
 
@@ -44,3 +51,5 @@ struct key *request_trusted_key(const char *trusted_desc,
 error:
 	return tkey;
 }
+
+MODULE_SOFTDEP("pre: trusted");

^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
  2019-03-19  6:06 ` Dan Williams
  (?)
@ 2019-03-19  6:06   ` Dan Williams
  -1 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-19  6:06 UTC (permalink / raw)
  To: keyrings
  Cc: linux-nvdimm, James Bottomley, linux-kernel, Mimi Zohar, David Howells

Now that the trusted key type is looked up by name rather than direct
symbol there is no need to play games with detecting the build
configuration. Make request_trusted_key() a static facility internal to
the encrypted-keys implementation.

Suggested-by: James Bottomley <jejb@linux.ibm.com>
Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: David Howells <dhowells@redhat.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 include/linux/key.h                              |    1 
 security/keys/encrypted-keys/Makefile            |    3 -
 security/keys/encrypted-keys/encrypted.c         |   32 +++++++++++++
 security/keys/encrypted-keys/encrypted.h         |   12 -----
 security/keys/encrypted-keys/masterkey_trusted.c |   55 ----------------------
 security/keys/internal.h                         |    2 -
 security/keys/key.c                              |    1 
 7 files changed, 34 insertions(+), 72 deletions(-)
 delete mode 100644 security/keys/encrypted-keys/masterkey_trusted.c

diff --git a/include/linux/key.h b/include/linux/key.h
index 7099985e35a9..e7bfd037d26f 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -270,6 +270,7 @@ static inline void key_ref_put(key_ref_t key_ref)
 	key_put(key_ref_to_ptr(key_ref));
 }
 
+extern struct key_type *key_type_lookup(const char *type);
 extern struct key *request_key(struct key_type *type,
 			       const char *description,
 			       const char *callout_info);
diff --git a/security/keys/encrypted-keys/Makefile b/security/keys/encrypted-keys/Makefile
index 7a44dce6f69d..d42487bb3d8a 100644
--- a/security/keys/encrypted-keys/Makefile
+++ b/security/keys/encrypted-keys/Makefile
@@ -6,6 +6,3 @@
 obj-$(CONFIG_ENCRYPTED_KEYS) += encrypted-keys.o
 
 encrypted-keys-y := encrypted.o ecryptfs_format.o
-masterkey-$(CONFIG_TRUSTED_KEYS) := masterkey_trusted.o
-masterkey-$(CONFIG_TRUSTED_KEYS)-$(CONFIG_ENCRYPTED_KEYS) := masterkey_trusted.o
-encrypted-keys-y += $(masterkey-y) $(masterkey-m-m)
diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index 347108f660a1..06925d3b30c9 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -423,6 +423,37 @@ static struct skcipher_request *init_skcipher_req(const u8 *key,
 	return req;
 }
 
+/*
+ * request_trusted_key - request the trusted key
+ *
+ * Trusted keys are sealed to PCRs and other metadata. Although userspace
+ * manages both trusted/encrypted key-types, like the encrypted key type
+ * data, trusted key type data is not visible decrypted from userspace.
+ */
+static struct key *request_trusted_key(const char *trusted_desc,
+				const u8 **master_key, size_t *master_keylen)
+{
+	struct trusted_key_payload *tpayload;
+	struct key_type *type;
+	struct key *tkey;
+
+	type = key_type_lookup("trusted");
+	if (IS_ERR(type)) {
+		tkey = (struct key *)type;
+		goto error;
+	}
+	tkey = request_key(type, trusted_desc, NULL);
+	if (IS_ERR(tkey))
+		goto error;
+
+	down_read(&tkey->sem);
+	tpayload = tkey->payload.data[0];
+	*master_key = tpayload->key;
+	*master_keylen = tpayload->key_len;
+error:
+	return tkey;
+}
+
 static struct key *request_master_key(struct encrypted_key_payload *epayload,
 				      const u8 **master_key, size_t *master_keylen)
 {
@@ -1025,3 +1056,4 @@ late_initcall(init_encrypted);
 module_exit(cleanup_encrypted);
 
 MODULE_LICENSE("GPL");
+MODULE_SOFTDEP("pre: trusted");
diff --git a/security/keys/encrypted-keys/encrypted.h b/security/keys/encrypted-keys/encrypted.h
index 1809995db452..0ae67824a24a 100644
--- a/security/keys/encrypted-keys/encrypted.h
+++ b/security/keys/encrypted-keys/encrypted.h
@@ -3,18 +3,6 @@
 #define __ENCRYPTED_KEY_H
 
 #define ENCRYPTED_DEBUG 0
-#if defined(CONFIG_TRUSTED_KEYS) || \
-  (defined(CONFIG_TRUSTED_KEYS_MODULE) && defined(CONFIG_ENCRYPTED_KEYS_MODULE))
-extern struct key *request_trusted_key(const char *trusted_desc,
-				       const u8 **master_key, size_t *master_keylen);
-#else
-static inline struct key *request_trusted_key(const char *trusted_desc,
-					      const u8 **master_key,
-					      size_t *master_keylen)
-{
-	return ERR_PTR(-EOPNOTSUPP);
-}
-#endif
 
 #if ENCRYPTED_DEBUG
 static inline void dump_master_key(const u8 *master_key, size_t master_keylen)
diff --git a/security/keys/encrypted-keys/masterkey_trusted.c b/security/keys/encrypted-keys/masterkey_trusted.c
deleted file mode 100644
index 7560aea6438d..000000000000
--- a/security/keys/encrypted-keys/masterkey_trusted.c
+++ /dev/null
@@ -1,55 +0,0 @@
-/*
- * Copyright (C) 2010 IBM Corporation
- * Copyright (C) 2010 Politecnico di Torino, Italy
- *                    TORSEC group -- http://security.polito.it
- *
- * Authors:
- * Mimi Zohar <zohar@us.ibm.com>
- * Roberto Sassu <roberto.sassu@polito.it>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, version 2 of the License.
- *
- * See Documentation/security/keys/trusted-encrypted.rst
- */
-
-#include <linux/uaccess.h>
-#include <linux/err.h>
-#include <keys/trusted-type.h>
-#include <keys/encrypted-type.h>
-#include "encrypted.h"
-#include "../internal.h"
-
-/*
- * request_trusted_key - request the trusted key
- *
- * Trusted keys are sealed to PCRs and other metadata. Although userspace
- * manages both trusted/encrypted key-types, like the encrypted key type
- * data, trusted key type data is not visible decrypted from userspace.
- */
-struct key *request_trusted_key(const char *trusted_desc,
-				const u8 **master_key, size_t *master_keylen)
-{
-	struct trusted_key_payload *tpayload;
-	struct key_type *type;
-	struct key *tkey;
-
-	type = key_type_lookup("trusted");
-	if (IS_ERR(type)) {
-		tkey = (struct key *)type;
-		goto error;
-	}
-	tkey = request_key(type, trusted_desc, NULL);
-	if (IS_ERR(tkey))
-		goto error;
-
-	down_read(&tkey->sem);
-	tpayload = tkey->payload.data[0];
-	*master_key = tpayload->key;
-	*master_keylen = tpayload->key_len;
-error:
-	return tkey;
-}
-
-MODULE_SOFTDEP("pre: trusted");
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 8f533c81aa8d..ea2eb78459bf 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -89,8 +89,6 @@ extern spinlock_t key_serial_lock;
 extern struct mutex key_construction_mutex;
 extern wait_queue_head_t request_key_conswq;
 
-
-extern struct key_type *key_type_lookup(const char *type);
 extern void key_type_put(struct key_type *ktype);
 
 extern int __key_link_begin(struct key *keyring,
diff --git a/security/keys/key.c b/security/keys/key.c
index 696f1c092c50..9045b62afb04 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -706,6 +706,7 @@ struct key_type *key_type_lookup(const char *type)
 found_kernel_type:
 	return ktype;
 }
+EXPORT_SYMBOL_GPL(key_type_lookup);
 
 void key_set_timeout(struct key *key, unsigned timeout)
 {

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
@ 2019-03-19  6:06   ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-19  6:06 UTC (permalink / raw)
  To: keyrings
  Cc: James Bottomley, Mimi Zohar, David Howells, vishal.l.verma,
	linux-nvdimm, linux-kernel

Now that the trusted key type is looked up by name rather than direct
symbol there is no need to play games with detecting the build
configuration. Make request_trusted_key() a static facility internal to
the encrypted-keys implementation.

Suggested-by: James Bottomley <jejb@linux.ibm.com>
Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: David Howells <dhowells@redhat.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 include/linux/key.h                              |    1 
 security/keys/encrypted-keys/Makefile            |    3 -
 security/keys/encrypted-keys/encrypted.c         |   32 +++++++++++++
 security/keys/encrypted-keys/encrypted.h         |   12 -----
 security/keys/encrypted-keys/masterkey_trusted.c |   55 ----------------------
 security/keys/internal.h                         |    2 -
 security/keys/key.c                              |    1 
 7 files changed, 34 insertions(+), 72 deletions(-)
 delete mode 100644 security/keys/encrypted-keys/masterkey_trusted.c

diff --git a/include/linux/key.h b/include/linux/key.h
index 7099985e35a9..e7bfd037d26f 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -270,6 +270,7 @@ static inline void key_ref_put(key_ref_t key_ref)
 	key_put(key_ref_to_ptr(key_ref));
 }
 
+extern struct key_type *key_type_lookup(const char *type);
 extern struct key *request_key(struct key_type *type,
 			       const char *description,
 			       const char *callout_info);
diff --git a/security/keys/encrypted-keys/Makefile b/security/keys/encrypted-keys/Makefile
index 7a44dce6f69d..d42487bb3d8a 100644
--- a/security/keys/encrypted-keys/Makefile
+++ b/security/keys/encrypted-keys/Makefile
@@ -6,6 +6,3 @@
 obj-$(CONFIG_ENCRYPTED_KEYS) += encrypted-keys.o
 
 encrypted-keys-y := encrypted.o ecryptfs_format.o
-masterkey-$(CONFIG_TRUSTED_KEYS) := masterkey_trusted.o
-masterkey-$(CONFIG_TRUSTED_KEYS)-$(CONFIG_ENCRYPTED_KEYS) := masterkey_trusted.o
-encrypted-keys-y += $(masterkey-y) $(masterkey-m-m)
diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index 347108f660a1..06925d3b30c9 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -423,6 +423,37 @@ static struct skcipher_request *init_skcipher_req(const u8 *key,
 	return req;
 }
 
+/*
+ * request_trusted_key - request the trusted key
+ *
+ * Trusted keys are sealed to PCRs and other metadata. Although userspace
+ * manages both trusted/encrypted key-types, like the encrypted key type
+ * data, trusted key type data is not visible decrypted from userspace.
+ */
+static struct key *request_trusted_key(const char *trusted_desc,
+				const u8 **master_key, size_t *master_keylen)
+{
+	struct trusted_key_payload *tpayload;
+	struct key_type *type;
+	struct key *tkey;
+
+	type = key_type_lookup("trusted");
+	if (IS_ERR(type)) {
+		tkey = (struct key *)type;
+		goto error;
+	}
+	tkey = request_key(type, trusted_desc, NULL);
+	if (IS_ERR(tkey))
+		goto error;
+
+	down_read(&tkey->sem);
+	tpayload = tkey->payload.data[0];
+	*master_key = tpayload->key;
+	*master_keylen = tpayload->key_len;
+error:
+	return tkey;
+}
+
 static struct key *request_master_key(struct encrypted_key_payload *epayload,
 				      const u8 **master_key, size_t *master_keylen)
 {
@@ -1025,3 +1056,4 @@ late_initcall(init_encrypted);
 module_exit(cleanup_encrypted);
 
 MODULE_LICENSE("GPL");
+MODULE_SOFTDEP("pre: trusted");
diff --git a/security/keys/encrypted-keys/encrypted.h b/security/keys/encrypted-keys/encrypted.h
index 1809995db452..0ae67824a24a 100644
--- a/security/keys/encrypted-keys/encrypted.h
+++ b/security/keys/encrypted-keys/encrypted.h
@@ -3,18 +3,6 @@
 #define __ENCRYPTED_KEY_H
 
 #define ENCRYPTED_DEBUG 0
-#if defined(CONFIG_TRUSTED_KEYS) || \
-  (defined(CONFIG_TRUSTED_KEYS_MODULE) && defined(CONFIG_ENCRYPTED_KEYS_MODULE))
-extern struct key *request_trusted_key(const char *trusted_desc,
-				       const u8 **master_key, size_t *master_keylen);
-#else
-static inline struct key *request_trusted_key(const char *trusted_desc,
-					      const u8 **master_key,
-					      size_t *master_keylen)
-{
-	return ERR_PTR(-EOPNOTSUPP);
-}
-#endif
 
 #if ENCRYPTED_DEBUG
 static inline void dump_master_key(const u8 *master_key, size_t master_keylen)
diff --git a/security/keys/encrypted-keys/masterkey_trusted.c b/security/keys/encrypted-keys/masterkey_trusted.c
deleted file mode 100644
index 7560aea6438d..000000000000
--- a/security/keys/encrypted-keys/masterkey_trusted.c
+++ /dev/null
@@ -1,55 +0,0 @@
-/*
- * Copyright (C) 2010 IBM Corporation
- * Copyright (C) 2010 Politecnico di Torino, Italy
- *                    TORSEC group -- http://security.polito.it
- *
- * Authors:
- * Mimi Zohar <zohar@us.ibm.com>
- * Roberto Sassu <roberto.sassu@polito.it>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, version 2 of the License.
- *
- * See Documentation/security/keys/trusted-encrypted.rst
- */
-
-#include <linux/uaccess.h>
-#include <linux/err.h>
-#include <keys/trusted-type.h>
-#include <keys/encrypted-type.h>
-#include "encrypted.h"
-#include "../internal.h"
-
-/*
- * request_trusted_key - request the trusted key
- *
- * Trusted keys are sealed to PCRs and other metadata. Although userspace
- * manages both trusted/encrypted key-types, like the encrypted key type
- * data, trusted key type data is not visible decrypted from userspace.
- */
-struct key *request_trusted_key(const char *trusted_desc,
-				const u8 **master_key, size_t *master_keylen)
-{
-	struct trusted_key_payload *tpayload;
-	struct key_type *type;
-	struct key *tkey;
-
-	type = key_type_lookup("trusted");
-	if (IS_ERR(type)) {
-		tkey = (struct key *)type;
-		goto error;
-	}
-	tkey = request_key(type, trusted_desc, NULL);
-	if (IS_ERR(tkey))
-		goto error;
-
-	down_read(&tkey->sem);
-	tpayload = tkey->payload.data[0];
-	*master_key = tpayload->key;
-	*master_keylen = tpayload->key_len;
-error:
-	return tkey;
-}
-
-MODULE_SOFTDEP("pre: trusted");
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 8f533c81aa8d..ea2eb78459bf 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -89,8 +89,6 @@ extern spinlock_t key_serial_lock;
 extern struct mutex key_construction_mutex;
 extern wait_queue_head_t request_key_conswq;
 
-
-extern struct key_type *key_type_lookup(const char *type);
 extern void key_type_put(struct key_type *ktype);
 
 extern int __key_link_begin(struct key *keyring,
diff --git a/security/keys/key.c b/security/keys/key.c
index 696f1c092c50..9045b62afb04 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -706,6 +706,7 @@ struct key_type *key_type_lookup(const char *type)
 found_kernel_type:
 	return ktype;
 }
+EXPORT_SYMBOL_GPL(key_type_lookup);
 
 void key_set_timeout(struct key *key, unsigned timeout)
 {


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
@ 2019-03-19  6:06   ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-19  6:06 UTC (permalink / raw)
  To: keyrings
  Cc: linux-nvdimm, James Bottomley, linux-kernel, Mimi Zohar, David Howells

Now that the trusted key type is looked up by name rather than direct
symbol there is no need to play games with detecting the build
configuration. Make request_trusted_key() a static facility internal to
the encrypted-keys implementation.

Suggested-by: James Bottomley <jejb@linux.ibm.com>
Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: David Howells <dhowells@redhat.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 include/linux/key.h                              |    1 
 security/keys/encrypted-keys/Makefile            |    3 -
 security/keys/encrypted-keys/encrypted.c         |   32 +++++++++++++
 security/keys/encrypted-keys/encrypted.h         |   12 -----
 security/keys/encrypted-keys/masterkey_trusted.c |   55 ----------------------
 security/keys/internal.h                         |    2 -
 security/keys/key.c                              |    1 
 7 files changed, 34 insertions(+), 72 deletions(-)
 delete mode 100644 security/keys/encrypted-keys/masterkey_trusted.c

diff --git a/include/linux/key.h b/include/linux/key.h
index 7099985e35a9..e7bfd037d26f 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -270,6 +270,7 @@ static inline void key_ref_put(key_ref_t key_ref)
 	key_put(key_ref_to_ptr(key_ref));
 }
 
+extern struct key_type *key_type_lookup(const char *type);
 extern struct key *request_key(struct key_type *type,
 			       const char *description,
 			       const char *callout_info);
diff --git a/security/keys/encrypted-keys/Makefile b/security/keys/encrypted-keys/Makefile
index 7a44dce6f69d..d42487bb3d8a 100644
--- a/security/keys/encrypted-keys/Makefile
+++ b/security/keys/encrypted-keys/Makefile
@@ -6,6 +6,3 @@
 obj-$(CONFIG_ENCRYPTED_KEYS) += encrypted-keys.o
 
 encrypted-keys-y := encrypted.o ecryptfs_format.o
-masterkey-$(CONFIG_TRUSTED_KEYS) := masterkey_trusted.o
-masterkey-$(CONFIG_TRUSTED_KEYS)-$(CONFIG_ENCRYPTED_KEYS) := masterkey_trusted.o
-encrypted-keys-y += $(masterkey-y) $(masterkey-m-m)
diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index 347108f660a1..06925d3b30c9 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -423,6 +423,37 @@ static struct skcipher_request *init_skcipher_req(const u8 *key,
 	return req;
 }
 
+/*
+ * request_trusted_key - request the trusted key
+ *
+ * Trusted keys are sealed to PCRs and other metadata. Although userspace
+ * manages both trusted/encrypted key-types, like the encrypted key type
+ * data, trusted key type data is not visible decrypted from userspace.
+ */
+static struct key *request_trusted_key(const char *trusted_desc,
+				const u8 **master_key, size_t *master_keylen)
+{
+	struct trusted_key_payload *tpayload;
+	struct key_type *type;
+	struct key *tkey;
+
+	type = key_type_lookup("trusted");
+	if (IS_ERR(type)) {
+		tkey = (struct key *)type;
+		goto error;
+	}
+	tkey = request_key(type, trusted_desc, NULL);
+	if (IS_ERR(tkey))
+		goto error;
+
+	down_read(&tkey->sem);
+	tpayload = tkey->payload.data[0];
+	*master_key = tpayload->key;
+	*master_keylen = tpayload->key_len;
+error:
+	return tkey;
+}
+
 static struct key *request_master_key(struct encrypted_key_payload *epayload,
 				      const u8 **master_key, size_t *master_keylen)
 {
@@ -1025,3 +1056,4 @@ late_initcall(init_encrypted);
 module_exit(cleanup_encrypted);
 
 MODULE_LICENSE("GPL");
+MODULE_SOFTDEP("pre: trusted");
diff --git a/security/keys/encrypted-keys/encrypted.h b/security/keys/encrypted-keys/encrypted.h
index 1809995db452..0ae67824a24a 100644
--- a/security/keys/encrypted-keys/encrypted.h
+++ b/security/keys/encrypted-keys/encrypted.h
@@ -3,18 +3,6 @@
 #define __ENCRYPTED_KEY_H
 
 #define ENCRYPTED_DEBUG 0
-#if defined(CONFIG_TRUSTED_KEYS) || \
-  (defined(CONFIG_TRUSTED_KEYS_MODULE) && defined(CONFIG_ENCRYPTED_KEYS_MODULE))
-extern struct key *request_trusted_key(const char *trusted_desc,
-				       const u8 **master_key, size_t *master_keylen);
-#else
-static inline struct key *request_trusted_key(const char *trusted_desc,
-					      const u8 **master_key,
-					      size_t *master_keylen)
-{
-	return ERR_PTR(-EOPNOTSUPP);
-}
-#endif
 
 #if ENCRYPTED_DEBUG
 static inline void dump_master_key(const u8 *master_key, size_t master_keylen)
diff --git a/security/keys/encrypted-keys/masterkey_trusted.c b/security/keys/encrypted-keys/masterkey_trusted.c
deleted file mode 100644
index 7560aea6438d..000000000000
--- a/security/keys/encrypted-keys/masterkey_trusted.c
+++ /dev/null
@@ -1,55 +0,0 @@
-/*
- * Copyright (C) 2010 IBM Corporation
- * Copyright (C) 2010 Politecnico di Torino, Italy
- *                    TORSEC group -- http://security.polito.it
- *
- * Authors:
- * Mimi Zohar <zohar@us.ibm.com>
- * Roberto Sassu <roberto.sassu@polito.it>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, version 2 of the License.
- *
- * See Documentation/security/keys/trusted-encrypted.rst
- */
-
-#include <linux/uaccess.h>
-#include <linux/err.h>
-#include <keys/trusted-type.h>
-#include <keys/encrypted-type.h>
-#include "encrypted.h"
-#include "../internal.h"
-
-/*
- * request_trusted_key - request the trusted key
- *
- * Trusted keys are sealed to PCRs and other metadata. Although userspace
- * manages both trusted/encrypted key-types, like the encrypted key type
- * data, trusted key type data is not visible decrypted from userspace.
- */
-struct key *request_trusted_key(const char *trusted_desc,
-				const u8 **master_key, size_t *master_keylen)
-{
-	struct trusted_key_payload *tpayload;
-	struct key_type *type;
-	struct key *tkey;
-
-	type = key_type_lookup("trusted");
-	if (IS_ERR(type)) {
-		tkey = (struct key *)type;
-		goto error;
-	}
-	tkey = request_key(type, trusted_desc, NULL);
-	if (IS_ERR(tkey))
-		goto error;
-
-	down_read(&tkey->sem);
-	tpayload = tkey->payload.data[0];
-	*master_key = tpayload->key;
-	*master_keylen = tpayload->key_len;
-error:
-	return tkey;
-}
-
-MODULE_SOFTDEP("pre: trusted");
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 8f533c81aa8d..ea2eb78459bf 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -89,8 +89,6 @@ extern spinlock_t key_serial_lock;
 extern struct mutex key_construction_mutex;
 extern wait_queue_head_t request_key_conswq;
 
-
-extern struct key_type *key_type_lookup(const char *type);
 extern void key_type_put(struct key_type *ktype);
 
 extern int __key_link_begin(struct key *keyring,
diff --git a/security/keys/key.c b/security/keys/key.c
index 696f1c092c50..9045b62afb04 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -706,6 +706,7 @@ struct key_type *key_type_lookup(const char *type)
 found_kernel_type:
 	return ktype;
 }
+EXPORT_SYMBOL_GPL(key_type_lookup);
 
 void key_set_timeout(struct key *key, unsigned timeout)
 {

^ permalink raw reply related	[flat|nested] 70+ messages in thread

* Re: [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
  2019-03-19  6:06   ` Dan Williams
  (?)
@ 2019-03-20  0:06     ` Mimi Zohar
  -1 siblings, 0 replies; 70+ messages in thread
From: Mimi Zohar @ 2019-03-20  0:06 UTC (permalink / raw)
  To: Dan Williams, keyrings
  Cc: David Howells, James Bottomley, linux-kernel, linux-nvdimm

On Mon, 2019-03-18 at 23:06 -0700, Dan Williams wrote:

< snip >

> +/*
> + * request_trusted_key - request the trusted key
> + *
> + * Trusted keys are sealed to PCRs and other metadata. Although userspace
> + * manages both trusted/encrypted key-types, like the encrypted key type
> + * data, trusted key type data is not visible decrypted from userspace.
> + */
> +static struct key *request_trusted_key(const char *trusted_desc,
> +				const u8 **master_key, size_t *master_keylen)
> +{
> +	struct trusted_key_payload *tpayload;
> +	struct key_type *type;
> +	struct key *tkey;
> +
> +	type = key_type_lookup("trusted");

The associated key_type_put() will need to be called.

> +	if (IS_ERR(type)) {
> +		tkey = (struct key *)type;
> +		goto error;
> +	}
> +	tkey = request_key(type, trusted_desc, NULL);
> +	if (IS_ERR(tkey))
> +		goto error;
> +
> +	down_read(&tkey->sem);
> +	tpayload = tkey->payload.data[0];
> +	*master_key = tpayload->key;
> +	*master_keylen = tpayload->key_len;
> +error:
> +	return tkey;
> +}
> +



> diff --git a/security/keys/key.c b/security/keys/key.c
> index 696f1c092c50..9045b62afb04 100644
> --- a/security/keys/key.c
> +++ b/security/keys/key.c
> @@ -706,6 +706,7 @@ struct key_type *key_type_lookup(const char *type)
>  found_kernel_type:
>  	return ktype;
>  }
> +EXPORT_SYMBOL_GPL(key_type_lookup);

Only the kernel is calling key_type_lookup().  Why does
key_type_lookup() need to be exported?

Mimi

>  
>  void key_set_timeout(struct key *key, unsigned timeout)
>  {
> 

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
@ 2019-03-20  0:06     ` Mimi Zohar
  0 siblings, 0 replies; 70+ messages in thread
From: Mimi Zohar @ 2019-03-20  0:06 UTC (permalink / raw)
  To: Dan Williams, keyrings
  Cc: James Bottomley, David Howells, vishal.l.verma, linux-nvdimm,
	linux-kernel

On Mon, 2019-03-18 at 23:06 -0700, Dan Williams wrote:

< snip >

> +/*
> + * request_trusted_key - request the trusted key
> + *
> + * Trusted keys are sealed to PCRs and other metadata. Although userspace
> + * manages both trusted/encrypted key-types, like the encrypted key type
> + * data, trusted key type data is not visible decrypted from userspace.
> + */
> +static struct key *request_trusted_key(const char *trusted_desc,
> +				const u8 **master_key, size_t *master_keylen)
> +{
> +	struct trusted_key_payload *tpayload;
> +	struct key_type *type;
> +	struct key *tkey;
> +
> +	type = key_type_lookup("trusted");

The associated key_type_put() will need to be called.

> +	if (IS_ERR(type)) {
> +		tkey = (struct key *)type;
> +		goto error;
> +	}
> +	tkey = request_key(type, trusted_desc, NULL);
> +	if (IS_ERR(tkey))
> +		goto error;
> +
> +	down_read(&tkey->sem);
> +	tpayload = tkey->payload.data[0];
> +	*master_key = tpayload->key;
> +	*master_keylen = tpayload->key_len;
> +error:
> +	return tkey;
> +}
> +



> diff --git a/security/keys/key.c b/security/keys/key.c
> index 696f1c092c50..9045b62afb04 100644
> --- a/security/keys/key.c
> +++ b/security/keys/key.c
> @@ -706,6 +706,7 @@ struct key_type *key_type_lookup(const char *type)
>  found_kernel_type:
>  	return ktype;
>  }
> +EXPORT_SYMBOL_GPL(key_type_lookup);

Only the kernel is calling key_type_lookup().  Why does
key_type_lookup() need to be exported?

Mimi

>  
>  void key_set_timeout(struct key *key, unsigned timeout)
>  {
> 


^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
@ 2019-03-20  0:06     ` Mimi Zohar
  0 siblings, 0 replies; 70+ messages in thread
From: Mimi Zohar @ 2019-03-20  0:06 UTC (permalink / raw)
  To: Dan Williams, keyrings
  Cc: David Howells, James Bottomley, linux-kernel, linux-nvdimm

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="maccentraleurope", Size: 1517 bytes --]

On Mon, 2019-03-18 at 23:06 -0700, Dan Williams wrote:

< snip >

> +/*
> + * request_trusted_key - request the trusted key
> + *
> + * Trusted keys are sealed to PCRs and other metadata. Although userspace
> + * manages both trusted/encrypted key-types, like the encrypted key type
> + * data, trusted key type data is not visible decrypted from userspace.
> + */
> +static struct key *request_trusted_key(const char *trusted_desc,
> +				const u8 **master_key, size_t *master_keylen)
> +{
> +	struct trusted_key_payload *tpayload;
> +	struct key_type *type;
> +	struct key *tkey;
> +
> +	type = key_type_lookup("trusted");

The associated key_type_put() will need to be called.

> +	if (IS_ERR(type)) {
> +		tkey = (struct key *)type;
> +		goto error;
> +	}
> +	tkey = request_key(type, trusted_desc, NULL);
> +	if (IS_ERR(tkey))
> +		goto error;
> +
> +	down_read(&tkey->sem);
> +	tpayload = tkey->payload.data[0];
> +	*master_key = tpayload->key;
> +	*master_keylen = tpayload->key_len;
> +error:
> +	return tkey;
> +}
> +



> diff --git a/security/keys/key.c b/security/keys/key.c
> index 696f1c092c50..9045b62afb04 100644
> --- a/security/keys/key.c
> +++ b/security/keys/key.c
> @@ -706,6 +706,7 @@ struct key_type *key_type_lookup(const char *type)
>  found_kernel_type:
>  	return ktype;
>  }
> +EXPORT_SYMBOL_GPL(key_type_lookup);

Only the kernel is calling key_type_lookup().  Why does
key_type_lookup() need to be exported?

Mimi

>  
>  void key_set_timeout(struct key *key, unsigned timeout)
>  {
> 

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
  2019-03-20  0:06     ` Mimi Zohar
  (?)
@ 2019-03-20  0:20       ` Dan Williams
  -1 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-20  0:20 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: linux-nvdimm, James Bottomley, Linux Kernel Mailing List,
	David Howells, keyrings

On Tue, Mar 19, 2019 at 5:07 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Mon, 2019-03-18 at 23:06 -0700, Dan Williams wrote:
>
> < snip >
>
> > +/*
> > + * request_trusted_key - request the trusted key
> > + *
> > + * Trusted keys are sealed to PCRs and other metadata. Although userspace
> > + * manages both trusted/encrypted key-types, like the encrypted key type
> > + * data, trusted key type data is not visible decrypted from userspace.
> > + */
> > +static struct key *request_trusted_key(const char *trusted_desc,
> > +                             const u8 **master_key, size_t *master_keylen)
> > +{
> > +     struct trusted_key_payload *tpayload;
> > +     struct key_type *type;
> > +     struct key *tkey;
> > +
> > +     type = key_type_lookup("trusted");
>
> The associated key_type_put() will need to be called.

Yes.

>
> > +     if (IS_ERR(type)) {
> > +             tkey = (struct key *)type;
> > +             goto error;
> > +     }
> > +     tkey = request_key(type, trusted_desc, NULL);
> > +     if (IS_ERR(tkey))
> > +             goto error;
> > +
> > +     down_read(&tkey->sem);
> > +     tpayload = tkey->payload.data[0];
> > +     *master_key = tpayload->key;
> > +     *master_keylen = tpayload->key_len;
> > +error:
> > +     return tkey;
> > +}
> > +
>
>
>
> > diff --git a/security/keys/key.c b/security/keys/key.c
> > index 696f1c092c50..9045b62afb04 100644
> > --- a/security/keys/key.c
> > +++ b/security/keys/key.c
> > @@ -706,6 +706,7 @@ struct key_type *key_type_lookup(const char *type)
> >  found_kernel_type:
> >       return ktype;
> >  }
> > +EXPORT_SYMBOL_GPL(key_type_lookup);

This needs to be moved to patch1.

> Only the kernel is calling key_type_lookup().  Why does
> key_type_lookup() need to be exported?

This patch series adds several new callers outside of keys-subsystem
core that need this export, the first one being encrypted-keys itself
in patch1.

drivers/nvdimm/security.c:57:   type = key_type_lookup("encrypted");
fs/ecryptfs/keystore.c:1627:    type = key_type_lookup("encrypted");
security/integrity/evm/evm_crypto.c:361:        type =
key_type_lookup("encrypted");
security/keys/encrypted-keys/encrypted.c:440:   type =
key_type_lookup("trusted");
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
@ 2019-03-20  0:20       ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-20  0:20 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: keyrings, James Bottomley, David Howells, Vishal L Verma,
	linux-nvdimm, Linux Kernel Mailing List

On Tue, Mar 19, 2019 at 5:07 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Mon, 2019-03-18 at 23:06 -0700, Dan Williams wrote:
>
> < snip >
>
> > +/*
> > + * request_trusted_key - request the trusted key
> > + *
> > + * Trusted keys are sealed to PCRs and other metadata. Although userspace
> > + * manages both trusted/encrypted key-types, like the encrypted key type
> > + * data, trusted key type data is not visible decrypted from userspace.
> > + */
> > +static struct key *request_trusted_key(const char *trusted_desc,
> > +                             const u8 **master_key, size_t *master_keylen)
> > +{
> > +     struct trusted_key_payload *tpayload;
> > +     struct key_type *type;
> > +     struct key *tkey;
> > +
> > +     type = key_type_lookup("trusted");
>
> The associated key_type_put() will need to be called.

Yes.

>
> > +     if (IS_ERR(type)) {
> > +             tkey = (struct key *)type;
> > +             goto error;
> > +     }
> > +     tkey = request_key(type, trusted_desc, NULL);
> > +     if (IS_ERR(tkey))
> > +             goto error;
> > +
> > +     down_read(&tkey->sem);
> > +     tpayload = tkey->payload.data[0];
> > +     *master_key = tpayload->key;
> > +     *master_keylen = tpayload->key_len;
> > +error:
> > +     return tkey;
> > +}
> > +
>
>
>
> > diff --git a/security/keys/key.c b/security/keys/key.c
> > index 696f1c092c50..9045b62afb04 100644
> > --- a/security/keys/key.c
> > +++ b/security/keys/key.c
> > @@ -706,6 +706,7 @@ struct key_type *key_type_lookup(const char *type)
> >  found_kernel_type:
> >       return ktype;
> >  }
> > +EXPORT_SYMBOL_GPL(key_type_lookup);

This needs to be moved to patch1.

> Only the kernel is calling key_type_lookup().  Why does
> key_type_lookup() need to be exported?

This patch series adds several new callers outside of keys-subsystem
core that need this export, the first one being encrypted-keys itself
in patch1.

drivers/nvdimm/security.c:57:   type = key_type_lookup("encrypted");
fs/ecryptfs/keystore.c:1627:    type = key_type_lookup("encrypted");
security/integrity/evm/evm_crypto.c:361:        type =
key_type_lookup("encrypted");
security/keys/encrypted-keys/encrypted.c:440:   type =
key_type_lookup("trusted");

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
@ 2019-03-20  0:20       ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-20  0:20 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: linux-nvdimm, James Bottomley, Linux Kernel Mailing List,
	David Howells, keyrings

On Tue, Mar 19, 2019 at 5:07 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Mon, 2019-03-18 at 23:06 -0700, Dan Williams wrote:
>
> < snip >
>
> > +/*
> > + * request_trusted_key - request the trusted key
> > + *
> > + * Trusted keys are sealed to PCRs and other metadata. Although userspace
> > + * manages both trusted/encrypted key-types, like the encrypted key type
> > + * data, trusted key type data is not visible decrypted from userspace.
> > + */
> > +static struct key *request_trusted_key(const char *trusted_desc,
> > +                             const u8 **master_key, size_t *master_keylen)
> > +{
> > +     struct trusted_key_payload *tpayload;
> > +     struct key_type *type;
> > +     struct key *tkey;
> > +
> > +     type = key_type_lookup("trusted");
>
> The associated key_type_put() will need to be called.

Yes.

>
> > +     if (IS_ERR(type)) {
> > +             tkey = (struct key *)type;
> > +             goto error;
> > +     }
> > +     tkey = request_key(type, trusted_desc, NULL);
> > +     if (IS_ERR(tkey))
> > +             goto error;
> > +
> > +     down_read(&tkey->sem);
> > +     tpayload = tkey->payload.data[0];
> > +     *master_key = tpayload->key;
> > +     *master_keylen = tpayload->key_len;
> > +error:
> > +     return tkey;
> > +}
> > +
>
>
>
> > diff --git a/security/keys/key.c b/security/keys/key.c
> > index 696f1c092c50..9045b62afb04 100644
> > --- a/security/keys/key.c
> > +++ b/security/keys/key.c
> > @@ -706,6 +706,7 @@ struct key_type *key_type_lookup(const char *type)
> >  found_kernel_type:
> >       return ktype;
> >  }
> > +EXPORT_SYMBOL_GPL(key_type_lookup);

This needs to be moved to patch1.

> Only the kernel is calling key_type_lookup().  Why does
> key_type_lookup() need to be exported?

This patch series adds several new callers outside of keys-subsystem
core that need this export, the first one being encrypted-keys itself
in patch1.

drivers/nvdimm/security.c:57:   type = key_type_lookup("encrypted");
fs/ecryptfs/keystore.c:1627:    type = key_type_lookup("encrypted");
security/integrity/evm/evm_crypto.c:361:        type key_type_lookup("encrypted");
security/keys/encrypted-keys/encrypted.c:440:   type key_type_lookup("trusted");

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
  2019-03-20  0:20       ` Dan Williams
  (?)
@ 2019-03-20  1:10         ` Mimi Zohar
  -1 siblings, 0 replies; 70+ messages in thread
From: Mimi Zohar @ 2019-03-20  1:10 UTC (permalink / raw)
  To: Dan Williams
  Cc: linux-nvdimm, James Bottomley, Linux Kernel Mailing List,
	David Howells, keyrings

On Tue, 2019-03-19 at 17:20 -0700, Dan Williams wrote:
> On Tue, Mar 19, 2019 at 5:07 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > On Mon, 2019-03-18 at 23:06 -0700, Dan Williams wrote:
> 
> > > diff --git a/security/keys/key.c b/security/keys/key.c
> > > index 696f1c092c50..9045b62afb04 100644
> > > --- a/security/keys/key.c
> > > +++ b/security/keys/key.c
> > > @@ -706,6 +706,7 @@ struct key_type *key_type_lookup(const char *type)
> > >  found_kernel_type:
> > >       return ktype;
> > >  }
> > > +EXPORT_SYMBOL_GPL(key_type_lookup);
> 
> This needs to be moved to patch1.
> 
> > Only the kernel is calling key_type_lookup().  Why does
> > key_type_lookup() need to be exported?
> 
> This patch series adds several new callers outside of keys-subsystem
> core that need this export, the first one being encrypted-keys itself
> in patch1.

It's needed, because they could be compiled as kernel modules, not
builtin (eg. EVM).

Mimi

> 
> drivers/nvdimm/security.c:57:   type = key_type_lookup("encrypted");
> fs/ecryptfs/keystore.c:1627:    type = key_type_lookup("encrypted");
> security/integrity/evm/evm_crypto.c:361:        type =
> key_type_lookup("encrypted");
> security/keys/encrypted-keys/encrypted.c:440:   type =
> key_type_lookup("trusted");
> 

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
@ 2019-03-20  1:10         ` Mimi Zohar
  0 siblings, 0 replies; 70+ messages in thread
From: Mimi Zohar @ 2019-03-20  1:10 UTC (permalink / raw)
  To: Dan Williams
  Cc: keyrings, James Bottomley, David Howells, Vishal L Verma,
	linux-nvdimm, Linux Kernel Mailing List

On Tue, 2019-03-19 at 17:20 -0700, Dan Williams wrote:
> On Tue, Mar 19, 2019 at 5:07 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > On Mon, 2019-03-18 at 23:06 -0700, Dan Williams wrote:
> 
> > > diff --git a/security/keys/key.c b/security/keys/key.c
> > > index 696f1c092c50..9045b62afb04 100644
> > > --- a/security/keys/key.c
> > > +++ b/security/keys/key.c
> > > @@ -706,6 +706,7 @@ struct key_type *key_type_lookup(const char *type)
> > >  found_kernel_type:
> > >       return ktype;
> > >  }
> > > +EXPORT_SYMBOL_GPL(key_type_lookup);
> 
> This needs to be moved to patch1.
> 
> > Only the kernel is calling key_type_lookup().  Why does
> > key_type_lookup() need to be exported?
> 
> This patch series adds several new callers outside of keys-subsystem
> core that need this export, the first one being encrypted-keys itself
> in patch1.

It's needed, because they could be compiled as kernel modules, not
builtin (eg. EVM).

Mimi

> 
> drivers/nvdimm/security.c:57:   type = key_type_lookup("encrypted");
> fs/ecryptfs/keystore.c:1627:    type = key_type_lookup("encrypted");
> security/integrity/evm/evm_crypto.c:361:        type =
> key_type_lookup("encrypted");
> security/keys/encrypted-keys/encrypted.c:440:   type =
> key_type_lookup("trusted");
> 


^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
@ 2019-03-20  1:10         ` Mimi Zohar
  0 siblings, 0 replies; 70+ messages in thread
From: Mimi Zohar @ 2019-03-20  1:10 UTC (permalink / raw)
  To: Dan Williams
  Cc: linux-nvdimm, James Bottomley, Linux Kernel Mailing List,
	David Howells, keyrings

On Tue, 2019-03-19 at 17:20 -0700, Dan Williams wrote:
> On Tue, Mar 19, 2019 at 5:07 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > On Mon, 2019-03-18 at 23:06 -0700, Dan Williams wrote:
> 
> > > diff --git a/security/keys/key.c b/security/keys/key.c
> > > index 696f1c092c50..9045b62afb04 100644
> > > --- a/security/keys/key.c
> > > +++ b/security/keys/key.c
> > > @@ -706,6 +706,7 @@ struct key_type *key_type_lookup(const char *type)
> > >  found_kernel_type:
> > >       return ktype;
> > >  }
> > > +EXPORT_SYMBOL_GPL(key_type_lookup);
> 
> This needs to be moved to patch1.
> 
> > Only the kernel is calling key_type_lookup().  Why does
> > key_type_lookup() need to be exported?
> 
> This patch series adds several new callers outside of keys-subsystem
> core that need this export, the first one being encrypted-keys itself
> in patch1.

It's needed, because they could be compiled as kernel modules, not
builtin (eg. EVM).

Mimi

> 
> drivers/nvdimm/security.c:57:   type = key_type_lookup("encrypted");
> fs/ecryptfs/keystore.c:1627:    type = key_type_lookup("encrypted");
> security/integrity/evm/evm_crypto.c:361:        type > key_type_lookup("encrypted");
> security/keys/encrypted-keys/encrypted.c:440:   type > key_type_lookup("trusted");
> 

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
  2019-03-20  1:10         ` Mimi Zohar
  (?)
@ 2019-03-20  1:34           ` Dan Williams
  -1 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-20  1:34 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: linux-nvdimm, James Bottomley, Linux Kernel Mailing List,
	David Howells, keyrings

On Tue, Mar 19, 2019 at 6:11 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Tue, 2019-03-19 at 17:20 -0700, Dan Williams wrote:
> > On Tue, Mar 19, 2019 at 5:07 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > > On Mon, 2019-03-18 at 23:06 -0700, Dan Williams wrote:
> >
> > > > diff --git a/security/keys/key.c b/security/keys/key.c
> > > > index 696f1c092c50..9045b62afb04 100644
> > > > --- a/security/keys/key.c
> > > > +++ b/security/keys/key.c
> > > > @@ -706,6 +706,7 @@ struct key_type *key_type_lookup(const char *type)
> > > >  found_kernel_type:
> > > >       return ktype;
> > > >  }
> > > > +EXPORT_SYMBOL_GPL(key_type_lookup);
> >
> > This needs to be moved to patch1.
> >
> > > Only the kernel is calling key_type_lookup().  Why does
> > > key_type_lookup() need to be exported?
> >
> > This patch series adds several new callers outside of keys-subsystem
> > core that need this export, the first one being encrypted-keys itself
> > in patch1.
>
> It's needed, because they could be compiled as kernel modules, not
> builtin (eg. EVM).
>

Right, but now I realize a problem. The side effect of having direct
module dependencies to the key_type_{encrypted,trusted} symbols is
that module reference counting is handled automatically.

So, I need to respin this with some explicit try_module_get() and
module_put() added otherwise the encrypted_keys facility can be
removed while active keys are instantiated.

> > drivers/nvdimm/security.c:57:   type = key_type_lookup("encrypted");
> > fs/ecryptfs/keystore.c:1627:    type = key_type_lookup("encrypted");
> > security/integrity/evm/evm_crypto.c:361:        type =
> > key_type_lookup("encrypted");
> > security/keys/encrypted-keys/encrypted.c:440:   type =
> > key_type_lookup("trusted");
> >
>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
@ 2019-03-20  1:34           ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-20  1:34 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: keyrings, James Bottomley, David Howells, Vishal L Verma,
	linux-nvdimm, Linux Kernel Mailing List

On Tue, Mar 19, 2019 at 6:11 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Tue, 2019-03-19 at 17:20 -0700, Dan Williams wrote:
> > On Tue, Mar 19, 2019 at 5:07 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > > On Mon, 2019-03-18 at 23:06 -0700, Dan Williams wrote:
> >
> > > > diff --git a/security/keys/key.c b/security/keys/key.c
> > > > index 696f1c092c50..9045b62afb04 100644
> > > > --- a/security/keys/key.c
> > > > +++ b/security/keys/key.c
> > > > @@ -706,6 +706,7 @@ struct key_type *key_type_lookup(const char *type)
> > > >  found_kernel_type:
> > > >       return ktype;
> > > >  }
> > > > +EXPORT_SYMBOL_GPL(key_type_lookup);
> >
> > This needs to be moved to patch1.
> >
> > > Only the kernel is calling key_type_lookup().  Why does
> > > key_type_lookup() need to be exported?
> >
> > This patch series adds several new callers outside of keys-subsystem
> > core that need this export, the first one being encrypted-keys itself
> > in patch1.
>
> It's needed, because they could be compiled as kernel modules, not
> builtin (eg. EVM).
>

Right, but now I realize a problem. The side effect of having direct
module dependencies to the key_type_{encrypted,trusted} symbols is
that module reference counting is handled automatically.

So, I need to respin this with some explicit try_module_get() and
module_put() added otherwise the encrypted_keys facility can be
removed while active keys are instantiated.

> > drivers/nvdimm/security.c:57:   type = key_type_lookup("encrypted");
> > fs/ecryptfs/keystore.c:1627:    type = key_type_lookup("encrypted");
> > security/integrity/evm/evm_crypto.c:361:        type =
> > key_type_lookup("encrypted");
> > security/keys/encrypted-keys/encrypted.c:440:   type =
> > key_type_lookup("trusted");
> >
>

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
@ 2019-03-20  1:34           ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-20  1:34 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: linux-nvdimm, James Bottomley, Linux Kernel Mailing List,
	David Howells, keyrings

On Tue, Mar 19, 2019 at 6:11 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Tue, 2019-03-19 at 17:20 -0700, Dan Williams wrote:
> > On Tue, Mar 19, 2019 at 5:07 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > > On Mon, 2019-03-18 at 23:06 -0700, Dan Williams wrote:
> >
> > > > diff --git a/security/keys/key.c b/security/keys/key.c
> > > > index 696f1c092c50..9045b62afb04 100644
> > > > --- a/security/keys/key.c
> > > > +++ b/security/keys/key.c
> > > > @@ -706,6 +706,7 @@ struct key_type *key_type_lookup(const char *type)
> > > >  found_kernel_type:
> > > >       return ktype;
> > > >  }
> > > > +EXPORT_SYMBOL_GPL(key_type_lookup);
> >
> > This needs to be moved to patch1.
> >
> > > Only the kernel is calling key_type_lookup().  Why does
> > > key_type_lookup() need to be exported?
> >
> > This patch series adds several new callers outside of keys-subsystem
> > core that need this export, the first one being encrypted-keys itself
> > in patch1.
>
> It's needed, because they could be compiled as kernel modules, not
> builtin (eg. EVM).
>

Right, but now I realize a problem. The side effect of having direct
module dependencies to the key_type_{encrypted,trusted} symbols is
that module reference counting is handled automatically.

So, I need to respin this with some explicit try_module_get() and
module_put() added otherwise the encrypted_keys facility can be
removed while active keys are instantiated.

> > drivers/nvdimm/security.c:57:   type = key_type_lookup("encrypted");
> > fs/ecryptfs/keystore.c:1627:    type = key_type_lookup("encrypted");
> > security/integrity/evm/evm_crypto.c:361:        type > > key_type_lookup("encrypted");
> > security/keys/encrypted-keys/encrypted.c:440:   type > > key_type_lookup("trusted");
> >
>

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
  2019-03-20  1:34           ` Dan Williams
  (?)
@ 2019-03-20  1:37             ` Dan Williams
  -1 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-20  1:37 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: linux-nvdimm, James Bottomley, Linux Kernel Mailing List,
	David Howells, keyrings

On Tue, Mar 19, 2019 at 6:34 PM Dan Williams <dan.j.williams@intel.com> wrote:
>
> On Tue, Mar 19, 2019 at 6:11 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Tue, 2019-03-19 at 17:20 -0700, Dan Williams wrote:
> > > On Tue, Mar 19, 2019 at 5:07 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > > > On Mon, 2019-03-18 at 23:06 -0700, Dan Williams wrote:
> > >
> > > > > diff --git a/security/keys/key.c b/security/keys/key.c
> > > > > index 696f1c092c50..9045b62afb04 100644
> > > > > --- a/security/keys/key.c
> > > > > +++ b/security/keys/key.c
> > > > > @@ -706,6 +706,7 @@ struct key_type *key_type_lookup(const char *type)
> > > > >  found_kernel_type:
> > > > >       return ktype;
> > > > >  }
> > > > > +EXPORT_SYMBOL_GPL(key_type_lookup);
> > >
> > > This needs to be moved to patch1.
> > >
> > > > Only the kernel is calling key_type_lookup().  Why does
> > > > key_type_lookup() need to be exported?
> > >
> > > This patch series adds several new callers outside of keys-subsystem
> > > core that need this export, the first one being encrypted-keys itself
> > > in patch1.
> >
> > It's needed, because they could be compiled as kernel modules, not
> > builtin (eg. EVM).
> >
>
> Right, but now I realize a problem. The side effect of having direct
> module dependencies to the key_type_{encrypted,trusted} symbols is
> that module reference counting is handled automatically.
>
> So, I need to respin this with some explicit try_module_get() and
> module_put() added otherwise the encrypted_keys facility can be
> removed while active keys are instantiated.

...and now I'm wondering if this refactoring is getting too big and
should wait for v5.2. In the meantime apply my small fix for v5.1

https://patchwork.kernel.org/patch/10858649/
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
@ 2019-03-20  1:37             ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-20  1:37 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: keyrings, James Bottomley, David Howells, Vishal L Verma,
	linux-nvdimm, Linux Kernel Mailing List

On Tue, Mar 19, 2019 at 6:34 PM Dan Williams <dan.j.williams@intel.com> wrote:
>
> On Tue, Mar 19, 2019 at 6:11 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Tue, 2019-03-19 at 17:20 -0700, Dan Williams wrote:
> > > On Tue, Mar 19, 2019 at 5:07 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > > > On Mon, 2019-03-18 at 23:06 -0700, Dan Williams wrote:
> > >
> > > > > diff --git a/security/keys/key.c b/security/keys/key.c
> > > > > index 696f1c092c50..9045b62afb04 100644
> > > > > --- a/security/keys/key.c
> > > > > +++ b/security/keys/key.c
> > > > > @@ -706,6 +706,7 @@ struct key_type *key_type_lookup(const char *type)
> > > > >  found_kernel_type:
> > > > >       return ktype;
> > > > >  }
> > > > > +EXPORT_SYMBOL_GPL(key_type_lookup);
> > >
> > > This needs to be moved to patch1.
> > >
> > > > Only the kernel is calling key_type_lookup().  Why does
> > > > key_type_lookup() need to be exported?
> > >
> > > This patch series adds several new callers outside of keys-subsystem
> > > core that need this export, the first one being encrypted-keys itself
> > > in patch1.
> >
> > It's needed, because they could be compiled as kernel modules, not
> > builtin (eg. EVM).
> >
>
> Right, but now I realize a problem. The side effect of having direct
> module dependencies to the key_type_{encrypted,trusted} symbols is
> that module reference counting is handled automatically.
>
> So, I need to respin this with some explicit try_module_get() and
> module_put() added otherwise the encrypted_keys facility can be
> removed while active keys are instantiated.

...and now I'm wondering if this refactoring is getting too big and
should wait for v5.2. In the meantime apply my small fix for v5.1

https://patchwork.kernel.org/patch/10858649/

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
@ 2019-03-20  1:37             ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-20  1:37 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: linux-nvdimm, James Bottomley, Linux Kernel Mailing List,
	David Howells, keyrings

On Tue, Mar 19, 2019 at 6:34 PM Dan Williams <dan.j.williams@intel.com> wrote:
>
> On Tue, Mar 19, 2019 at 6:11 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Tue, 2019-03-19 at 17:20 -0700, Dan Williams wrote:
> > > On Tue, Mar 19, 2019 at 5:07 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > > > On Mon, 2019-03-18 at 23:06 -0700, Dan Williams wrote:
> > >
> > > > > diff --git a/security/keys/key.c b/security/keys/key.c
> > > > > index 696f1c092c50..9045b62afb04 100644
> > > > > --- a/security/keys/key.c
> > > > > +++ b/security/keys/key.c
> > > > > @@ -706,6 +706,7 @@ struct key_type *key_type_lookup(const char *type)
> > > > >  found_kernel_type:
> > > > >       return ktype;
> > > > >  }
> > > > > +EXPORT_SYMBOL_GPL(key_type_lookup);
> > >
> > > This needs to be moved to patch1.
> > >
> > > > Only the kernel is calling key_type_lookup().  Why does
> > > > key_type_lookup() need to be exported?
> > >
> > > This patch series adds several new callers outside of keys-subsystem
> > > core that need this export, the first one being encrypted-keys itself
> > > in patch1.
> >
> > It's needed, because they could be compiled as kernel modules, not
> > builtin (eg. EVM).
> >
>
> Right, but now I realize a problem. The side effect of having direct
> module dependencies to the key_type_{encrypted,trusted} symbols is
> that module reference counting is handled automatically.
>
> So, I need to respin this with some explicit try_module_get() and
> module_put() added otherwise the encrypted_keys facility can be
> removed while active keys are instantiated.

...and now I'm wondering if this refactoring is getting too big and
should wait for v5.2. In the meantime apply my small fix for v5.1

https://patchwork.kernel.org/patch/10858649/

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
  2019-03-20  0:20       ` Dan Williams
  (?)
@ 2019-03-20  2:35         ` Mimi Zohar
  -1 siblings, 0 replies; 70+ messages in thread
From: Mimi Zohar @ 2019-03-20  2:35 UTC (permalink / raw)
  To: Dan Williams
  Cc: linux-nvdimm, James Bottomley, Linux Kernel Mailing List,
	David Howells, keyrings

On Tue, 2019-03-19 at 17:20 -0700, Dan Williams wrote:
> On Tue, Mar 19, 2019 at 5:07 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Mon, 2019-03-18 at 23:06 -0700, Dan Williams wrote:
> >
> > < snip >
> >
> > > +/*
> > > + * request_trusted_key - request the trusted key
> > > + *
> > > + * Trusted keys are sealed to PCRs and other metadata. Although userspace
> > > + * manages both trusted/encrypted key-types, like the encrypted key type
> > > + * data, trusted key type data is not visible decrypted from userspace.
> > > + */
> > > +static struct key *request_trusted_key(const char *trusted_desc,
> > > +                             const u8 **master_key, size_t *master_keylen)
> > > +{
> > > +     struct trusted_key_payload *tpayload;
> > > +     struct key_type *type;
> > > +     struct key *tkey;
> > > +
> > > +     type = key_type_lookup("trusted");
> >
> > The associated key_type_put() will need to be called.
> 
> Yes.

I don't know if defining a key_type_lookup() wrapper, perhaps named
is_key_type_available(), would help.  Both key_type_lookup() and
key_type_put() would be called.  The existing code could then remain
the same.

Mimi

> 
> >
> > > +     if (IS_ERR(type)) {
> > > +             tkey = (struct key *)type;
> > > +             goto error;
> > > +     }
> > > +     tkey = request_key(type, trusted_desc, NULL);
> > > +     if (IS_ERR(tkey))
> > > +             goto error;
> > > +
> > > +     down_read(&tkey->sem);
> > > +     tpayload = tkey->payload.data[0];
> > > +     *master_key = tpayload->key;
> > > +     *master_keylen = tpayload->key_len;
> > > +error:
> > > +     return tkey;
> > > +}
> > > +
> >

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
@ 2019-03-20  2:35         ` Mimi Zohar
  0 siblings, 0 replies; 70+ messages in thread
From: Mimi Zohar @ 2019-03-20  2:35 UTC (permalink / raw)
  To: Dan Williams
  Cc: keyrings, James Bottomley, David Howells, Vishal L Verma,
	linux-nvdimm, Linux Kernel Mailing List

On Tue, 2019-03-19 at 17:20 -0700, Dan Williams wrote:
> On Tue, Mar 19, 2019 at 5:07 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Mon, 2019-03-18 at 23:06 -0700, Dan Williams wrote:
> >
> > < snip >
> >
> > > +/*
> > > + * request_trusted_key - request the trusted key
> > > + *
> > > + * Trusted keys are sealed to PCRs and other metadata. Although userspace
> > > + * manages both trusted/encrypted key-types, like the encrypted key type
> > > + * data, trusted key type data is not visible decrypted from userspace.
> > > + */
> > > +static struct key *request_trusted_key(const char *trusted_desc,
> > > +                             const u8 **master_key, size_t *master_keylen)
> > > +{
> > > +     struct trusted_key_payload *tpayload;
> > > +     struct key_type *type;
> > > +     struct key *tkey;
> > > +
> > > +     type = key_type_lookup("trusted");
> >
> > The associated key_type_put() will need to be called.
> 
> Yes.

I don't know if defining a key_type_lookup() wrapper, perhaps named
is_key_type_available(), would help.  Both key_type_lookup() and
key_type_put() would be called.  The existing code could then remain
the same.

Mimi

> 
> >
> > > +     if (IS_ERR(type)) {
> > > +             tkey = (struct key *)type;
> > > +             goto error;
> > > +     }
> > > +     tkey = request_key(type, trusted_desc, NULL);
> > > +     if (IS_ERR(tkey))
> > > +             goto error;
> > > +
> > > +     down_read(&tkey->sem);
> > > +     tpayload = tkey->payload.data[0];
> > > +     *master_key = tpayload->key;
> > > +     *master_keylen = tpayload->key_len;
> > > +error:
> > > +     return tkey;
> > > +}
> > > +
> >


^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
@ 2019-03-20  2:35         ` Mimi Zohar
  0 siblings, 0 replies; 70+ messages in thread
From: Mimi Zohar @ 2019-03-20  2:35 UTC (permalink / raw)
  To: Dan Williams
  Cc: linux-nvdimm, James Bottomley, Linux Kernel Mailing List,
	David Howells, keyrings

On Tue, 2019-03-19 at 17:20 -0700, Dan Williams wrote:
> On Tue, Mar 19, 2019 at 5:07 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Mon, 2019-03-18 at 23:06 -0700, Dan Williams wrote:
> >
> > < snip >
> >
> > > +/*
> > > + * request_trusted_key - request the trusted key
> > > + *
> > > + * Trusted keys are sealed to PCRs and other metadata. Although userspace
> > > + * manages both trusted/encrypted key-types, like the encrypted key type
> > > + * data, trusted key type data is not visible decrypted from userspace.
> > > + */
> > > +static struct key *request_trusted_key(const char *trusted_desc,
> > > +                             const u8 **master_key, size_t *master_keylen)
> > > +{
> > > +     struct trusted_key_payload *tpayload;
> > > +     struct key_type *type;
> > > +     struct key *tkey;
> > > +
> > > +     type = key_type_lookup("trusted");
> >
> > The associated key_type_put() will need to be called.
> 
> Yes.

I don't know if defining a key_type_lookup() wrapper, perhaps named
is_key_type_available(), would help. ┬áBoth key_type_lookup() and
key_type_put() would be called. ┬áThe existing code could then remain
the same.

Mimi

> 
> >
> > > +     if (IS_ERR(type)) {
> > > +             tkey = (struct key *)type;
> > > +             goto error;
> > > +     }
> > > +     tkey = request_key(type, trusted_desc, NULL);
> > > +     if (IS_ERR(tkey))
> > > +             goto error;
> > > +
> > > +     down_read(&tkey->sem);
> > > +     tpayload = tkey->payload.data[0];
> > > +     *master_key = tpayload->key;
> > > +     *master_keylen = tpayload->key_len;
> > > +error:
> > > +     return tkey;
> > > +}
> > > +
> >

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
  2019-03-20  2:35         ` Mimi Zohar
  (?)
@ 2019-03-20  5:48           ` Dan Williams
  -1 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-20  5:48 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: linux-nvdimm, James Bottomley, Linux Kernel Mailing List,
	David Howells, keyrings

On Tue, Mar 19, 2019 at 7:36 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Tue, 2019-03-19 at 17:20 -0700, Dan Williams wrote:
> > On Tue, Mar 19, 2019 at 5:07 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > >
> > > On Mon, 2019-03-18 at 23:06 -0700, Dan Williams wrote:
> > >
> > > < snip >
> > >
> > > > +/*
> > > > + * request_trusted_key - request the trusted key
> > > > + *
> > > > + * Trusted keys are sealed to PCRs and other metadata. Although userspace
> > > > + * manages both trusted/encrypted key-types, like the encrypted key type
> > > > + * data, trusted key type data is not visible decrypted from userspace.
> > > > + */
> > > > +static struct key *request_trusted_key(const char *trusted_desc,
> > > > +                             const u8 **master_key, size_t *master_keylen)
> > > > +{
> > > > +     struct trusted_key_payload *tpayload;
> > > > +     struct key_type *type;
> > > > +     struct key *tkey;
> > > > +
> > > > +     type = key_type_lookup("trusted");
> > >
> > > The associated key_type_put() will need to be called.
> >
> > Yes.
>
> I don't know if defining a key_type_lookup() wrapper, perhaps named
> is_key_type_available(), would help.  Both key_type_lookup() and
> key_type_put() would be called.  The existing code could then remain
> the same.
>

Maybe, but something still needs to pin the hosting module. I think
this means that the first call to key_type->instantiate() pins the
hosting module, and the ->destroy() of the last key for the key_type
unpins the module. It does mean that the ->destroy() method is no
longer optional.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
@ 2019-03-20  5:48           ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-20  5:48 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: keyrings, James Bottomley, David Howells, Vishal L Verma,
	linux-nvdimm, Linux Kernel Mailing List

On Tue, Mar 19, 2019 at 7:36 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Tue, 2019-03-19 at 17:20 -0700, Dan Williams wrote:
> > On Tue, Mar 19, 2019 at 5:07 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > >
> > > On Mon, 2019-03-18 at 23:06 -0700, Dan Williams wrote:
> > >
> > > < snip >
> > >
> > > > +/*
> > > > + * request_trusted_key - request the trusted key
> > > > + *
> > > > + * Trusted keys are sealed to PCRs and other metadata. Although userspace
> > > > + * manages both trusted/encrypted key-types, like the encrypted key type
> > > > + * data, trusted key type data is not visible decrypted from userspace.
> > > > + */
> > > > +static struct key *request_trusted_key(const char *trusted_desc,
> > > > +                             const u8 **master_key, size_t *master_keylen)
> > > > +{
> > > > +     struct trusted_key_payload *tpayload;
> > > > +     struct key_type *type;
> > > > +     struct key *tkey;
> > > > +
> > > > +     type = key_type_lookup("trusted");
> > >
> > > The associated key_type_put() will need to be called.
> >
> > Yes.
>
> I don't know if defining a key_type_lookup() wrapper, perhaps named
> is_key_type_available(), would help.  Both key_type_lookup() and
> key_type_put() would be called.  The existing code could then remain
> the same.
>

Maybe, but something still needs to pin the hosting module. I think
this means that the first call to key_type->instantiate() pins the
hosting module, and the ->destroy() of the last key for the key_type
unpins the module. It does mean that the ->destroy() method is no
longer optional.

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
@ 2019-03-20  5:48           ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-20  5:48 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: linux-nvdimm, James Bottomley, Linux Kernel Mailing List,
	David Howells, keyrings

On Tue, Mar 19, 2019 at 7:36 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Tue, 2019-03-19 at 17:20 -0700, Dan Williams wrote:
> > On Tue, Mar 19, 2019 at 5:07 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > >
> > > On Mon, 2019-03-18 at 23:06 -0700, Dan Williams wrote:
> > >
> > > < snip >
> > >
> > > > +/*
> > > > + * request_trusted_key - request the trusted key
> > > > + *
> > > > + * Trusted keys are sealed to PCRs and other metadata. Although userspace
> > > > + * manages both trusted/encrypted key-types, like the encrypted key type
> > > > + * data, trusted key type data is not visible decrypted from userspace.
> > > > + */
> > > > +static struct key *request_trusted_key(const char *trusted_desc,
> > > > +                             const u8 **master_key, size_t *master_keylen)
> > > > +{
> > > > +     struct trusted_key_payload *tpayload;
> > > > +     struct key_type *type;
> > > > +     struct key *tkey;
> > > > +
> > > > +     type = key_type_lookup("trusted");
> > >
> > > The associated key_type_put() will need to be called.
> >
> > Yes.
>
> I don't know if defining a key_type_lookup() wrapper, perhaps named
> is_key_type_available(), would help.  Both key_type_lookup() and
> key_type_put() would be called.  The existing code could then remain
> the same.
>

Maybe, but something still needs to pin the hosting module. I think
this means that the first call to key_type->instantiate() pins the
hosting module, and the ->destroy() of the last key for the key_type
unpins the module. It does mean that the ->destroy() method is no
longer optional.

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
  2019-03-20  5:48           ` Dan Williams
  (?)
@ 2019-03-20 12:06             ` Mimi Zohar
  -1 siblings, 0 replies; 70+ messages in thread
From: Mimi Zohar @ 2019-03-20 12:06 UTC (permalink / raw)
  To: Dan Williams
  Cc: linux-nvdimm, James Bottomley, Linux Kernel Mailing List,
	David Howells, keyrings

On Tue, 2019-03-19 at 22:48 -0700, Dan Williams wrote:
> On Tue, Mar 19, 2019 at 7:36 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Tue, 2019-03-19 at 17:20 -0700, Dan Williams wrote:
> > > On Tue, Mar 19, 2019 at 5:07 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > > >
> > > > On Mon, 2019-03-18 at 23:06 -0700, Dan Williams wrote:
> > > >
> > > > < snip >
> > > >
> > > > > +/*
> > > > > + * request_trusted_key - request the trusted key
> > > > > + *
> > > > > + * Trusted keys are sealed to PCRs and other metadata. Although userspace
> > > > > + * manages both trusted/encrypted key-types, like the encrypted key type
> > > > > + * data, trusted key type data is not visible decrypted from userspace.
> > > > > + */
> > > > > +static struct key *request_trusted_key(const char *trusted_desc,
> > > > > +                             const u8 **master_key, size_t *master_keylen)
> > > > > +{
> > > > > +     struct trusted_key_payload *tpayload;
> > > > > +     struct key_type *type;
> > > > > +     struct key *tkey;
> > > > > +
> > > > > +     type = key_type_lookup("trusted");
> > > >
> > > > The associated key_type_put() will need to be called.
> > >
> > > Yes.
> >
> > I don't know if defining a key_type_lookup() wrapper, perhaps named
> > is_key_type_available(), would help.  Both key_type_lookup() and
> > key_type_put() would be called.  The existing code could then remain
> > the same.
> >
> 
> Maybe, but something still needs to pin the hosting module. I think
> this means that the first call to key_type->instantiate() pins the
> hosting module, and the ->destroy() of the last key for the key_type
> unpins the module. It does mean that the ->destroy() method is no
> longer optional.

This sounds like it isn't a new problem.  Both issues need to be
addressed, but I think we should differentiate between them and
address them separately.

In terms of the original nvdimm encrypted/trusted key problem, the
above suggestion requires the least amount of change.  For v5.2, I
would replace it with the full updated patch set.

Mimi

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
@ 2019-03-20 12:06             ` Mimi Zohar
  0 siblings, 0 replies; 70+ messages in thread
From: Mimi Zohar @ 2019-03-20 12:06 UTC (permalink / raw)
  To: Dan Williams
  Cc: keyrings, James Bottomley, David Howells, Vishal L Verma,
	linux-nvdimm, Linux Kernel Mailing List

On Tue, 2019-03-19 at 22:48 -0700, Dan Williams wrote:
> On Tue, Mar 19, 2019 at 7:36 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Tue, 2019-03-19 at 17:20 -0700, Dan Williams wrote:
> > > On Tue, Mar 19, 2019 at 5:07 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > > >
> > > > On Mon, 2019-03-18 at 23:06 -0700, Dan Williams wrote:
> > > >
> > > > < snip >
> > > >
> > > > > +/*
> > > > > + * request_trusted_key - request the trusted key
> > > > > + *
> > > > > + * Trusted keys are sealed to PCRs and other metadata. Although userspace
> > > > > + * manages both trusted/encrypted key-types, like the encrypted key type
> > > > > + * data, trusted key type data is not visible decrypted from userspace.
> > > > > + */
> > > > > +static struct key *request_trusted_key(const char *trusted_desc,
> > > > > +                             const u8 **master_key, size_t *master_keylen)
> > > > > +{
> > > > > +     struct trusted_key_payload *tpayload;
> > > > > +     struct key_type *type;
> > > > > +     struct key *tkey;
> > > > > +
> > > > > +     type = key_type_lookup("trusted");
> > > >
> > > > The associated key_type_put() will need to be called.
> > >
> > > Yes.
> >
> > I don't know if defining a key_type_lookup() wrapper, perhaps named
> > is_key_type_available(), would help.  Both key_type_lookup() and
> > key_type_put() would be called.  The existing code could then remain
> > the same.
> >
> 
> Maybe, but something still needs to pin the hosting module. I think
> this means that the first call to key_type->instantiate() pins the
> hosting module, and the ->destroy() of the last key for the key_type
> unpins the module. It does mean that the ->destroy() method is no
> longer optional.

This sounds like it isn't a new problem.  Both issues need to be
addressed, but I think we should differentiate between them and
address them separately.

In terms of the original nvdimm encrypted/trusted key problem, the
above suggestion requires the least amount of change.  For v5.2, I
would replace it with the full updated patch set.

Mimi


^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
@ 2019-03-20 12:06             ` Mimi Zohar
  0 siblings, 0 replies; 70+ messages in thread
From: Mimi Zohar @ 2019-03-20 12:06 UTC (permalink / raw)
  To: Dan Williams
  Cc: linux-nvdimm, James Bottomley, Linux Kernel Mailing List,
	David Howells, keyrings

On Tue, 2019-03-19 at 22:48 -0700, Dan Williams wrote:
> On Tue, Mar 19, 2019 at 7:36 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Tue, 2019-03-19 at 17:20 -0700, Dan Williams wrote:
> > > On Tue, Mar 19, 2019 at 5:07 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > > >
> > > > On Mon, 2019-03-18 at 23:06 -0700, Dan Williams wrote:
> > > >
> > > > < snip >
> > > >
> > > > > +/*
> > > > > + * request_trusted_key - request the trusted key
> > > > > + *
> > > > > + * Trusted keys are sealed to PCRs and other metadata. Although userspace
> > > > > + * manages both trusted/encrypted key-types, like the encrypted key type
> > > > > + * data, trusted key type data is not visible decrypted from userspace.
> > > > > + */
> > > > > +static struct key *request_trusted_key(const char *trusted_desc,
> > > > > +                             const u8 **master_key, size_t *master_keylen)
> > > > > +{
> > > > > +     struct trusted_key_payload *tpayload;
> > > > > +     struct key_type *type;
> > > > > +     struct key *tkey;
> > > > > +
> > > > > +     type = key_type_lookup("trusted");
> > > >
> > > > The associated key_type_put() will need to be called.
> > >
> > > Yes.
> >
> > I don't know if defining a key_type_lookup() wrapper, perhaps named
> > is_key_type_available(), would help.  Both key_type_lookup() and
> > key_type_put() would be called.  The existing code could then remain
> > the same.
> >
> 
> Maybe, but something still needs to pin the hosting module. I think
> this means that the first call to key_type->instantiate() pins the
> hosting module, and the ->destroy() of the last key for the key_type
> unpins the module. It does mean that the ->destroy() method is no
> longer optional.

This sounds like it isn't a new problem.  Both issues need to be
addressed, but I think we should differentiate between them and
address them separately.

In terms of the original nvdimm encrypted/trusted key problem, the
above suggestion requires the least amount of change.  For v5.2, I
would replace it with the full updated patch set.

Mimi

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
  2019-03-20 12:06             ` Mimi Zohar
  (?)
@ 2019-03-20 15:27               ` Dan Williams
  -1 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-20 15:27 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: linux-nvdimm, James Bottomley, Linux Kernel Mailing List,
	David Howells, keyrings

On Wed, Mar 20, 2019 at 5:07 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Tue, 2019-03-19 at 22:48 -0700, Dan Williams wrote:
> > On Tue, Mar 19, 2019 at 7:36 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > >
> > > On Tue, 2019-03-19 at 17:20 -0700, Dan Williams wrote:
> > > > On Tue, Mar 19, 2019 at 5:07 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > > > >
> > > > > On Mon, 2019-03-18 at 23:06 -0700, Dan Williams wrote:
> > > > >
> > > > > < snip >
> > > > >
> > > > > > +/*
> > > > > > + * request_trusted_key - request the trusted key
> > > > > > + *
> > > > > > + * Trusted keys are sealed to PCRs and other metadata. Although userspace
> > > > > > + * manages both trusted/encrypted key-types, like the encrypted key type
> > > > > > + * data, trusted key type data is not visible decrypted from userspace.
> > > > > > + */
> > > > > > +static struct key *request_trusted_key(const char *trusted_desc,
> > > > > > +                             const u8 **master_key, size_t *master_keylen)
> > > > > > +{
> > > > > > +     struct trusted_key_payload *tpayload;
> > > > > > +     struct key_type *type;
> > > > > > +     struct key *tkey;
> > > > > > +
> > > > > > +     type = key_type_lookup("trusted");
> > > > >
> > > > > The associated key_type_put() will need to be called.
> > > >
> > > > Yes.
> > >
> > > I don't know if defining a key_type_lookup() wrapper, perhaps named
> > > is_key_type_available(), would help.  Both key_type_lookup() and
> > > key_type_put() would be called.  The existing code could then remain
> > > the same.
> > >
> >
> > Maybe, but something still needs to pin the hosting module. I think
> > this means that the first call to key_type->instantiate() pins the
> > hosting module, and the ->destroy() of the last key for the key_type
> > unpins the module. It does mean that the ->destroy() method is no
> > longer optional.
>
> This sounds like it isn't a new problem.  Both issues need to be
> addressed, but I think we should differentiate between them and
> address them separately.
>
> In terms of the original nvdimm encrypted/trusted key problem, the
> above suggestion requires the least amount of change.  For v5.2, I
> would replace it with the full updated patch set.

I believe smallest amount of change is this single patch:

https://patchwork.kernel.org/patch/10858649/
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
@ 2019-03-20 15:27               ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-20 15:27 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: keyrings, James Bottomley, David Howells, Vishal L Verma,
	linux-nvdimm, Linux Kernel Mailing List

On Wed, Mar 20, 2019 at 5:07 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Tue, 2019-03-19 at 22:48 -0700, Dan Williams wrote:
> > On Tue, Mar 19, 2019 at 7:36 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > >
> > > On Tue, 2019-03-19 at 17:20 -0700, Dan Williams wrote:
> > > > On Tue, Mar 19, 2019 at 5:07 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > > > >
> > > > > On Mon, 2019-03-18 at 23:06 -0700, Dan Williams wrote:
> > > > >
> > > > > < snip >
> > > > >
> > > > > > +/*
> > > > > > + * request_trusted_key - request the trusted key
> > > > > > + *
> > > > > > + * Trusted keys are sealed to PCRs and other metadata. Although userspace
> > > > > > + * manages both trusted/encrypted key-types, like the encrypted key type
> > > > > > + * data, trusted key type data is not visible decrypted from userspace.
> > > > > > + */
> > > > > > +static struct key *request_trusted_key(const char *trusted_desc,
> > > > > > +                             const u8 **master_key, size_t *master_keylen)
> > > > > > +{
> > > > > > +     struct trusted_key_payload *tpayload;
> > > > > > +     struct key_type *type;
> > > > > > +     struct key *tkey;
> > > > > > +
> > > > > > +     type = key_type_lookup("trusted");
> > > > >
> > > > > The associated key_type_put() will need to be called.
> > > >
> > > > Yes.
> > >
> > > I don't know if defining a key_type_lookup() wrapper, perhaps named
> > > is_key_type_available(), would help.  Both key_type_lookup() and
> > > key_type_put() would be called.  The existing code could then remain
> > > the same.
> > >
> >
> > Maybe, but something still needs to pin the hosting module. I think
> > this means that the first call to key_type->instantiate() pins the
> > hosting module, and the ->destroy() of the last key for the key_type
> > unpins the module. It does mean that the ->destroy() method is no
> > longer optional.
>
> This sounds like it isn't a new problem.  Both issues need to be
> addressed, but I think we should differentiate between them and
> address them separately.
>
> In terms of the original nvdimm encrypted/trusted key problem, the
> above suggestion requires the least amount of change.  For v5.2, I
> would replace it with the full updated patch set.

I believe smallest amount of change is this single patch:

https://patchwork.kernel.org/patch/10858649/

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key()
@ 2019-03-20 15:27               ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-20 15:27 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: linux-nvdimm, James Bottomley, Linux Kernel Mailing List,
	David Howells, keyrings

On Wed, Mar 20, 2019 at 5:07 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Tue, 2019-03-19 at 22:48 -0700, Dan Williams wrote:
> > On Tue, Mar 19, 2019 at 7:36 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > >
> > > On Tue, 2019-03-19 at 17:20 -0700, Dan Williams wrote:
> > > > On Tue, Mar 19, 2019 at 5:07 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > > > >
> > > > > On Mon, 2019-03-18 at 23:06 -0700, Dan Williams wrote:
> > > > >
> > > > > < snip >
> > > > >
> > > > > > +/*
> > > > > > + * request_trusted_key - request the trusted key
> > > > > > + *
> > > > > > + * Trusted keys are sealed to PCRs and other metadata. Although userspace
> > > > > > + * manages both trusted/encrypted key-types, like the encrypted key type
> > > > > > + * data, trusted key type data is not visible decrypted from userspace.
> > > > > > + */
> > > > > > +static struct key *request_trusted_key(const char *trusted_desc,
> > > > > > +                             const u8 **master_key, size_t *master_keylen)
> > > > > > +{
> > > > > > +     struct trusted_key_payload *tpayload;
> > > > > > +     struct key_type *type;
> > > > > > +     struct key *tkey;
> > > > > > +
> > > > > > +     type = key_type_lookup("trusted");
> > > > >
> > > > > The associated key_type_put() will need to be called.
> > > >
> > > > Yes.
> > >
> > > I don't know if defining a key_type_lookup() wrapper, perhaps named
> > > is_key_type_available(), would help.  Both key_type_lookup() and
> > > key_type_put() would be called.  The existing code could then remain
> > > the same.
> > >
> >
> > Maybe, but something still needs to pin the hosting module. I think
> > this means that the first call to key_type->instantiate() pins the
> > hosting module, and the ->destroy() of the last key for the key_type
> > unpins the module. It does mean that the ->destroy() method is no
> > longer optional.
>
> This sounds like it isn't a new problem.  Both issues need to be
> addressed, but I think we should differentiate between them and
> address them separately.
>
> In terms of the original nvdimm encrypted/trusted key problem, the
> above suggestion requires the least amount of change.  For v5.2, I
> would replace it with the full updated patch set.

I believe smallest amount of change is this single patch:

https://patchwork.kernel.org/patch/10858649/

^ permalink raw reply	[flat|nested] 70+ messages in thread

* [PATCH 3/6] libnvdimm/security: Drop direct dependency on key_type_encrypted
  2019-03-19  6:06 ` Dan Williams
  (?)
@ 2019-03-19  6:06   ` Dan Williams
  -1 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-19  6:06 UTC (permalink / raw)
  To: keyrings; +Cc: linux-nvdimm, linux-kernel

Lookup the key type by name and protect libnvdimm from encrypted_keys.ko
module load failures.

Cc: Vishal Verma <vishal.l.verma@intel.com>
Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Keith Busch <keith.busch@intel.com>
Cc: Ira Weiny <ira.weiny@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/nvdimm/security.c |   11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c
index f8bb746a549f..7f9e412f743a 100644
--- a/drivers/nvdimm/security.c
+++ b/drivers/nvdimm/security.c
@@ -48,12 +48,17 @@ static void nvdimm_put_key(struct key *key)
 static struct key *nvdimm_request_key(struct nvdimm *nvdimm)
 {
 	struct key *key = NULL;
+	struct key_type *type;
 	static const char NVDIMM_PREFIX[] = "nvdimm:";
 	char desc[NVDIMM_KEY_DESC_LEN + sizeof(NVDIMM_PREFIX)];
 	struct device *dev = &nvdimm->dev;
 
 	sprintf(desc, "%s%s", NVDIMM_PREFIX, nvdimm->dimm_id);
-	key = request_key(&key_type_encrypted, desc, "");
+	type = key_type_lookup("encrypted");
+	if (IS_ERR(type))
+		return (struct key *) type;
+
+	key = request_key(type, desc, "");
 	if (IS_ERR(key)) {
 		if (PTR_ERR(key) == -ENOKEY)
 			dev_dbg(dev, "request_key() found no key\n");
@@ -88,7 +93,7 @@ static struct key *nvdimm_lookup_user_key(struct nvdimm *nvdimm,
 		return NULL;
 
 	key = key_ref_to_ptr(keyref);
-	if (key->type != &key_type_encrypted) {
+	if (strcmp(key->type->name, "encrypted") != 0) {
 		key_put(key);
 		return NULL;
 	}
@@ -452,3 +457,5 @@ void nvdimm_security_overwrite_query(struct work_struct *work)
 	__nvdimm_security_overwrite_query(nvdimm);
 	nvdimm_bus_unlock(&nvdimm->dev);
 }
+
+MODULE_SOFTDEP("pre: encrypted_keys");

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 3/6] libnvdimm/security: Drop direct dependency on key_type_encrypted
@ 2019-03-19  6:06   ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-19  6:06 UTC (permalink / raw)
  To: keyrings
  Cc: Vishal Verma, Dave Jiang, Keith Busch, Ira Weiny, linux-nvdimm,
	linux-kernel

Lookup the key type by name and protect libnvdimm from encrypted_keys.ko
module load failures.

Cc: Vishal Verma <vishal.l.verma@intel.com>
Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Keith Busch <keith.busch@intel.com>
Cc: Ira Weiny <ira.weiny@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/nvdimm/security.c |   11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c
index f8bb746a549f..7f9e412f743a 100644
--- a/drivers/nvdimm/security.c
+++ b/drivers/nvdimm/security.c
@@ -48,12 +48,17 @@ static void nvdimm_put_key(struct key *key)
 static struct key *nvdimm_request_key(struct nvdimm *nvdimm)
 {
 	struct key *key = NULL;
+	struct key_type *type;
 	static const char NVDIMM_PREFIX[] = "nvdimm:";
 	char desc[NVDIMM_KEY_DESC_LEN + sizeof(NVDIMM_PREFIX)];
 	struct device *dev = &nvdimm->dev;
 
 	sprintf(desc, "%s%s", NVDIMM_PREFIX, nvdimm->dimm_id);
-	key = request_key(&key_type_encrypted, desc, "");
+	type = key_type_lookup("encrypted");
+	if (IS_ERR(type))
+		return (struct key *) type;
+
+	key = request_key(type, desc, "");
 	if (IS_ERR(key)) {
 		if (PTR_ERR(key) == -ENOKEY)
 			dev_dbg(dev, "request_key() found no key\n");
@@ -88,7 +93,7 @@ static struct key *nvdimm_lookup_user_key(struct nvdimm *nvdimm,
 		return NULL;
 
 	key = key_ref_to_ptr(keyref);
-	if (key->type != &key_type_encrypted) {
+	if (strcmp(key->type->name, "encrypted") != 0) {
 		key_put(key);
 		return NULL;
 	}
@@ -452,3 +457,5 @@ void nvdimm_security_overwrite_query(struct work_struct *work)
 	__nvdimm_security_overwrite_query(nvdimm);
 	nvdimm_bus_unlock(&nvdimm->dev);
 }
+
+MODULE_SOFTDEP("pre: encrypted_keys");


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 3/6] libnvdimm/security: Drop direct dependency on key_type_encrypted
@ 2019-03-19  6:06   ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-19  6:06 UTC (permalink / raw)
  To: keyrings; +Cc: linux-nvdimm, linux-kernel

Lookup the key type by name and protect libnvdimm from encrypted_keys.ko
module load failures.

Cc: Vishal Verma <vishal.l.verma@intel.com>
Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Keith Busch <keith.busch@intel.com>
Cc: Ira Weiny <ira.weiny@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/nvdimm/security.c |   11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c
index f8bb746a549f..7f9e412f743a 100644
--- a/drivers/nvdimm/security.c
+++ b/drivers/nvdimm/security.c
@@ -48,12 +48,17 @@ static void nvdimm_put_key(struct key *key)
 static struct key *nvdimm_request_key(struct nvdimm *nvdimm)
 {
 	struct key *key = NULL;
+	struct key_type *type;
 	static const char NVDIMM_PREFIX[] = "nvdimm:";
 	char desc[NVDIMM_KEY_DESC_LEN + sizeof(NVDIMM_PREFIX)];
 	struct device *dev = &nvdimm->dev;
 
 	sprintf(desc, "%s%s", NVDIMM_PREFIX, nvdimm->dimm_id);
-	key = request_key(&key_type_encrypted, desc, "");
+	type = key_type_lookup("encrypted");
+	if (IS_ERR(type))
+		return (struct key *) type;
+
+	key = request_key(type, desc, "");
 	if (IS_ERR(key)) {
 		if (PTR_ERR(key) = -ENOKEY)
 			dev_dbg(dev, "request_key() found no key\n");
@@ -88,7 +93,7 @@ static struct key *nvdimm_lookup_user_key(struct nvdimm *nvdimm,
 		return NULL;
 
 	key = key_ref_to_ptr(keyref);
-	if (key->type != &key_type_encrypted) {
+	if (strcmp(key->type->name, "encrypted") != 0) {
 		key_put(key);
 		return NULL;
 	}
@@ -452,3 +457,5 @@ void nvdimm_security_overwrite_query(struct work_struct *work)
 	__nvdimm_security_overwrite_query(nvdimm);
 	nvdimm_bus_unlock(&nvdimm->dev);
 }
+
+MODULE_SOFTDEP("pre: encrypted_keys");

^ permalink raw reply related	[flat|nested] 70+ messages in thread

* Re: [PATCH 3/6] libnvdimm/security: Drop direct dependency on key_type_encrypted
  2019-03-19  6:06   ` Dan Williams
  (?)
@ 2019-03-19 16:46     ` Dave Jiang
  -1 siblings, 0 replies; 70+ messages in thread
From: Dave Jiang @ 2019-03-19 16:46 UTC (permalink / raw)
  To: Dan Williams, keyrings; +Cc: linux-kernel, linux-nvdimm



On 3/18/19 11:06 PM, Dan Williams wrote:
> Lookup the key type by name and protect libnvdimm from encrypted_keys.ko
> module load failures.
> 
> Cc: Vishal Verma <vishal.l.verma@intel.com>
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Keith Busch <keith.busch@intel.com>
> Cc: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/nvdimm/security.c |   11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c
> index f8bb746a549f..7f9e412f743a 100644
> --- a/drivers/nvdimm/security.c
> +++ b/drivers/nvdimm/security.c
> @@ -48,12 +48,17 @@ static void nvdimm_put_key(struct key *key)
>  static struct key *nvdimm_request_key(struct nvdimm *nvdimm)
>  {
>  	struct key *key = NULL;
> +	struct key_type *type;
>  	static const char NVDIMM_PREFIX[] = "nvdimm:";
>  	char desc[NVDIMM_KEY_DESC_LEN + sizeof(NVDIMM_PREFIX)];
>  	struct device *dev = &nvdimm->dev;
>  
>  	sprintf(desc, "%s%s", NVDIMM_PREFIX, nvdimm->dimm_id);
> -	key = request_key(&key_type_encrypted, desc, "");
> +	type = key_type_lookup("encrypted");
> +	if (IS_ERR(type))
> +		return (struct key *) type;
> +
> +	key = request_key(type, desc, "");
>  	if (IS_ERR(key)) {
>  		if (PTR_ERR(key) == -ENOKEY)
>  			dev_dbg(dev, "request_key() found no key\n");
> @@ -88,7 +93,7 @@ static struct key *nvdimm_lookup_user_key(struct nvdimm *nvdimm,
>  		return NULL;
>  
>  	key = key_ref_to_ptr(keyref);
> -	if (key->type != &key_type_encrypted) {
> +	if (strcmp(key->type->name, "encrypted") != 0) {
>  		key_put(key);
>  		return NULL;
>  	}
> @@ -452,3 +457,5 @@ void nvdimm_security_overwrite_query(struct work_struct *work)
>  	__nvdimm_security_overwrite_query(nvdimm);
>  	nvdimm_bus_unlock(&nvdimm->dev);
>  }
> +
> +MODULE_SOFTDEP("pre: encrypted_keys");
> 
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 3/6] libnvdimm/security: Drop direct dependency on key_type_encrypted
@ 2019-03-19 16:46     ` Dave Jiang
  0 siblings, 0 replies; 70+ messages in thread
From: Dave Jiang @ 2019-03-19 16:46 UTC (permalink / raw)
  To: Dan Williams, keyrings
  Cc: Vishal Verma, Keith Busch, Ira Weiny, linux-nvdimm, linux-kernel



On 3/18/19 11:06 PM, Dan Williams wrote:
> Lookup the key type by name and protect libnvdimm from encrypted_keys.ko
> module load failures.
> 
> Cc: Vishal Verma <vishal.l.verma@intel.com>
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Keith Busch <keith.busch@intel.com>
> Cc: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/nvdimm/security.c |   11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c
> index f8bb746a549f..7f9e412f743a 100644
> --- a/drivers/nvdimm/security.c
> +++ b/drivers/nvdimm/security.c
> @@ -48,12 +48,17 @@ static void nvdimm_put_key(struct key *key)
>  static struct key *nvdimm_request_key(struct nvdimm *nvdimm)
>  {
>  	struct key *key = NULL;
> +	struct key_type *type;
>  	static const char NVDIMM_PREFIX[] = "nvdimm:";
>  	char desc[NVDIMM_KEY_DESC_LEN + sizeof(NVDIMM_PREFIX)];
>  	struct device *dev = &nvdimm->dev;
>  
>  	sprintf(desc, "%s%s", NVDIMM_PREFIX, nvdimm->dimm_id);
> -	key = request_key(&key_type_encrypted, desc, "");
> +	type = key_type_lookup("encrypted");
> +	if (IS_ERR(type))
> +		return (struct key *) type;
> +
> +	key = request_key(type, desc, "");
>  	if (IS_ERR(key)) {
>  		if (PTR_ERR(key) == -ENOKEY)
>  			dev_dbg(dev, "request_key() found no key\n");
> @@ -88,7 +93,7 @@ static struct key *nvdimm_lookup_user_key(struct nvdimm *nvdimm,
>  		return NULL;
>  
>  	key = key_ref_to_ptr(keyref);
> -	if (key->type != &key_type_encrypted) {
> +	if (strcmp(key->type->name, "encrypted") != 0) {
>  		key_put(key);
>  		return NULL;
>  	}
> @@ -452,3 +457,5 @@ void nvdimm_security_overwrite_query(struct work_struct *work)
>  	__nvdimm_security_overwrite_query(nvdimm);
>  	nvdimm_bus_unlock(&nvdimm->dev);
>  }
> +
> +MODULE_SOFTDEP("pre: encrypted_keys");
> 

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 3/6] libnvdimm/security: Drop direct dependency on key_type_encrypted
@ 2019-03-19 16:46     ` Dave Jiang
  0 siblings, 0 replies; 70+ messages in thread
From: Dave Jiang @ 2019-03-19 16:46 UTC (permalink / raw)
  To: Dan Williams, keyrings; +Cc: linux-kernel, linux-nvdimm



On 3/18/19 11:06 PM, Dan Williams wrote:
> Lookup the key type by name and protect libnvdimm from encrypted_keys.ko
> module load failures.
> 
> Cc: Vishal Verma <vishal.l.verma@intel.com>
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Keith Busch <keith.busch@intel.com>
> Cc: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/nvdimm/security.c |   11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c
> index f8bb746a549f..7f9e412f743a 100644
> --- a/drivers/nvdimm/security.c
> +++ b/drivers/nvdimm/security.c
> @@ -48,12 +48,17 @@ static void nvdimm_put_key(struct key *key)
>  static struct key *nvdimm_request_key(struct nvdimm *nvdimm)
>  {
>  	struct key *key = NULL;
> +	struct key_type *type;
>  	static const char NVDIMM_PREFIX[] = "nvdimm:";
>  	char desc[NVDIMM_KEY_DESC_LEN + sizeof(NVDIMM_PREFIX)];
>  	struct device *dev = &nvdimm->dev;
>  
>  	sprintf(desc, "%s%s", NVDIMM_PREFIX, nvdimm->dimm_id);
> -	key = request_key(&key_type_encrypted, desc, "");
> +	type = key_type_lookup("encrypted");
> +	if (IS_ERR(type))
> +		return (struct key *) type;
> +
> +	key = request_key(type, desc, "");
>  	if (IS_ERR(key)) {
>  		if (PTR_ERR(key) = -ENOKEY)
>  			dev_dbg(dev, "request_key() found no key\n");
> @@ -88,7 +93,7 @@ static struct key *nvdimm_lookup_user_key(struct nvdimm *nvdimm,
>  		return NULL;
>  
>  	key = key_ref_to_ptr(keyref);
> -	if (key->type != &key_type_encrypted) {
> +	if (strcmp(key->type->name, "encrypted") != 0) {
>  		key_put(key);
>  		return NULL;
>  	}
> @@ -452,3 +457,5 @@ void nvdimm_security_overwrite_query(struct work_struct *work)
>  	__nvdimm_security_overwrite_query(nvdimm);
>  	nvdimm_bus_unlock(&nvdimm->dev);
>  }
> +
> +MODULE_SOFTDEP("pre: encrypted_keys");
> 

^ permalink raw reply	[flat|nested] 70+ messages in thread

* [PATCH 4/6] security/keys/ecryptfs: Drop direct dependency on key_type_encrypted
  2019-03-19  6:06 ` Dan Williams
  (?)
  (?)
@ 2019-03-19  6:06   ` Dan Williams
  -1 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-19  6:06 UTC (permalink / raw)
  To: keyrings; +Cc: linux-nvdimm, ecryptfs, linux-kernel, Tyler Hicks

Lookup the key type by name and protect ecryptfs from encrypted_keys.ko
module load failures, and cleanup the configuration dependencies on the
definition of the ecryptfs_get_encrypted_key() helper.

Cc: Tyler Hicks <tyhicks@canonical.com>
Cc: <ecryptfs@vger.kernel.org>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 fs/ecryptfs/ecryptfs_kernel.h |   22 +---------------------
 fs/ecryptfs/keystore.c        |   12 ++++++++++++
 2 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
index e74cb2a0b299..3106d23d95f0 100644
--- a/fs/ecryptfs/ecryptfs_kernel.h
+++ b/fs/ecryptfs/ecryptfs_kernel.h
@@ -87,13 +87,12 @@ struct ecryptfs_page_crypt_context {
 	} param;
 };
 
-#if defined(CONFIG_ENCRYPTED_KEYS) || defined(CONFIG_ENCRYPTED_KEYS_MODULE)
 static inline struct ecryptfs_auth_tok *
 ecryptfs_get_encrypted_key_payload_data(struct key *key)
 {
 	struct encrypted_key_payload *payload;
 
-	if (key->type != &key_type_encrypted)
+	if (strcmp(key->type->name, "encrypted") != 0)
 		return NULL;
 
 	payload = key->payload.data[0];
@@ -103,25 +102,6 @@ ecryptfs_get_encrypted_key_payload_data(struct key *key)
 	return (struct ecryptfs_auth_tok *)payload->payload_data;
 }
 
-static inline struct key *ecryptfs_get_encrypted_key(char *sig)
-{
-	return request_key(&key_type_encrypted, sig, NULL);
-}
-
-#else
-static inline struct ecryptfs_auth_tok *
-ecryptfs_get_encrypted_key_payload_data(struct key *key)
-{
-	return NULL;
-}
-
-static inline struct key *ecryptfs_get_encrypted_key(char *sig)
-{
-	return ERR_PTR(-ENOKEY);
-}
-
-#endif /* CONFIG_ENCRYPTED_KEYS */
-
 static inline struct ecryptfs_auth_tok *
 ecryptfs_get_key_payload_data(struct key *key)
 {
diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c
index e74fe84d0886..52a01dd57f4a 100644
--- a/fs/ecryptfs/keystore.c
+++ b/fs/ecryptfs/keystore.c
@@ -1619,6 +1619,17 @@ parse_tag_11_packet(unsigned char *data, unsigned char *contents,
 	return rc;
 }
 
+static struct key *ecryptfs_get_encrypted_key(char *sig)
+{
+	struct key_type *type;
+	struct key *key;
+
+	type = key_type_lookup("encrypted");
+	if (IS_ERR(type))
+		return (struct key *) type;
+	return request_key(type, sig, NULL);
+}
+
 int ecryptfs_keyring_auth_tok_for_sig(struct key **auth_tok_key,
 				      struct ecryptfs_auth_tok **auth_tok,
 				      char *sig)
@@ -2542,3 +2553,4 @@ ecryptfs_add_global_auth_tok(struct ecryptfs_mount_crypt_stat *mount_crypt_stat,
 	return 0;
 }
 
+MODULE_SOFTDEP("pre: encrypted_keys");

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 4/6] security/keys/ecryptfs: Drop direct dependency on key_type_encrypted
@ 2019-03-19  6:06   ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-19  6:06 UTC (permalink / raw)
  To: keyrings-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
	ecryptfs-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Tyler Hicks

Lookup the key type by name and protect ecryptfs from encrypted_keys.ko
module load failures, and cleanup the configuration dependencies on the
definition of the ecryptfs_get_encrypted_key() helper.

Cc: Tyler Hicks <tyhicks-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Cc: <ecryptfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Signed-off-by: Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 fs/ecryptfs/ecryptfs_kernel.h |   22 +---------------------
 fs/ecryptfs/keystore.c        |   12 ++++++++++++
 2 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
index e74cb2a0b299..3106d23d95f0 100644
--- a/fs/ecryptfs/ecryptfs_kernel.h
+++ b/fs/ecryptfs/ecryptfs_kernel.h
@@ -87,13 +87,12 @@ struct ecryptfs_page_crypt_context {
 	} param;
 };
 
-#if defined(CONFIG_ENCRYPTED_KEYS) || defined(CONFIG_ENCRYPTED_KEYS_MODULE)
 static inline struct ecryptfs_auth_tok *
 ecryptfs_get_encrypted_key_payload_data(struct key *key)
 {
 	struct encrypted_key_payload *payload;
 
-	if (key->type != &key_type_encrypted)
+	if (strcmp(key->type->name, "encrypted") != 0)
 		return NULL;
 
 	payload = key->payload.data[0];
@@ -103,25 +102,6 @@ ecryptfs_get_encrypted_key_payload_data(struct key *key)
 	return (struct ecryptfs_auth_tok *)payload->payload_data;
 }
 
-static inline struct key *ecryptfs_get_encrypted_key(char *sig)
-{
-	return request_key(&key_type_encrypted, sig, NULL);
-}
-
-#else
-static inline struct ecryptfs_auth_tok *
-ecryptfs_get_encrypted_key_payload_data(struct key *key)
-{
-	return NULL;
-}
-
-static inline struct key *ecryptfs_get_encrypted_key(char *sig)
-{
-	return ERR_PTR(-ENOKEY);
-}
-
-#endif /* CONFIG_ENCRYPTED_KEYS */
-
 static inline struct ecryptfs_auth_tok *
 ecryptfs_get_key_payload_data(struct key *key)
 {
diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c
index e74fe84d0886..52a01dd57f4a 100644
--- a/fs/ecryptfs/keystore.c
+++ b/fs/ecryptfs/keystore.c
@@ -1619,6 +1619,17 @@ parse_tag_11_packet(unsigned char *data, unsigned char *contents,
 	return rc;
 }
 
+static struct key *ecryptfs_get_encrypted_key(char *sig)
+{
+	struct key_type *type;
+	struct key *key;
+
+	type = key_type_lookup("encrypted");
+	if (IS_ERR(type))
+		return (struct key *) type;
+	return request_key(type, sig, NULL);
+}
+
 int ecryptfs_keyring_auth_tok_for_sig(struct key **auth_tok_key,
 				      struct ecryptfs_auth_tok **auth_tok,
 				      char *sig)
@@ -2542,3 +2553,4 @@ ecryptfs_add_global_auth_tok(struct ecryptfs_mount_crypt_stat *mount_crypt_stat,
 	return 0;
 }
 
+MODULE_SOFTDEP("pre: encrypted_keys");

^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 4/6] security/keys/ecryptfs: Drop direct dependency on key_type_encrypted
@ 2019-03-19  6:06   ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-19  6:06 UTC (permalink / raw)
  To: keyrings
  Cc: Tyler Hicks, ecryptfs, vishal.l.verma, linux-nvdimm, linux-kernel

Lookup the key type by name and protect ecryptfs from encrypted_keys.ko
module load failures, and cleanup the configuration dependencies on the
definition of the ecryptfs_get_encrypted_key() helper.

Cc: Tyler Hicks <tyhicks@canonical.com>
Cc: <ecryptfs@vger.kernel.org>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 fs/ecryptfs/ecryptfs_kernel.h |   22 +---------------------
 fs/ecryptfs/keystore.c        |   12 ++++++++++++
 2 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
index e74cb2a0b299..3106d23d95f0 100644
--- a/fs/ecryptfs/ecryptfs_kernel.h
+++ b/fs/ecryptfs/ecryptfs_kernel.h
@@ -87,13 +87,12 @@ struct ecryptfs_page_crypt_context {
 	} param;
 };
 
-#if defined(CONFIG_ENCRYPTED_KEYS) || defined(CONFIG_ENCRYPTED_KEYS_MODULE)
 static inline struct ecryptfs_auth_tok *
 ecryptfs_get_encrypted_key_payload_data(struct key *key)
 {
 	struct encrypted_key_payload *payload;
 
-	if (key->type != &key_type_encrypted)
+	if (strcmp(key->type->name, "encrypted") != 0)
 		return NULL;
 
 	payload = key->payload.data[0];
@@ -103,25 +102,6 @@ ecryptfs_get_encrypted_key_payload_data(struct key *key)
 	return (struct ecryptfs_auth_tok *)payload->payload_data;
 }
 
-static inline struct key *ecryptfs_get_encrypted_key(char *sig)
-{
-	return request_key(&key_type_encrypted, sig, NULL);
-}
-
-#else
-static inline struct ecryptfs_auth_tok *
-ecryptfs_get_encrypted_key_payload_data(struct key *key)
-{
-	return NULL;
-}
-
-static inline struct key *ecryptfs_get_encrypted_key(char *sig)
-{
-	return ERR_PTR(-ENOKEY);
-}
-
-#endif /* CONFIG_ENCRYPTED_KEYS */
-
 static inline struct ecryptfs_auth_tok *
 ecryptfs_get_key_payload_data(struct key *key)
 {
diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c
index e74fe84d0886..52a01dd57f4a 100644
--- a/fs/ecryptfs/keystore.c
+++ b/fs/ecryptfs/keystore.c
@@ -1619,6 +1619,17 @@ parse_tag_11_packet(unsigned char *data, unsigned char *contents,
 	return rc;
 }
 
+static struct key *ecryptfs_get_encrypted_key(char *sig)
+{
+	struct key_type *type;
+	struct key *key;
+
+	type = key_type_lookup("encrypted");
+	if (IS_ERR(type))
+		return (struct key *) type;
+	return request_key(type, sig, NULL);
+}
+
 int ecryptfs_keyring_auth_tok_for_sig(struct key **auth_tok_key,
 				      struct ecryptfs_auth_tok **auth_tok,
 				      char *sig)
@@ -2542,3 +2553,4 @@ ecryptfs_add_global_auth_tok(struct ecryptfs_mount_crypt_stat *mount_crypt_stat,
 	return 0;
 }
 
+MODULE_SOFTDEP("pre: encrypted_keys");


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 4/6] security/keys/ecryptfs: Drop direct dependency on key_type_encrypted
@ 2019-03-19  6:06   ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-19  6:06 UTC (permalink / raw)
  To: keyrings; +Cc: linux-nvdimm, ecryptfs, linux-kernel, Tyler Hicks

Lookup the key type by name and protect ecryptfs from encrypted_keys.ko
module load failures, and cleanup the configuration dependencies on the
definition of the ecryptfs_get_encrypted_key() helper.

Cc: Tyler Hicks <tyhicks@canonical.com>
Cc: <ecryptfs@vger.kernel.org>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 fs/ecryptfs/ecryptfs_kernel.h |   22 +---------------------
 fs/ecryptfs/keystore.c        |   12 ++++++++++++
 2 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
index e74cb2a0b299..3106d23d95f0 100644
--- a/fs/ecryptfs/ecryptfs_kernel.h
+++ b/fs/ecryptfs/ecryptfs_kernel.h
@@ -87,13 +87,12 @@ struct ecryptfs_page_crypt_context {
 	} param;
 };
 
-#if defined(CONFIG_ENCRYPTED_KEYS) || defined(CONFIG_ENCRYPTED_KEYS_MODULE)
 static inline struct ecryptfs_auth_tok *
 ecryptfs_get_encrypted_key_payload_data(struct key *key)
 {
 	struct encrypted_key_payload *payload;
 
-	if (key->type != &key_type_encrypted)
+	if (strcmp(key->type->name, "encrypted") != 0)
 		return NULL;
 
 	payload = key->payload.data[0];
@@ -103,25 +102,6 @@ ecryptfs_get_encrypted_key_payload_data(struct key *key)
 	return (struct ecryptfs_auth_tok *)payload->payload_data;
 }
 
-static inline struct key *ecryptfs_get_encrypted_key(char *sig)
-{
-	return request_key(&key_type_encrypted, sig, NULL);
-}
-
-#else
-static inline struct ecryptfs_auth_tok *
-ecryptfs_get_encrypted_key_payload_data(struct key *key)
-{
-	return NULL;
-}
-
-static inline struct key *ecryptfs_get_encrypted_key(char *sig)
-{
-	return ERR_PTR(-ENOKEY);
-}
-
-#endif /* CONFIG_ENCRYPTED_KEYS */
-
 static inline struct ecryptfs_auth_tok *
 ecryptfs_get_key_payload_data(struct key *key)
 {
diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c
index e74fe84d0886..52a01dd57f4a 100644
--- a/fs/ecryptfs/keystore.c
+++ b/fs/ecryptfs/keystore.c
@@ -1619,6 +1619,17 @@ parse_tag_11_packet(unsigned char *data, unsigned char *contents,
 	return rc;
 }
 
+static struct key *ecryptfs_get_encrypted_key(char *sig)
+{
+	struct key_type *type;
+	struct key *key;
+
+	type = key_type_lookup("encrypted");
+	if (IS_ERR(type))
+		return (struct key *) type;
+	return request_key(type, sig, NULL);
+}
+
 int ecryptfs_keyring_auth_tok_for_sig(struct key **auth_tok_key,
 				      struct ecryptfs_auth_tok **auth_tok,
 				      char *sig)
@@ -2542,3 +2553,4 @@ ecryptfs_add_global_auth_tok(struct ecryptfs_mount_crypt_stat *mount_crypt_stat,
 	return 0;
 }
 
+MODULE_SOFTDEP("pre: encrypted_keys");

^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 5/6] security/integrity/evm: Drop direct dependency on key_type_encrypted
  2019-03-19  6:06 ` Dan Williams
  (?)
@ 2019-03-19  6:06   ` Dan Williams
  -1 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-19  6:06 UTC (permalink / raw)
  To: keyrings; +Cc: linux-integrity, linux-kernel, Mimi Zohar, linux-nvdimm

Lookup the key type by name and protect evm from encrypted_keys.ko
module load failures.

Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: <linux-integrity@vger.kernel.org>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 security/integrity/evm/evm_crypto.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index c37d08118af5..5c65c3aef427 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -354,10 +354,15 @@ int evm_init_hmac(struct inode *inode, const struct xattr *lsm_xattr,
 int evm_init_key(void)
 {
 	struct key *evm_key;
+	struct key_type *type;
 	struct encrypted_key_payload *ekp;
 	int rc;
 
-	evm_key = request_key(&key_type_encrypted, EVMKEY, NULL);
+	type = key_type_lookup("encrypted");
+	if (IS_ERR(type))
+		return PTR_ERR(type);
+
+	evm_key = request_key(type, EVMKEY, NULL);
 	if (IS_ERR(evm_key))
 		return -ENOENT;
 
@@ -372,3 +377,5 @@ int evm_init_key(void)
 	key_put(evm_key);
 	return rc;
 }
+
+MODULE_SOFTDEP("pre: encrypted_keys");

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 5/6] security/integrity/evm: Drop direct dependency on key_type_encrypted
@ 2019-03-19  6:06   ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-19  6:06 UTC (permalink / raw)
  To: keyrings
  Cc: Mimi Zohar, linux-integrity, vishal.l.verma, linux-nvdimm, linux-kernel

Lookup the key type by name and protect evm from encrypted_keys.ko
module load failures.

Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: <linux-integrity@vger.kernel.org>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 security/integrity/evm/evm_crypto.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index c37d08118af5..5c65c3aef427 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -354,10 +354,15 @@ int evm_init_hmac(struct inode *inode, const struct xattr *lsm_xattr,
 int evm_init_key(void)
 {
 	struct key *evm_key;
+	struct key_type *type;
 	struct encrypted_key_payload *ekp;
 	int rc;
 
-	evm_key = request_key(&key_type_encrypted, EVMKEY, NULL);
+	type = key_type_lookup("encrypted");
+	if (IS_ERR(type))
+		return PTR_ERR(type);
+
+	evm_key = request_key(type, EVMKEY, NULL);
 	if (IS_ERR(evm_key))
 		return -ENOENT;
 
@@ -372,3 +377,5 @@ int evm_init_key(void)
 	key_put(evm_key);
 	return rc;
 }
+
+MODULE_SOFTDEP("pre: encrypted_keys");


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 5/6] security/integrity/evm: Drop direct dependency on key_type_encrypted
@ 2019-03-19  6:06   ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-19  6:06 UTC (permalink / raw)
  To: keyrings
  Cc: Mimi Zohar, linux-integrity, vishal.l.verma, linux-nvdimm, linux-kernel

Lookup the key type by name and protect evm from encrypted_keys.ko
module load failures.

Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: <linux-integrity@vger.kernel.org>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 security/integrity/evm/evm_crypto.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index c37d08118af5..5c65c3aef427 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -354,10 +354,15 @@ int evm_init_hmac(struct inode *inode, const struct xattr *lsm_xattr,
 int evm_init_key(void)
 {
 	struct key *evm_key;
+	struct key_type *type;
 	struct encrypted_key_payload *ekp;
 	int rc;
 
-	evm_key = request_key(&key_type_encrypted, EVMKEY, NULL);
+	type = key_type_lookup("encrypted");
+	if (IS_ERR(type))
+		return PTR_ERR(type);
+
+	evm_key = request_key(type, EVMKEY, NULL);
 	if (IS_ERR(evm_key))
 		return -ENOENT;
 
@@ -372,3 +377,5 @@ int evm_init_key(void)
 	key_put(evm_key);
 	return rc;
 }
+
+MODULE_SOFTDEP("pre: encrypted_keys");

^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 6/6] security/keys/encrypted: Drop export of key_type_encrypted
  2019-03-19  6:06 ` Dan Williams
  (?)
@ 2019-03-19  6:06   ` Dan Williams
  -1 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-19  6:06 UTC (permalink / raw)
  To: keyrings; +Cc: David Howells, linux-kernel, Mimi Zohar, linux-nvdimm

Now that all users lookup the key type by name, drop the export and the
direct module dependency.

Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: David Howells <dhowells@redhat.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 include/keys/encrypted-type.h            |    2 --
 security/keys/encrypted-keys/encrypted.c |    3 +--
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/include/keys/encrypted-type.h b/include/keys/encrypted-type.h
index 1d4541370a64..dd509835b4a4 100644
--- a/include/keys/encrypted-type.h
+++ b/include/keys/encrypted-type.h
@@ -33,6 +33,4 @@ struct encrypted_key_payload {
 	u8 payload_data[0];	/* payload data + datablob + hmac */
 };
 
-extern struct key_type key_type_encrypted;
-
 #endif /* _KEYS_ENCRYPTED_TYPE_H */
diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index 06925d3b30c9..c3999d5e2a19 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -1012,7 +1012,7 @@ static void encrypted_destroy(struct key *key)
 	kzfree(key->payload.data[0]);
 }
 
-struct key_type key_type_encrypted = {
+static struct key_type key_type_encrypted = {
 	.name = "encrypted",
 	.instantiate = encrypted_instantiate,
 	.update = encrypted_update,
@@ -1020,7 +1020,6 @@ struct key_type key_type_encrypted = {
 	.describe = user_describe,
 	.read = encrypted_read,
 };
-EXPORT_SYMBOL_GPL(key_type_encrypted);
 
 static int __init init_encrypted(void)
 {

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 6/6] security/keys/encrypted: Drop export of key_type_encrypted
@ 2019-03-19  6:06   ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-19  6:06 UTC (permalink / raw)
  To: keyrings
  Cc: Mimi Zohar, David Howells, vishal.l.verma, linux-nvdimm, linux-kernel

Now that all users lookup the key type by name, drop the export and the
direct module dependency.

Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: David Howells <dhowells@redhat.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 include/keys/encrypted-type.h            |    2 --
 security/keys/encrypted-keys/encrypted.c |    3 +--
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/include/keys/encrypted-type.h b/include/keys/encrypted-type.h
index 1d4541370a64..dd509835b4a4 100644
--- a/include/keys/encrypted-type.h
+++ b/include/keys/encrypted-type.h
@@ -33,6 +33,4 @@ struct encrypted_key_payload {
 	u8 payload_data[0];	/* payload data + datablob + hmac */
 };
 
-extern struct key_type key_type_encrypted;
-
 #endif /* _KEYS_ENCRYPTED_TYPE_H */
diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index 06925d3b30c9..c3999d5e2a19 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -1012,7 +1012,7 @@ static void encrypted_destroy(struct key *key)
 	kzfree(key->payload.data[0]);
 }
 
-struct key_type key_type_encrypted = {
+static struct key_type key_type_encrypted = {
 	.name = "encrypted",
 	.instantiate = encrypted_instantiate,
 	.update = encrypted_update,
@@ -1020,7 +1020,6 @@ struct key_type key_type_encrypted = {
 	.describe = user_describe,
 	.read = encrypted_read,
 };
-EXPORT_SYMBOL_GPL(key_type_encrypted);
 
 static int __init init_encrypted(void)
 {


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 6/6] security/keys/encrypted: Drop export of key_type_encrypted
@ 2019-03-19  6:06   ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-19  6:06 UTC (permalink / raw)
  To: keyrings; +Cc: David Howells, linux-kernel, Mimi Zohar, linux-nvdimm

Now that all users lookup the key type by name, drop the export and the
direct module dependency.

Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: David Howells <dhowells@redhat.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 include/keys/encrypted-type.h            |    2 --
 security/keys/encrypted-keys/encrypted.c |    3 +--
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/include/keys/encrypted-type.h b/include/keys/encrypted-type.h
index 1d4541370a64..dd509835b4a4 100644
--- a/include/keys/encrypted-type.h
+++ b/include/keys/encrypted-type.h
@@ -33,6 +33,4 @@ struct encrypted_key_payload {
 	u8 payload_data[0];	/* payload data + datablob + hmac */
 };
 
-extern struct key_type key_type_encrypted;
-
 #endif /* _KEYS_ENCRYPTED_TYPE_H */
diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index 06925d3b30c9..c3999d5e2a19 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -1012,7 +1012,7 @@ static void encrypted_destroy(struct key *key)
 	kzfree(key->payload.data[0]);
 }
 
-struct key_type key_type_encrypted = {
+static struct key_type key_type_encrypted = {
 	.name = "encrypted",
 	.instantiate = encrypted_instantiate,
 	.update = encrypted_update,
@@ -1020,7 +1020,6 @@ struct key_type key_type_encrypted = {
 	.describe = user_describe,
 	.read = encrypted_read,
 };
-EXPORT_SYMBOL_GPL(key_type_encrypted);
 
 static int __init init_encrypted(void)
 {

^ permalink raw reply related	[flat|nested] 70+ messages in thread

* Re: [PATCH 0/6] security/keys/encrypted: Break module dependency chain
  2019-03-19  6:06 ` Dan Williams
@ 2019-03-19 21:01   ` Dan Williams
  -1 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-19 21:01 UTC (permalink / raw)
  To: keyrings
  Cc: Ira Weiny, Dave Jiang, Tyler Hicks, Keith Busch, Jarkko Sakkinen,
	David Howells, Vishal Verma, James Bottomley, Mimi Zohar,
	linux-integrity, ecryptfs, Roberto Sassu, linux-nvdimm,
	Linux Kernel Mailing List

On Mon, Mar 18, 2019 at 11:18 PM Dan Williams <dan.j.williams@intel.com> wrote:
>
> With v5.1-rc1 all the nvdimm sub-system regression tests started failing
> because the libnvdimm module failed to load in the qemu-kvm test
> environment.  Critically that environment does not have a TPM. Commit
> 240730437deb "KEYS: trusted: explicitly use tpm_chip structure..."
> started to require a TPM to be present for the trusted.ko module to load
> where there was no requirement for that before.
>
> Rather than undo the "fail if no hardware" behavior James points out
> that the module dependencies can be broken by looking up the key-type by
> name. Remove the dependencies on the "key_type_trusted" and
> "key_type_encrypted" symbol exports, and clean up other boilerplate that
> supported those exports in different configurations.

Any feedback? Was hoping to get at least patch1 in the queue for
v5.1-rc2 since this effectively disables the nvdimm driver on typical
configurations. Jarkko, would you be willing to merge it since the
regression came through your tree?

> Dan Williams (6):
>       security/keys/encrypted: Allow operation without trusted.ko
>       security/keys/encrypted: Clean up request_trusted_key()
>       libnvdimm/security: Drop direct dependency on key_type_encrypted
>       security/keys/ecryptfs: Drop direct dependency on key_type_encrypted
>       security/integrity/evm: Drop direct dependency on key_type_encrypted
>       security/keys/encrypted: Drop export of key_type_encrypted
>
>
>  drivers/nvdimm/security.c                        |   11 ++++-
>  fs/ecryptfs/ecryptfs_kernel.h                    |   22 -----------
>  fs/ecryptfs/keystore.c                           |   12 ++++++
>  include/keys/encrypted-type.h                    |    2 -
>  include/linux/key.h                              |    1
>  security/integrity/evm/evm_crypto.c              |    9 ++++
>  security/keys/encrypted-keys/Makefile            |    3 -
>  security/keys/encrypted-keys/encrypted.c         |   35 ++++++++++++++++-
>  security/keys/encrypted-keys/encrypted.h         |   12 ------
>  security/keys/encrypted-keys/masterkey_trusted.c |   46 ----------------------
>  security/keys/internal.h                         |    2 -
>  security/keys/key.c                              |    1
>  12 files changed, 65 insertions(+), 91 deletions(-)
>  delete mode 100644 security/keys/encrypted-keys/masterkey_trusted.c

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 0/6] security/keys/encrypted: Break module dependency chain
@ 2019-03-19 21:01   ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-19 21:01 UTC (permalink / raw)
  To: keyrings
  Cc: Ira Weiny, Dave Jiang, Tyler Hicks, Keith Busch, Jarkko Sakkinen,
	David Howells, Vishal Verma, James Bottomley, Mimi Zohar,
	linux-integrity, ecryptfs, Roberto Sassu, linux-nvdimm,
	Linux Kernel Mailing List

On Mon, Mar 18, 2019 at 11:18 PM Dan Williams <dan.j.williams@intel.com> wrote:
>
> With v5.1-rc1 all the nvdimm sub-system regression tests started failing
> because the libnvdimm module failed to load in the qemu-kvm test
> environment.  Critically that environment does not have a TPM. Commit
> 240730437deb "KEYS: trusted: explicitly use tpm_chip structure..."
> started to require a TPM to be present for the trusted.ko module to load
> where there was no requirement for that before.
>
> Rather than undo the "fail if no hardware" behavior James points out
> that the module dependencies can be broken by looking up the key-type by
> name. Remove the dependencies on the "key_type_trusted" and
> "key_type_encrypted" symbol exports, and clean up other boilerplate that
> supported those exports in different configurations.

Any feedback? Was hoping to get at least patch1 in the queue for
v5.1-rc2 since this effectively disables the nvdimm driver on typical
configurations. Jarkko, would you be willing to merge it since the
regression came through your tree?

> Dan Williams (6):
>       security/keys/encrypted: Allow operation without trusted.ko
>       security/keys/encrypted: Clean up request_trusted_key()
>       libnvdimm/security: Drop direct dependency on key_type_encrypted
>       security/keys/ecryptfs: Drop direct dependency on key_type_encrypted
>       security/integrity/evm: Drop direct dependency on key_type_encrypted
>       security/keys/encrypted: Drop export of key_type_encrypted
>
>
>  drivers/nvdimm/security.c                        |   11 ++++-
>  fs/ecryptfs/ecryptfs_kernel.h                    |   22 -----------
>  fs/ecryptfs/keystore.c                           |   12 ++++++
>  include/keys/encrypted-type.h                    |    2 -
>  include/linux/key.h                              |    1
>  security/integrity/evm/evm_crypto.c              |    9 ++++
>  security/keys/encrypted-keys/Makefile            |    3 -
>  security/keys/encrypted-keys/encrypted.c         |   35 ++++++++++++++++-
>  security/keys/encrypted-keys/encrypted.h         |   12 ------
>  security/keys/encrypted-keys/masterkey_trusted.c |   46 ----------------------
>  security/keys/internal.h                         |    2 -
>  security/keys/key.c                              |    1
>  12 files changed, 65 insertions(+), 91 deletions(-)
>  delete mode 100644 security/keys/encrypted-keys/masterkey_trusted.c

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 0/6] security/keys/encrypted: Break module dependency chain
  2019-03-19 21:01   ` Dan Williams
  (?)
@ 2019-03-19 21:08     ` James Bottomley
  -1 siblings, 0 replies; 70+ messages in thread
From: James Bottomley @ 2019-03-19 21:08 UTC (permalink / raw)
  To: Dan Williams, keyrings
  Cc: David Howells, linux-nvdimm, ecryptfs, Roberto Sassu,
	Linux Kernel Mailing List, Jarkko Sakkinen, Tyler Hicks,
	Mimi Zohar, linux-integrity

On Tue, 2019-03-19 at 14:01 -0700, Dan Williams wrote:
> On Mon, Mar 18, 2019 at 11:18 PM Dan Williams <dan.j.williams@intel.c
> om> wrote:
> > 
> > With v5.1-rc1 all the nvdimm sub-system regression tests started
> > failing because the libnvdimm module failed to load in the qemu-kvm 
> > test environment.  Critically that environment does not have a TPM.
> > Commit 240730437deb "KEYS: trusted: explicitly use tpm_chip
> > structure..." started to require a TPM to be present for the
> > trusted.ko module to load where there was no requirement for that
> > before.
> > 
> > Rather than undo the "fail if no hardware" behavior James points
> > out that the module dependencies can be broken by looking up the
> > key-type by name. Remove the dependencies on the "key_type_trusted"
> > and "key_type_encrypted" symbol exports, and clean up other
> > boilerplate that supported those exports in different
> > configurations.
> 
> Any feedback? Was hoping to get at least patch1 in the queue for
> v5.1-rc2 since this effectively disables the nvdimm driver on typical
> configurations. Jarkko, would you be willing to merge it since the
> regression came through your tree?

The reason I sent out the RFC was to see if the people who actually
wrote the code had any reasons they needed the current way of doing
things preserving.

I think your series looks fine except you need to export
key_type_lookup (patch 2) before you use it (patch 1) to preserve
bisectability of builds.

James

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 0/6] security/keys/encrypted: Break module dependency chain
@ 2019-03-19 21:08     ` James Bottomley
  0 siblings, 0 replies; 70+ messages in thread
From: James Bottomley @ 2019-03-19 21:08 UTC (permalink / raw)
  To: Dan Williams, keyrings
  Cc: Ira Weiny, Dave Jiang, Tyler Hicks, Keith Busch, Jarkko Sakkinen,
	David Howells, Vishal Verma, Mimi Zohar, linux-integrity,
	ecryptfs, Roberto Sassu, linux-nvdimm, Linux Kernel Mailing List

On Tue, 2019-03-19 at 14:01 -0700, Dan Williams wrote:
> On Mon, Mar 18, 2019 at 11:18 PM Dan Williams <dan.j.williams@intel.c
> om> wrote:
> > 
> > With v5.1-rc1 all the nvdimm sub-system regression tests started
> > failing because the libnvdimm module failed to load in the qemu-kvm 
> > test environment.  Critically that environment does not have a TPM.
> > Commit 240730437deb "KEYS: trusted: explicitly use tpm_chip
> > structure..." started to require a TPM to be present for the
> > trusted.ko module to load where there was no requirement for that
> > before.
> > 
> > Rather than undo the "fail if no hardware" behavior James points
> > out that the module dependencies can be broken by looking up the
> > key-type by name. Remove the dependencies on the "key_type_trusted"
> > and "key_type_encrypted" symbol exports, and clean up other
> > boilerplate that supported those exports in different
> > configurations.
> 
> Any feedback? Was hoping to get at least patch1 in the queue for
> v5.1-rc2 since this effectively disables the nvdimm driver on typical
> configurations. Jarkko, would you be willing to merge it since the
> regression came through your tree?

The reason I sent out the RFC was to see if the people who actually
wrote the code had any reasons they needed the current way of doing
things preserving.

I think your series looks fine except you need to export
key_type_lookup (patch 2) before you use it (patch 1) to preserve
bisectability of builds.

James


^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 0/6] security/keys/encrypted: Break module dependency chain
@ 2019-03-19 21:08     ` James Bottomley
  0 siblings, 0 replies; 70+ messages in thread
From: James Bottomley @ 2019-03-19 21:08 UTC (permalink / raw)
  To: Dan Williams, keyrings
  Cc: Ira Weiny, Dave Jiang, Tyler Hicks, Keith Busch, Jarkko Sakkinen,
	David Howells, Vishal Verma, Mimi Zohar, linux-integrity,
	ecryptfs, Roberto Sassu, linux-nvdimm, Linux Kernel Mailing List

On Tue, 2019-03-19 at 14:01 -0700, Dan Williams wrote:
> On Mon, Mar 18, 2019 at 11:18 PM Dan Williams <dan.j.williams@intel.c
> om> wrote:
> > 
> > With v5.1-rc1 all the nvdimm sub-system regression tests started
> > failing because the libnvdimm module failed to load in the qemu-kvm 
> > test environment.  Critically that environment does not have a TPM.
> > Commit 240730437deb "KEYS: trusted: explicitly use tpm_chip
> > structure..." started to require a TPM to be present for the
> > trusted.ko module to load where there was no requirement for that
> > before.
> > 
> > Rather than undo the "fail if no hardware" behavior James points
> > out that the module dependencies can be broken by looking up the
> > key-type by name. Remove the dependencies on the "key_type_trusted"
> > and "key_type_encrypted" symbol exports, and clean up other
> > boilerplate that supported those exports in different
> > configurations.
> 
> Any feedback? Was hoping to get at least patch1 in the queue for
> v5.1-rc2 since this effectively disables the nvdimm driver on typical
> configurations. Jarkko, would you be willing to merge it since the
> regression came through your tree?

The reason I sent out the RFC was to see if the people who actually
wrote the code had any reasons they needed the current way of doing
things preserving.

I think your series looks fine except you need to export
key_type_lookup (patch 2) before you use it (patch 1) to preserve
bisectability of builds.

James

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 0/6] security/keys/encrypted: Break module dependency chain
  2019-03-19 21:08     ` James Bottomley
  (?)
@ 2019-03-19 21:23       ` Dan Williams
  -1 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-19 21:23 UTC (permalink / raw)
  To: James Bottomley
  Cc: David Howells, linux-nvdimm, ecryptfs, Roberto Sassu,
	Linux Kernel Mailing List, Jarkko Sakkinen, Tyler Hicks,
	keyrings, Mimi Zohar, linux-integrity

On Tue, Mar 19, 2019 at 2:09 PM James Bottomley <jejb@linux.ibm.com> wrote:
>
> On Tue, 2019-03-19 at 14:01 -0700, Dan Williams wrote:
> > On Mon, Mar 18, 2019 at 11:18 PM Dan Williams <dan.j.williams@intel.c
> > om> wrote:
> > >
> > > With v5.1-rc1 all the nvdimm sub-system regression tests started
> > > failing because the libnvdimm module failed to load in the qemu-kvm
> > > test environment.  Critically that environment does not have a TPM.
> > > Commit 240730437deb "KEYS: trusted: explicitly use tpm_chip
> > > structure..." started to require a TPM to be present for the
> > > trusted.ko module to load where there was no requirement for that
> > > before.
> > >
> > > Rather than undo the "fail if no hardware" behavior James points
> > > out that the module dependencies can be broken by looking up the
> > > key-type by name. Remove the dependencies on the "key_type_trusted"
> > > and "key_type_encrypted" symbol exports, and clean up other
> > > boilerplate that supported those exports in different
> > > configurations.
> >
> > Any feedback? Was hoping to get at least patch1 in the queue for
> > v5.1-rc2 since this effectively disables the nvdimm driver on typical
> > configurations. Jarkko, would you be willing to merge it since the
> > regression came through your tree?
>
> The reason I sent out the RFC was to see if the people who actually
> wrote the code had any reasons they needed the current way of doing
> things preserving.
>
> I think your series looks fine except you need to export
> key_type_lookup (patch 2) before you use it (patch 1) to preserve
> bisectability of builds.

Good catch, I fumbled that around. I'll respin and let 0day chew on it
before sending out another version.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 0/6] security/keys/encrypted: Break module dependency chain
@ 2019-03-19 21:23       ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-19 21:23 UTC (permalink / raw)
  To: James Bottomley
  Cc: keyrings, Ira Weiny, Dave Jiang, Tyler Hicks, Keith Busch,
	Jarkko Sakkinen, David Howells, Vishal Verma, Mimi Zohar,
	linux-integrity, ecryptfs, Roberto Sassu, linux-nvdimm,
	Linux Kernel Mailing List

On Tue, Mar 19, 2019 at 2:09 PM James Bottomley <jejb@linux.ibm.com> wrote:
>
> On Tue, 2019-03-19 at 14:01 -0700, Dan Williams wrote:
> > On Mon, Mar 18, 2019 at 11:18 PM Dan Williams <dan.j.williams@intel.c
> > om> wrote:
> > >
> > > With v5.1-rc1 all the nvdimm sub-system regression tests started
> > > failing because the libnvdimm module failed to load in the qemu-kvm
> > > test environment.  Critically that environment does not have a TPM.
> > > Commit 240730437deb "KEYS: trusted: explicitly use tpm_chip
> > > structure..." started to require a TPM to be present for the
> > > trusted.ko module to load where there was no requirement for that
> > > before.
> > >
> > > Rather than undo the "fail if no hardware" behavior James points
> > > out that the module dependencies can be broken by looking up the
> > > key-type by name. Remove the dependencies on the "key_type_trusted"
> > > and "key_type_encrypted" symbol exports, and clean up other
> > > boilerplate that supported those exports in different
> > > configurations.
> >
> > Any feedback? Was hoping to get at least patch1 in the queue for
> > v5.1-rc2 since this effectively disables the nvdimm driver on typical
> > configurations. Jarkko, would you be willing to merge it since the
> > regression came through your tree?
>
> The reason I sent out the RFC was to see if the people who actually
> wrote the code had any reasons they needed the current way of doing
> things preserving.
>
> I think your series looks fine except you need to export
> key_type_lookup (patch 2) before you use it (patch 1) to preserve
> bisectability of builds.

Good catch, I fumbled that around. I'll respin and let 0day chew on it
before sending out another version.

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 0/6] security/keys/encrypted: Break module dependency chain
@ 2019-03-19 21:23       ` Dan Williams
  0 siblings, 0 replies; 70+ messages in thread
From: Dan Williams @ 2019-03-19 21:23 UTC (permalink / raw)
  To: James Bottomley
  Cc: keyrings, Ira Weiny, Dave Jiang, Tyler Hicks, Keith Busch,
	Jarkko Sakkinen, David Howells, Vishal Verma, Mimi Zohar,
	linux-integrity, ecryptfs, Roberto Sassu, linux-nvdimm,
	Linux Kernel Mailing List

On Tue, Mar 19, 2019 at 2:09 PM James Bottomley <jejb@linux.ibm.com> wrote:
>
> On Tue, 2019-03-19 at 14:01 -0700, Dan Williams wrote:
> > On Mon, Mar 18, 2019 at 11:18 PM Dan Williams <dan.j.williams@intel.c
> > om> wrote:
> > >
> > > With v5.1-rc1 all the nvdimm sub-system regression tests started
> > > failing because the libnvdimm module failed to load in the qemu-kvm
> > > test environment.  Critically that environment does not have a TPM.
> > > Commit 240730437deb "KEYS: trusted: explicitly use tpm_chip
> > > structure..." started to require a TPM to be present for the
> > > trusted.ko module to load where there was no requirement for that
> > > before.
> > >
> > > Rather than undo the "fail if no hardware" behavior James points
> > > out that the module dependencies can be broken by looking up the
> > > key-type by name. Remove the dependencies on the "key_type_trusted"
> > > and "key_type_encrypted" symbol exports, and clean up other
> > > boilerplate that supported those exports in different
> > > configurations.
> >
> > Any feedback? Was hoping to get at least patch1 in the queue for
> > v5.1-rc2 since this effectively disables the nvdimm driver on typical
> > configurations. Jarkko, would you be willing to merge it since the
> > regression came through your tree?
>
> The reason I sent out the RFC was to see if the people who actually
> wrote the code had any reasons they needed the current way of doing
> things preserving.
>
> I think your series looks fine except you need to export
> key_type_lookup (patch 2) before you use it (patch 1) to preserve
> bisectability of builds.

Good catch, I fumbled that around. I'll respin and let 0day chew on it
before sending out another version.

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 0/6] security/keys/encrypted: Break module dependency chain
  2019-03-19 21:08     ` James Bottomley
  (?)
  (?)
@ 2019-03-20  1:20       ` Mimi Zohar
  -1 siblings, 0 replies; 70+ messages in thread
From: Mimi Zohar @ 2019-03-20  1:20 UTC (permalink / raw)
  To: James Bottomley, Dan Williams, keyrings
  Cc: David Howells, linux-nvdimm, ecryptfs, Roberto Sassu,
	Linux Kernel Mailing List, Jarkko Sakkinen, Tyler Hicks,
	linux-integrity

On Tue, 2019-03-19 at 14:08 -0700, James Bottomley wrote:
> On Tue, 2019-03-19 at 14:01 -0700, Dan Williams wrote:
> > On Mon, Mar 18, 2019 at 11:18 PM Dan Williams <dan.j.williams@intel.c
> > om> wrote:
> > > 
> > > With v5.1-rc1 all the nvdimm sub-system regression tests started
> > > failing because the libnvdimm module failed to load in the qemu-kvm 
> > > test environment.  Critically that environment does not have a TPM.
> > > Commit 240730437deb "KEYS: trusted: explicitly use tpm_chip
> > > structure..." started to require a TPM to be present for the
> > > trusted.ko module to load where there was no requirement for that
> > > before.
> > > 
> > > Rather than undo the "fail if no hardware" behavior James points
> > > out that the module dependencies can be broken by looking up the
> > > key-type by name. Remove the dependencies on the "key_type_trusted"
> > > and "key_type_encrypted" symbol exports, and clean up other
> > > boilerplate that supported those exports in different
> > > configurations.
> > 
> > Any feedback? Was hoping to get at least patch1 in the queue for
> > v5.1-rc2 since this effectively disables the nvdimm driver on typical
> > configurations. Jarkko, would you be willing to merge it since the
> > regression came through your tree?
> 
> The reason I sent out the RFC was to see if the people who actually
> wrote the code had any reasons they needed the current way of doing
> things preserving.

No, it looks fine.  The error return codes will change, but I don't
think that is a problem.

Mimi

> 
> I think your series looks fine except you need to export
> key_type_lookup (patch 2) before you use it (patch 1) to preserve
> bisectability of builds.
> 
> James

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 0/6] security/keys/encrypted: Break module dependency chain
@ 2019-03-20  1:20       ` Mimi Zohar
  0 siblings, 0 replies; 70+ messages in thread
From: Mimi Zohar @ 2019-03-20  1:20 UTC (permalink / raw)
  To: James Bottomley, Dan Williams, keyrings
  Cc: Ira Weiny, Dave Jiang, Tyler Hicks, Keith Busch, Jarkko Sakkinen,
	David Howells, Vishal Verma, linux-integrity, ecryptfs,
	Roberto Sassu, linux-nvdimm, Linux Kernel Mailing List

On Tue, 2019-03-19 at 14:08 -0700, James Bottomley wrote:
> On Tue, 2019-03-19 at 14:01 -0700, Dan Williams wrote:
> > On Mon, Mar 18, 2019 at 11:18 PM Dan Williams <dan.j.williams@intel.c
> > om> wrote:
> > > 
> > > With v5.1-rc1 all the nvdimm sub-system regression tests started
> > > failing because the libnvdimm module failed to load in the qemu-kvm 
> > > test environment.  Critically that environment does not have a TPM.
> > > Commit 240730437deb "KEYS: trusted: explicitly use tpm_chip
> > > structure..." started to require a TPM to be present for the
> > > trusted.ko module to load where there was no requirement for that
> > > before.
> > > 
> > > Rather than undo the "fail if no hardware" behavior James points
> > > out that the module dependencies can be broken by looking up the
> > > key-type by name. Remove the dependencies on the "key_type_trusted"
> > > and "key_type_encrypted" symbol exports, and clean up other
> > > boilerplate that supported those exports in different
> > > configurations.
> > 
> > Any feedback? Was hoping to get at least patch1 in the queue for
> > v5.1-rc2 since this effectively disables the nvdimm driver on typical
> > configurations. Jarkko, would you be willing to merge it since the
> > regression came through your tree?
> 
> The reason I sent out the RFC was to see if the people who actually
> wrote the code had any reasons they needed the current way of doing
> things preserving.

No, it looks fine. Â The error return codes will change, but I don't
think that is a problem.

Mimi

> 
> I think your series looks fine except you need to export
> key_type_lookup (patch 2) before you use it (patch 1) to preserve
> bisectability of builds.
> 
> James


^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 0/6] security/keys/encrypted: Break module dependency chain
@ 2019-03-20  1:20       ` Mimi Zohar
  0 siblings, 0 replies; 70+ messages in thread
From: Mimi Zohar @ 2019-03-20  1:20 UTC (permalink / raw)
  To: James Bottomley, Dan Williams, keyrings
  Cc: Ira Weiny, Dave Jiang, Tyler Hicks, Keith Busch, Jarkko Sakkinen,
	David Howells, Vishal Verma, linux-integrity, ecryptfs,
	Roberto Sassu, linux-nvdimm, Linux Kernel Mailing List

On Tue, 2019-03-19 at 14:08 -0700, James Bottomley wrote:
> On Tue, 2019-03-19 at 14:01 -0700, Dan Williams wrote:
> > On Mon, Mar 18, 2019 at 11:18 PM Dan Williams <dan.j.williams@intel.c
> > om> wrote:
> > > 
> > > With v5.1-rc1 all the nvdimm sub-system regression tests started
> > > failing because the libnvdimm module failed to load in the qemu-kvm 
> > > test environment.  Critically that environment does not have a TPM.
> > > Commit 240730437deb "KEYS: trusted: explicitly use tpm_chip
> > > structure..." started to require a TPM to be present for the
> > > trusted.ko module to load where there was no requirement for that
> > > before.
> > > 
> > > Rather than undo the "fail if no hardware" behavior James points
> > > out that the module dependencies can be broken by looking up the
> > > key-type by name. Remove the dependencies on the "key_type_trusted"
> > > and "key_type_encrypted" symbol exports, and clean up other
> > > boilerplate that supported those exports in different
> > > configurations.
> > 
> > Any feedback? Was hoping to get at least patch1 in the queue for
> > v5.1-rc2 since this effectively disables the nvdimm driver on typical
> > configurations. Jarkko, would you be willing to merge it since the
> > regression came through your tree?
> 
> The reason I sent out the RFC was to see if the people who actually
> wrote the code had any reasons they needed the current way of doing
> things preserving.

No, it looks fine.  The error return codes will change, but I don't
think that is a problem.

Mimi

> 
> I think your series looks fine except you need to export
> key_type_lookup (patch 2) before you use it (patch 1) to preserve
> bisectability of builds.
> 
> James


^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 0/6] security/keys/encrypted: Break module dependency chain
@ 2019-03-20  1:20       ` Mimi Zohar
  0 siblings, 0 replies; 70+ messages in thread
From: Mimi Zohar @ 2019-03-20  1:20 UTC (permalink / raw)
  To: James Bottomley, Dan Williams, keyrings
  Cc: Ira Weiny, Dave Jiang, Tyler Hicks, Keith Busch, Jarkko Sakkinen,
	David Howells, Vishal Verma, linux-integrity, ecryptfs,
	Roberto Sassu, linux-nvdimm, Linux Kernel Mailing List

On Tue, 2019-03-19 at 14:08 -0700, James Bottomley wrote:
> On Tue, 2019-03-19 at 14:01 -0700, Dan Williams wrote:
> > On Mon, Mar 18, 2019 at 11:18 PM Dan Williams <dan.j.williams@intel.c
> > om> wrote:
> > > 
> > > With v5.1-rc1 all the nvdimm sub-system regression tests started
> > > failing because the libnvdimm module failed to load in the qemu-kvm 
> > > test environment.  Critically that environment does not have a TPM.
> > > Commit 240730437deb "KEYS: trusted: explicitly use tpm_chip
> > > structure..." started to require a TPM to be present for the
> > > trusted.ko module to load where there was no requirement for that
> > > before.
> > > 
> > > Rather than undo the "fail if no hardware" behavior James points
> > > out that the module dependencies can be broken by looking up the
> > > key-type by name. Remove the dependencies on the "key_type_trusted"
> > > and "key_type_encrypted" symbol exports, and clean up other
> > > boilerplate that supported those exports in different
> > > configurations.
> > 
> > Any feedback? Was hoping to get at least patch1 in the queue for
> > v5.1-rc2 since this effectively disables the nvdimm driver on typical
> > configurations. Jarkko, would you be willing to merge it since the
> > regression came through your tree?
> 
> The reason I sent out the RFC was to see if the people who actually
> wrote the code had any reasons they needed the current way of doing
> things preserving.

No, it looks fine. ┬áThe error return codes will change, but I don't
think that is a problem.

Mimi

> 
> I think your series looks fine except you need to export
> key_type_lookup (patch 2) before you use it (patch 1) to preserve
> bisectability of builds.
> 
> James

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 0/6] security/keys/encrypted: Break module dependency chain
  2019-03-19 21:01   ` Dan Williams
  (?)
@ 2019-03-21 13:45     ` Jarkko Sakkinen
  -1 siblings, 0 replies; 70+ messages in thread
From: Jarkko Sakkinen @ 2019-03-21 13:45 UTC (permalink / raw)
  To: Dan Williams
  Cc: David Howells, linux-nvdimm, James Bottomley, ecryptfs,
	Roberto Sassu, Linux Kernel Mailing List, Mimi Zohar,
	Tyler Hicks, keyrings, linux-integrity

On Tue, Mar 19, 2019 at 02:01:44PM -0700, Dan Williams wrote:
> On Mon, Mar 18, 2019 at 11:18 PM Dan Williams <dan.j.williams@intel.com> wrote:
> >
> > With v5.1-rc1 all the nvdimm sub-system regression tests started failing
> > because the libnvdimm module failed to load in the qemu-kvm test
> > environment.  Critically that environment does not have a TPM. Commit
> > 240730437deb "KEYS: trusted: explicitly use tpm_chip structure..."
> > started to require a TPM to be present for the trusted.ko module to load
> > where there was no requirement for that before.
> >
> > Rather than undo the "fail if no hardware" behavior James points out
> > that the module dependencies can be broken by looking up the key-type by
> > name. Remove the dependencies on the "key_type_trusted" and
> > "key_type_encrypted" symbol exports, and clean up other boilerplate that
> > supported those exports in different configurations.
> 
> Any feedback? Was hoping to get at least patch1 in the queue for
> v5.1-rc2 since this effectively disables the nvdimm driver on typical
> configurations. Jarkko, would you be willing to merge it since the
> regression came through your tree?

Yes, of course. The feedback has been extremely passive because I've
been sick leave for the early week :-)

Before I'm merging this I'm just thinking that would it be better
idea to merge a patch for trusted.c that reverts the old behavior
with cc to stable and fixes tags as I said in my earlier response.

It would less intrusive for stable kernels. Lets quickly sort out
the best strategy before merging.

/Jarkko
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 0/6] security/keys/encrypted: Break module dependency chain
@ 2019-03-21 13:45     ` Jarkko Sakkinen
  0 siblings, 0 replies; 70+ messages in thread
From: Jarkko Sakkinen @ 2019-03-21 13:45 UTC (permalink / raw)
  To: Dan Williams
  Cc: keyrings, Ira Weiny, Dave Jiang, Tyler Hicks, Keith Busch,
	David Howells, Vishal Verma, James Bottomley, Mimi Zohar,
	linux-integrity, ecryptfs, Roberto Sassu, linux-nvdimm,
	Linux Kernel Mailing List

On Tue, Mar 19, 2019 at 02:01:44PM -0700, Dan Williams wrote:
> On Mon, Mar 18, 2019 at 11:18 PM Dan Williams <dan.j.williams@intel.com> wrote:
> >
> > With v5.1-rc1 all the nvdimm sub-system regression tests started failing
> > because the libnvdimm module failed to load in the qemu-kvm test
> > environment.  Critically that environment does not have a TPM. Commit
> > 240730437deb "KEYS: trusted: explicitly use tpm_chip structure..."
> > started to require a TPM to be present for the trusted.ko module to load
> > where there was no requirement for that before.
> >
> > Rather than undo the "fail if no hardware" behavior James points out
> > that the module dependencies can be broken by looking up the key-type by
> > name. Remove the dependencies on the "key_type_trusted" and
> > "key_type_encrypted" symbol exports, and clean up other boilerplate that
> > supported those exports in different configurations.
> 
> Any feedback? Was hoping to get at least patch1 in the queue for
> v5.1-rc2 since this effectively disables the nvdimm driver on typical
> configurations. Jarkko, would you be willing to merge it since the
> regression came through your tree?

Yes, of course. The feedback has been extremely passive because I've
been sick leave for the early week :-)

Before I'm merging this I'm just thinking that would it be better
idea to merge a patch for trusted.c that reverts the old behavior
with cc to stable and fixes tags as I said in my earlier response.

It would less intrusive for stable kernels. Lets quickly sort out
the best strategy before merging.

/Jarkko

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 0/6] security/keys/encrypted: Break module dependency chain
@ 2019-03-21 13:45     ` Jarkko Sakkinen
  0 siblings, 0 replies; 70+ messages in thread
From: Jarkko Sakkinen @ 2019-03-21 13:45 UTC (permalink / raw)
  To: Dan Williams
  Cc: keyrings, Ira Weiny, Dave Jiang, Tyler Hicks, Keith Busch,
	David Howells, Vishal Verma, James Bottomley, Mimi Zohar,
	linux-integrity, ecryptfs, Roberto Sassu, linux-nvdimm,
	Linux Kernel Mailing List

On Tue, Mar 19, 2019 at 02:01:44PM -0700, Dan Williams wrote:
> On Mon, Mar 18, 2019 at 11:18 PM Dan Williams <dan.j.williams@intel.com> wrote:
> >
> > With v5.1-rc1 all the nvdimm sub-system regression tests started failing
> > because the libnvdimm module failed to load in the qemu-kvm test
> > environment.  Critically that environment does not have a TPM. Commit
> > 240730437deb "KEYS: trusted: explicitly use tpm_chip structure..."
> > started to require a TPM to be present for the trusted.ko module to load
> > where there was no requirement for that before.
> >
> > Rather than undo the "fail if no hardware" behavior James points out
> > that the module dependencies can be broken by looking up the key-type by
> > name. Remove the dependencies on the "key_type_trusted" and
> > "key_type_encrypted" symbol exports, and clean up other boilerplate that
> > supported those exports in different configurations.
> 
> Any feedback? Was hoping to get at least patch1 in the queue for
> v5.1-rc2 since this effectively disables the nvdimm driver on typical
> configurations. Jarkko, would you be willing to merge it since the
> regression came through your tree?

Yes, of course. The feedback has been extremely passive because I've
been sick leave for the early week :-)

Before I'm merging this I'm just thinking that would it be better
idea to merge a patch for trusted.c that reverts the old behavior
with cc to stable and fixes tags as I said in my earlier response.

It would less intrusive for stable kernels. Lets quickly sort out
the best strategy before merging.

/Jarkko

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 0/6] security/keys/encrypted: Break module dependency chain
  2019-03-21 13:45     ` Jarkko Sakkinen
  (?)
@ 2019-03-21 13:48       ` Jarkko Sakkinen
  -1 siblings, 0 replies; 70+ messages in thread
From: Jarkko Sakkinen @ 2019-03-21 13:48 UTC (permalink / raw)
  To: Dan Williams
  Cc: David Howells, linux-nvdimm, James Bottomley, ecryptfs,
	Roberto Sassu, Linux Kernel Mailing List, Mimi Zohar,
	Tyler Hicks, keyrings, linux-integrity

On Thu, Mar 21, 2019 at 03:45:49PM +0200, Jarkko Sakkinen wrote:
> On Tue, Mar 19, 2019 at 02:01:44PM -0700, Dan Williams wrote:
> > On Mon, Mar 18, 2019 at 11:18 PM Dan Williams <dan.j.williams@intel.com> wrote:
> > >
> > > With v5.1-rc1 all the nvdimm sub-system regression tests started failing
> > > because the libnvdimm module failed to load in the qemu-kvm test
> > > environment.  Critically that environment does not have a TPM. Commit
> > > 240730437deb "KEYS: trusted: explicitly use tpm_chip structure..."
> > > started to require a TPM to be present for the trusted.ko module to load
> > > where there was no requirement for that before.
> > >
> > > Rather than undo the "fail if no hardware" behavior James points out
> > > that the module dependencies can be broken by looking up the key-type by
> > > name. Remove the dependencies on the "key_type_trusted" and
> > > "key_type_encrypted" symbol exports, and clean up other boilerplate that
> > > supported those exports in different configurations.
> > 
> > Any feedback? Was hoping to get at least patch1 in the queue for
> > v5.1-rc2 since this effectively disables the nvdimm driver on typical
> > configurations. Jarkko, would you be willing to merge it since the
> > regression came through your tree?
> 
> Yes, of course. The feedback has been extremely passive because I've
> been sick leave for the early week :-)
> 
> Before I'm merging this I'm just thinking that would it be better
> idea to merge a patch for trusted.c that reverts the old behavior
> with cc to stable and fixes tags as I said in my earlier response.
> 
> It would less intrusive for stable kernels. Lets quickly sort out
> the best strategy before merging.

I.e. the way I see the situation:

1. Reverting the old behavior in the sense that missing TPM does
   not prevent init of trusted.ko should be done right now.
2. Your patch could be definitely merged but not as a bug fix.
3. At some point we could consider failing the init of trusted.ko
   if TPM is missing because that is kind of senseful anyway with
   better testing now that we understand the dependency context
   better.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 0/6] security/keys/encrypted: Break module dependency chain
@ 2019-03-21 13:48       ` Jarkko Sakkinen
  0 siblings, 0 replies; 70+ messages in thread
From: Jarkko Sakkinen @ 2019-03-21 13:48 UTC (permalink / raw)
  To: Dan Williams
  Cc: keyrings, Ira Weiny, Dave Jiang, Tyler Hicks, Keith Busch,
	David Howells, Vishal Verma, James Bottomley, Mimi Zohar,
	linux-integrity, ecryptfs, Roberto Sassu, linux-nvdimm,
	Linux Kernel Mailing List

On Thu, Mar 21, 2019 at 03:45:49PM +0200, Jarkko Sakkinen wrote:
> On Tue, Mar 19, 2019 at 02:01:44PM -0700, Dan Williams wrote:
> > On Mon, Mar 18, 2019 at 11:18 PM Dan Williams <dan.j.williams@intel.com> wrote:
> > >
> > > With v5.1-rc1 all the nvdimm sub-system regression tests started failing
> > > because the libnvdimm module failed to load in the qemu-kvm test
> > > environment.  Critically that environment does not have a TPM. Commit
> > > 240730437deb "KEYS: trusted: explicitly use tpm_chip structure..."
> > > started to require a TPM to be present for the trusted.ko module to load
> > > where there was no requirement for that before.
> > >
> > > Rather than undo the "fail if no hardware" behavior James points out
> > > that the module dependencies can be broken by looking up the key-type by
> > > name. Remove the dependencies on the "key_type_trusted" and
> > > "key_type_encrypted" symbol exports, and clean up other boilerplate that
> > > supported those exports in different configurations.
> > 
> > Any feedback? Was hoping to get at least patch1 in the queue for
> > v5.1-rc2 since this effectively disables the nvdimm driver on typical
> > configurations. Jarkko, would you be willing to merge it since the
> > regression came through your tree?
> 
> Yes, of course. The feedback has been extremely passive because I've
> been sick leave for the early week :-)
> 
> Before I'm merging this I'm just thinking that would it be better
> idea to merge a patch for trusted.c that reverts the old behavior
> with cc to stable and fixes tags as I said in my earlier response.
> 
> It would less intrusive for stable kernels. Lets quickly sort out
> the best strategy before merging.

I.e. the way I see the situation:

1. Reverting the old behavior in the sense that missing TPM does
   not prevent init of trusted.ko should be done right now.
2. Your patch could be definitely merged but not as a bug fix.
3. At some point we could consider failing the init of trusted.ko
   if TPM is missing because that is kind of senseful anyway with
   better testing now that we understand the dependency context
   better.

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 0/6] security/keys/encrypted: Break module dependency chain
@ 2019-03-21 13:48       ` Jarkko Sakkinen
  0 siblings, 0 replies; 70+ messages in thread
From: Jarkko Sakkinen @ 2019-03-21 13:48 UTC (permalink / raw)
  To: Dan Williams
  Cc: keyrings, Ira Weiny, Dave Jiang, Tyler Hicks, Keith Busch,
	David Howells, Vishal Verma, James Bottomley, Mimi Zohar,
	linux-integrity, ecryptfs, Roberto Sassu, linux-nvdimm,
	Linux Kernel Mailing List

On Thu, Mar 21, 2019 at 03:45:49PM +0200, Jarkko Sakkinen wrote:
> On Tue, Mar 19, 2019 at 02:01:44PM -0700, Dan Williams wrote:
> > On Mon, Mar 18, 2019 at 11:18 PM Dan Williams <dan.j.williams@intel.com> wrote:
> > >
> > > With v5.1-rc1 all the nvdimm sub-system regression tests started failing
> > > because the libnvdimm module failed to load in the qemu-kvm test
> > > environment.  Critically that environment does not have a TPM. Commit
> > > 240730437deb "KEYS: trusted: explicitly use tpm_chip structure..."
> > > started to require a TPM to be present for the trusted.ko module to load
> > > where there was no requirement for that before.
> > >
> > > Rather than undo the "fail if no hardware" behavior James points out
> > > that the module dependencies can be broken by looking up the key-type by
> > > name. Remove the dependencies on the "key_type_trusted" and
> > > "key_type_encrypted" symbol exports, and clean up other boilerplate that
> > > supported those exports in different configurations.
> > 
> > Any feedback? Was hoping to get at least patch1 in the queue for
> > v5.1-rc2 since this effectively disables the nvdimm driver on typical
> > configurations. Jarkko, would you be willing to merge it since the
> > regression came through your tree?
> 
> Yes, of course. The feedback has been extremely passive because I've
> been sick leave for the early week :-)
> 
> Before I'm merging this I'm just thinking that would it be better
> idea to merge a patch for trusted.c that reverts the old behavior
> with cc to stable and fixes tags as I said in my earlier response.
> 
> It would less intrusive for stable kernels. Lets quickly sort out
> the best strategy before merging.

I.e. the way I see the situation:

1. Reverting the old behavior in the sense that missing TPM does
   not prevent init of trusted.ko should be done right now.
2. Your patch could be definitely merged but not as a bug fix.
3. At some point we could consider failing the init of trusted.ko
   if TPM is missing because that is kind of senseful anyway with
   better testing now that we understand the dependency context
   better.

^ permalink raw reply	[flat|nested] 70+ messages in thread

end of thread, other threads:[~2019-03-21 13:48 UTC | newest]

Thread overview: 70+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-19  6:06 [PATCH 0/6] security/keys/encrypted: Break module dependency chain Dan Williams
2019-03-19  6:06 ` Dan Williams
2019-03-19  6:06 ` Dan Williams
2019-03-19  6:06 ` [PATCH 1/6] security/keys/encrypted: Allow operation without trusted.ko Dan Williams
2019-03-19  6:06   ` Dan Williams
2019-03-19  6:06   ` Dan Williams
2019-03-19  6:06 ` [PATCH 2/6] security/keys/encrypted: Clean up request_trusted_key() Dan Williams
2019-03-19  6:06   ` Dan Williams
2019-03-19  6:06   ` Dan Williams
2019-03-20  0:06   ` Mimi Zohar
2019-03-20  0:06     ` Mimi Zohar
2019-03-20  0:06     ` Mimi Zohar
2019-03-20  0:20     ` Dan Williams
2019-03-20  0:20       ` Dan Williams
2019-03-20  0:20       ` Dan Williams
2019-03-20  1:10       ` Mimi Zohar
2019-03-20  1:10         ` Mimi Zohar
2019-03-20  1:10         ` Mimi Zohar
2019-03-20  1:34         ` Dan Williams
2019-03-20  1:34           ` Dan Williams
2019-03-20  1:34           ` Dan Williams
2019-03-20  1:37           ` Dan Williams
2019-03-20  1:37             ` Dan Williams
2019-03-20  1:37             ` Dan Williams
2019-03-20  2:35       ` Mimi Zohar
2019-03-20  2:35         ` Mimi Zohar
2019-03-20  2:35         ` Mimi Zohar
2019-03-20  5:48         ` Dan Williams
2019-03-20  5:48           ` Dan Williams
2019-03-20  5:48           ` Dan Williams
2019-03-20 12:06           ` Mimi Zohar
2019-03-20 12:06             ` Mimi Zohar
2019-03-20 12:06             ` Mimi Zohar
2019-03-20 15:27             ` Dan Williams
2019-03-20 15:27               ` Dan Williams
2019-03-20 15:27               ` Dan Williams
2019-03-19  6:06 ` [PATCH 3/6] libnvdimm/security: Drop direct dependency on key_type_encrypted Dan Williams
2019-03-19  6:06   ` Dan Williams
2019-03-19  6:06   ` Dan Williams
2019-03-19 16:46   ` Dave Jiang
2019-03-19 16:46     ` Dave Jiang
2019-03-19 16:46     ` Dave Jiang
2019-03-19  6:06 ` [PATCH 4/6] security/keys/ecryptfs: " Dan Williams
2019-03-19  6:06   ` Dan Williams
2019-03-19  6:06   ` Dan Williams
2019-03-19  6:06   ` Dan Williams
2019-03-19  6:06 ` [PATCH 5/6] security/integrity/evm: " Dan Williams
2019-03-19  6:06   ` Dan Williams
2019-03-19  6:06   ` Dan Williams
2019-03-19  6:06 ` [PATCH 6/6] security/keys/encrypted: Drop export of key_type_encrypted Dan Williams
2019-03-19  6:06   ` Dan Williams
2019-03-19  6:06   ` Dan Williams
2019-03-19 21:01 ` [PATCH 0/6] security/keys/encrypted: Break module dependency chain Dan Williams
2019-03-19 21:01   ` Dan Williams
2019-03-19 21:08   ` James Bottomley
2019-03-19 21:08     ` James Bottomley
2019-03-19 21:08     ` James Bottomley
2019-03-19 21:23     ` Dan Williams
2019-03-19 21:23       ` Dan Williams
2019-03-19 21:23       ` Dan Williams
2019-03-20  1:20     ` Mimi Zohar
2019-03-20  1:20       ` Mimi Zohar
2019-03-20  1:20       ` Mimi Zohar
2019-03-20  1:20       ` Mimi Zohar
2019-03-21 13:45   ` Jarkko Sakkinen
2019-03-21 13:45     ` Jarkko Sakkinen
2019-03-21 13:45     ` Jarkko Sakkinen
2019-03-21 13:48     ` Jarkko Sakkinen
2019-03-21 13:48       ` Jarkko Sakkinen
2019-03-21 13:48       ` Jarkko Sakkinen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.