* [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-18 23:45 ` Dan Williams 0 siblings, 0 replies; 50+ messages in thread From: Dan Williams @ 2019-03-18 23:45 UTC (permalink / raw) To: jarkko.sakkinen Cc: linux-nvdimm, James Bottomley, Roberto Sassu, linux-kernel, Mimi Zohar, David Howells, keyrings Rather than fail initialization of the trusted.ko module, arrange for the module to load, but rely on trusted_instantiate() to fail trusted-key operations. Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...") Cc: Roberto Sassu <roberto.sassu@huawei.com> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> Cc: James Bottomley <jejb@linux.ibm.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/trusted.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/security/keys/trusted.c b/security/keys/trusted.c index bcc9c6ead7fd..d959597a688e 100644 --- a/security/keys/trusted.c +++ b/security/keys/trusted.c @@ -45,6 +45,13 @@ struct sdesc { static struct crypto_shash *hashalg; static struct crypto_shash *hmacalg; +static struct device *chip_dev(struct tpm_chip *chip) +{ + if (chip) + return &chip->dev; + return NULL; +} + static struct sdesc *init_sdesc(struct crypto_shash *alg) { struct sdesc *sdesc; @@ -1224,6 +1231,14 @@ static int __init init_digests(void) int ret; int i; + /* + * Hardware tpm operations are disabled, but allow the software + * module to initialize, and depend on trusted_instantiate() to + * fail any attempts to access the missing hardware. + */ + if (!chip) + return 0; + ret = tpm_get_random(chip, digest, TPM_MAX_DIGEST_SIZE); if (ret < 0) return ret; @@ -1246,8 +1261,6 @@ static int __init init_trusted(void) int ret; chip = tpm_default_chip(); - if (!chip) - return -ENOENT; ret = init_digests(); if (ret < 0) goto err_put; @@ -1263,13 +1276,13 @@ static int __init init_trusted(void) err_free: kfree(digests); err_put: - put_device(&chip->dev); + put_device(chip_dev(chip)); return ret; } static void __exit cleanup_trusted(void) { - put_device(&chip->dev); + put_device(chip_dev(chip)); kfree(digests); trusted_shash_release(); unregister_key_type(&key_type_trusted); _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm ^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-18 23:45 ` Dan Williams 0 siblings, 0 replies; 50+ messages in thread From: Dan Williams @ 2019-03-18 23:45 UTC (permalink / raw) To: jarkko.sakkinen Cc: Roberto Sassu, James Bottomley, Mimi Zohar, David Howells, keyrings, linux-nvdimm, linux-kernel Rather than fail initialization of the trusted.ko module, arrange for the module to load, but rely on trusted_instantiate() to fail trusted-key operations. Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...") Cc: Roberto Sassu <roberto.sassu@huawei.com> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> Cc: James Bottomley <jejb@linux.ibm.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/trusted.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/security/keys/trusted.c b/security/keys/trusted.c index bcc9c6ead7fd..d959597a688e 100644 --- a/security/keys/trusted.c +++ b/security/keys/trusted.c @@ -45,6 +45,13 @@ struct sdesc { static struct crypto_shash *hashalg; static struct crypto_shash *hmacalg; +static struct device *chip_dev(struct tpm_chip *chip) +{ + if (chip) + return &chip->dev; + return NULL; +} + static struct sdesc *init_sdesc(struct crypto_shash *alg) { struct sdesc *sdesc; @@ -1224,6 +1231,14 @@ static int __init init_digests(void) int ret; int i; + /* + * Hardware tpm operations are disabled, but allow the software + * module to initialize, and depend on trusted_instantiate() to + * fail any attempts to access the missing hardware. + */ + if (!chip) + return 0; + ret = tpm_get_random(chip, digest, TPM_MAX_DIGEST_SIZE); if (ret < 0) return ret; @@ -1246,8 +1261,6 @@ static int __init init_trusted(void) int ret; chip = tpm_default_chip(); - if (!chip) - return -ENOENT; ret = init_digests(); if (ret < 0) goto err_put; @@ -1263,13 +1276,13 @@ static int __init init_trusted(void) err_free: kfree(digests); err_put: - put_device(&chip->dev); + put_device(chip_dev(chip)); return ret; } static void __exit cleanup_trusted(void) { - put_device(&chip->dev); + put_device(chip_dev(chip)); kfree(digests); trusted_shash_release(); unregister_key_type(&key_type_trusted); ^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-18 23:45 ` Dan Williams 0 siblings, 0 replies; 50+ messages in thread From: Dan Williams @ 2019-03-18 23:45 UTC (permalink / raw) To: jarkko.sakkinen Cc: linux-nvdimm, James Bottomley, Roberto Sassu, linux-kernel, Mimi Zohar, David Howells, keyrings Rather than fail initialization of the trusted.ko module, arrange for the module to load, but rely on trusted_instantiate() to fail trusted-key operations. Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...") Cc: Roberto Sassu <roberto.sassu@huawei.com> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> Cc: James Bottomley <jejb@linux.ibm.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/trusted.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/security/keys/trusted.c b/security/keys/trusted.c index bcc9c6ead7fd..d959597a688e 100644 --- a/security/keys/trusted.c +++ b/security/keys/trusted.c @@ -45,6 +45,13 @@ struct sdesc { static struct crypto_shash *hashalg; static struct crypto_shash *hmacalg; +static struct device *chip_dev(struct tpm_chip *chip) +{ + if (chip) + return &chip->dev; + return NULL; +} + static struct sdesc *init_sdesc(struct crypto_shash *alg) { struct sdesc *sdesc; @@ -1224,6 +1231,14 @@ static int __init init_digests(void) int ret; int i; + /* + * Hardware tpm operations are disabled, but allow the software + * module to initialize, and depend on trusted_instantiate() to + * fail any attempts to access the missing hardware. + */ + if (!chip) + return 0; + ret = tpm_get_random(chip, digest, TPM_MAX_DIGEST_SIZE); if (ret < 0) return ret; @@ -1246,8 +1261,6 @@ static int __init init_trusted(void) int ret; chip = tpm_default_chip(); - if (!chip) - return -ENOENT; ret = init_digests(); if (ret < 0) goto err_put; @@ -1263,13 +1276,13 @@ static int __init init_trusted(void) err_free: kfree(digests); err_put: - put_device(&chip->dev); + put_device(chip_dev(chip)); return ret; } static void __exit cleanup_trusted(void) { - put_device(&chip->dev); + put_device(chip_dev(chip)); kfree(digests); trusted_shash_release(); unregister_key_type(&key_type_trusted); ^ permalink raw reply related [flat|nested] 50+ messages in thread
[parent not found: <155295271345.1945351.6465460744078693578.stgit-p8uTFz9XbKj2zm6wflaqv1nYeNYlB/vhral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM [not found] ` <155295271345.1945351.6465460744078693578.stgit-p8uTFz9XbKj2zm6wflaqv1nYeNYlB/vhral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2019-03-19 0:24 ` James Bottomley @ 2019-03-19 0:24 ` James Bottomley 0 siblings, 0 replies; 50+ messages in thread From: James Bottomley @ 2019-03-19 0:24 UTC (permalink / raw) To: Dan Williams, jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA Cc: linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Roberto Sassu, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Mimi Zohar, David Howells, keyrings-u79uwXL29TY76Z2rM5mHXA On Mon, 2019-03-18 at 16:45 -0700, Dan Williams wrote: > Rather than fail initialization of the trusted.ko module, arrange for > the module to load, but rely on trusted_instantiate() to fail > trusted-key operations. What actual problem is this fixing? To me it would seem like an enhancement to make the trusted module fail at load time if there's no TPM rather than waiting until first use to find out it can never work. Is there some piece of user code that depends on the successful insertion of trusted.ko? James ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-19 0:24 ` James Bottomley 0 siblings, 0 replies; 50+ messages in thread From: James Bottomley @ 2019-03-19 0:24 UTC (permalink / raw) To: Dan Williams, jarkko.sakkinen Cc: Roberto Sassu, Mimi Zohar, David Howells, keyrings, linux-nvdimm, linux-kernel On Mon, 2019-03-18 at 16:45 -0700, Dan Williams wrote: > Rather than fail initialization of the trusted.ko module, arrange for > the module to load, but rely on trusted_instantiate() to fail > trusted-key operations. What actual problem is this fixing? To me it would seem like an enhancement to make the trusted module fail at load time if there's no TPM rather than waiting until first use to find out it can never work. Is there some piece of user code that depends on the successful insertion of trusted.ko? James ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-19 0:24 ` James Bottomley 0 siblings, 0 replies; 50+ messages in thread From: James Bottomley @ 2019-03-19 0:24 UTC (permalink / raw) To: Dan Williams, jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA Cc: linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Roberto Sassu, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Mimi Zohar, David Howells, keyrings-u79uwXL29TY76Z2rM5mHXA On Mon, 2019-03-18 at 16:45 -0700, Dan Williams wrote: > Rather than fail initialization of the trusted.ko module, arrange for > the module to load, but rely on trusted_instantiate() to fail > trusted-key operations. What actual problem is this fixing? To me it would seem like an enhancement to make the trusted module fail at load time if there's no TPM rather than waiting until first use to find out it can never work. Is there some piece of user code that depends on the successful insertion of trusted.ko? James ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM 2019-03-19 0:24 ` James Bottomley (?) @ 2019-03-19 0:30 ` Dan Williams -1 siblings, 0 replies; 50+ messages in thread From: Dan Williams @ 2019-03-19 0:30 UTC (permalink / raw) To: James Bottomley Cc: linux-nvdimm, Roberto Sassu, Linux Kernel Mailing List, Mimi Zohar, David Howells, keyrings, Jarkko Sakkinen On Mon, Mar 18, 2019 at 5:24 PM James Bottomley <jejb@linux.ibm.com> wrote: > > On Mon, 2019-03-18 at 16:45 -0700, Dan Williams wrote: > > Rather than fail initialization of the trusted.ko module, arrange for > > the module to load, but rely on trusted_instantiate() to fail > > trusted-key operations. > > What actual problem is this fixing? To me it would seem like an > enhancement to make the trusted module fail at load time if there's no > TPM rather than waiting until first use to find out it can never work. > Is there some piece of user code that depends on the successful > insertion of trusted.ko? The module dependency chain relies on it. If that can be broken that would also be an acceptable fix. I found this through the following dependency chain: libnvdimm.ko -> encrypted_keys.ko -> trusted.ko. "key_type_trusted" is the symbol that encrypted_keys needs regardless of whether the tpm is present. _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-19 0:30 ` Dan Williams 0 siblings, 0 replies; 50+ messages in thread From: Dan Williams @ 2019-03-19 0:30 UTC (permalink / raw) To: James Bottomley Cc: Jarkko Sakkinen, linux-nvdimm, Roberto Sassu, Linux Kernel Mailing List, Mimi Zohar, David Howells, keyrings On Mon, Mar 18, 2019 at 5:24 PM James Bottomley <jejb@linux.ibm.com> wrote: > > On Mon, 2019-03-18 at 16:45 -0700, Dan Williams wrote: > > Rather than fail initialization of the trusted.ko module, arrange for > > the module to load, but rely on trusted_instantiate() to fail > > trusted-key operations. > > What actual problem is this fixing? To me it would seem like an > enhancement to make the trusted module fail at load time if there's no > TPM rather than waiting until first use to find out it can never work. > Is there some piece of user code that depends on the successful > insertion of trusted.ko? The module dependency chain relies on it. If that can be broken that would also be an acceptable fix. I found this through the following dependency chain: libnvdimm.ko -> encrypted_keys.ko -> trusted.ko. "key_type_trusted" is the symbol that encrypted_keys needs regardless of whether the tpm is present. ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-19 0:30 ` Dan Williams 0 siblings, 0 replies; 50+ messages in thread From: Dan Williams @ 2019-03-19 0:30 UTC (permalink / raw) To: James Bottomley Cc: linux-nvdimm, Roberto Sassu, Linux Kernel Mailing List, Mimi Zohar, David Howells, keyrings, Jarkko Sakkinen On Mon, Mar 18, 2019 at 5:24 PM James Bottomley <jejb@linux.ibm.com> wrote: > > On Mon, 2019-03-18 at 16:45 -0700, Dan Williams wrote: > > Rather than fail initialization of the trusted.ko module, arrange for > > the module to load, but rely on trusted_instantiate() to fail > > trusted-key operations. > > What actual problem is this fixing? To me it would seem like an > enhancement to make the trusted module fail at load time if there's no > TPM rather than waiting until first use to find out it can never work. > Is there some piece of user code that depends on the successful > insertion of trusted.ko? The module dependency chain relies on it. If that can be broken that would also be an acceptable fix. I found this through the following dependency chain: libnvdimm.ko -> encrypted_keys.ko -> trusted.ko. "key_type_trusted" is the symbol that encrypted_keys needs regardless of whether the tpm is present. ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <CAA9_cmcOD2zPaaNbkYAaH5DRDRAebPkW+hwPA0zPKY4kU8R-rg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM [not found] ` <CAA9_cmcOD2zPaaNbkYAaH5DRDRAebPkW+hwPA0zPKY4kU8R-rg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2019-03-19 0:56 ` James Bottomley @ 2019-03-19 0:56 ` James Bottomley 1 sibling, 0 replies; 50+ messages in thread From: James Bottomley @ 2019-03-19 0:56 UTC (permalink / raw) To: Dan Williams Cc: linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Roberto Sassu, Linux Kernel Mailing List, Mimi Zohar, David Howells, keyrings-u79uwXL29TY76Z2rM5mHXA, Jarkko Sakkinen On Mon, 2019-03-18 at 17:30 -0700, Dan Williams wrote: > On Mon, Mar 18, 2019 at 5:24 PM James Bottomley <jejb-tEXmvtCZX7AybS5Ee8rs3A@public.gmane.org> > wrote: > > > > On Mon, 2019-03-18 at 16:45 -0700, Dan Williams wrote: > > > Rather than fail initialization of the trusted.ko module, arrange > > > for the module to load, but rely on trusted_instantiate() to fail > > > trusted-key operations. > > > > What actual problem is this fixing? To me it would seem like an > > enhancement to make the trusted module fail at load time if there's > > no TPM rather than waiting until first use to find out it can never > > work. Is there some piece of user code that depends on the > > successful insertion of trusted.ko? > > The module dependency chain relies on it. If that can be broken that > would also be an acceptable fix. > > I found this through the following dependency chain: libnvdimm.ko -> > encrypted_keys.ko -> trusted.ko. > > "key_type_trusted" is the symbol that encrypted_keys needs regardless > of whether the tpm is present. That's a nasty dependency caused by every key type module exporting a symbol for its key type. It really seems that key types should be looked up by name not symbol to prevent more of these dependency issues from spreading. Something like this (untested and definitely won't work without doing an EXPORT_SYMBOL on key_type_lookup). If it does look acceptable we can also disentangle the nasty module dependencies in the encrypted key code around masterkey which alone should be a huge improvement because that code is too hacky to live. James --- diff --git a/security/keys/encrypted-keys/masterkey_trusted.c b/security/keys/encrypted-keys/masterkey_trusted.c index dc3d18cae642..b98416f091e2 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 @@ -32,8 +33,14 @@ struct key *request_trusted_key(const char *trusted_desc, { struct trusted_key_payload *tpayload; struct key *tkey; + struct key_type *type; - 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; ^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-19 0:56 ` James Bottomley 0 siblings, 0 replies; 50+ messages in thread From: James Bottomley @ 2019-03-19 0:56 UTC (permalink / raw) To: Dan Williams Cc: Jarkko Sakkinen, linux-nvdimm, Roberto Sassu, Linux Kernel Mailing List, Mimi Zohar, David Howells, keyrings On Mon, 2019-03-18 at 17:30 -0700, Dan Williams wrote: > On Mon, Mar 18, 2019 at 5:24 PM James Bottomley <jejb@linux.ibm.com> > wrote: > > > > On Mon, 2019-03-18 at 16:45 -0700, Dan Williams wrote: > > > Rather than fail initialization of the trusted.ko module, arrange > > > for the module to load, but rely on trusted_instantiate() to fail > > > trusted-key operations. > > > > What actual problem is this fixing? To me it would seem like an > > enhancement to make the trusted module fail at load time if there's > > no TPM rather than waiting until first use to find out it can never > > work. Is there some piece of user code that depends on the > > successful insertion of trusted.ko? > > The module dependency chain relies on it. If that can be broken that > would also be an acceptable fix. > > I found this through the following dependency chain: libnvdimm.ko -> > encrypted_keys.ko -> trusted.ko. > > "key_type_trusted" is the symbol that encrypted_keys needs regardless > of whether the tpm is present. That's a nasty dependency caused by every key type module exporting a symbol for its key type. It really seems that key types should be looked up by name not symbol to prevent more of these dependency issues from spreading. Something like this (untested and definitely won't work without doing an EXPORT_SYMBOL on key_type_lookup). If it does look acceptable we can also disentangle the nasty module dependencies in the encrypted key code around masterkey which alone should be a huge improvement because that code is too hacky to live. James --- diff --git a/security/keys/encrypted-keys/masterkey_trusted.c b/security/keys/encrypted-keys/masterkey_trusted.c index dc3d18cae642..b98416f091e2 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 @@ -32,8 +33,14 @@ struct key *request_trusted_key(const char *trusted_desc, { struct trusted_key_payload *tpayload; struct key *tkey; + struct key_type *type; - 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; ^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-19 0:56 ` James Bottomley 0 siblings, 0 replies; 50+ messages in thread From: James Bottomley @ 2019-03-19 0:56 UTC (permalink / raw) To: Dan Williams Cc: linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Roberto Sassu, Linux Kernel Mailing List, Mimi Zohar, David Howells, keyrings-u79uwXL29TY76Z2rM5mHXA, Jarkko Sakkinen On Mon, 2019-03-18 at 17:30 -0700, Dan Williams wrote: > On Mon, Mar 18, 2019 at 5:24 PM James Bottomley <jejb@linux.ibm.com> > wrote: > > > > On Mon, 2019-03-18 at 16:45 -0700, Dan Williams wrote: > > > Rather than fail initialization of the trusted.ko module, arrange > > > for the module to load, but rely on trusted_instantiate() to fail > > > trusted-key operations. > > > > What actual problem is this fixing? To me it would seem like an > > enhancement to make the trusted module fail at load time if there's > > no TPM rather than waiting until first use to find out it can never > > work. Is there some piece of user code that depends on the > > successful insertion of trusted.ko? > > The module dependency chain relies on it. If that can be broken that > would also be an acceptable fix. > > I found this through the following dependency chain: libnvdimm.ko -> > encrypted_keys.ko -> trusted.ko. > > "key_type_trusted" is the symbol that encrypted_keys needs regardless > of whether the tpm is present. That's a nasty dependency caused by every key type module exporting a symbol for its key type. It really seems that key types should be looked up by name not symbol to prevent more of these dependency issues from spreading. Something like this (untested and definitely won't work without doing an EXPORT_SYMBOL on key_type_lookup). If it does look acceptable we can also disentangle the nasty module dependencies in the encrypted key code around masterkey which alone should be a huge improvement because that code is too hacky to live. James --- diff --git a/security/keys/encrypted-keys/masterkey_trusted.c b/security/keys/encrypted-keys/masterkey_trusted.c index dc3d18cae642..b98416f091e2 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 @@ -32,8 +33,14 @@ struct key *request_trusted_key(const char *trusted_desc, { struct trusted_key_payload *tpayload; struct key *tkey; + struct key_type *type; - 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; ^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM 2019-03-19 0:56 ` James Bottomley (?) @ 2019-03-19 1:34 ` Dan Williams -1 siblings, 0 replies; 50+ messages in thread From: Dan Williams @ 2019-03-19 1:34 UTC (permalink / raw) To: James Bottomley Cc: linux-nvdimm, Roberto Sassu, Linux Kernel Mailing List, Mimi Zohar, David Howells, keyrings, Jarkko Sakkinen On Mon, Mar 18, 2019 at 5:56 PM James Bottomley <jejb@linux.ibm.com> wrote: > > On Mon, 2019-03-18 at 17:30 -0700, Dan Williams wrote: > > On Mon, Mar 18, 2019 at 5:24 PM James Bottomley <jejb@linux.ibm.com> > > wrote: > > > > > > On Mon, 2019-03-18 at 16:45 -0700, Dan Williams wrote: > > > > Rather than fail initialization of the trusted.ko module, arrange > > > > for the module to load, but rely on trusted_instantiate() to fail > > > > trusted-key operations. > > > > > > What actual problem is this fixing? To me it would seem like an > > > enhancement to make the trusted module fail at load time if there's > > > no TPM rather than waiting until first use to find out it can never > > > work. Is there some piece of user code that depends on the > > > successful insertion of trusted.ko? > > > > The module dependency chain relies on it. If that can be broken that > > would also be an acceptable fix. > > > > I found this through the following dependency chain: libnvdimm.ko -> > > encrypted_keys.ko -> trusted.ko. > > > > "key_type_trusted" is the symbol that encrypted_keys needs regardless > > of whether the tpm is present. > > That's a nasty dependency caused by every key type module exporting a > symbol for its key type. It really seems that key types should be > looked up by name not symbol to prevent more of these dependency issues > from spreading. Something like this (untested and definitely won't > work without doing an EXPORT_SYMBOL on key_type_lookup). > > If it does look acceptable we can also disentangle the nasty module > dependencies in the encrypted key code around masterkey which alone > should be a huge improvement because that code is too hacky to live. Looks good to me. I fired it up with the export added and also included a: MODULE_SOFTDEP("pre: trusted"); ...to encourage trusted.ko to be ready, if possible, for the lookup. _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-19 1:34 ` Dan Williams 0 siblings, 0 replies; 50+ messages in thread From: Dan Williams @ 2019-03-19 1:34 UTC (permalink / raw) To: James Bottomley Cc: Jarkko Sakkinen, linux-nvdimm, Roberto Sassu, Linux Kernel Mailing List, Mimi Zohar, David Howells, keyrings On Mon, Mar 18, 2019 at 5:56 PM James Bottomley <jejb@linux.ibm.com> wrote: > > On Mon, 2019-03-18 at 17:30 -0700, Dan Williams wrote: > > On Mon, Mar 18, 2019 at 5:24 PM James Bottomley <jejb@linux.ibm.com> > > wrote: > > > > > > On Mon, 2019-03-18 at 16:45 -0700, Dan Williams wrote: > > > > Rather than fail initialization of the trusted.ko module, arrange > > > > for the module to load, but rely on trusted_instantiate() to fail > > > > trusted-key operations. > > > > > > What actual problem is this fixing? To me it would seem like an > > > enhancement to make the trusted module fail at load time if there's > > > no TPM rather than waiting until first use to find out it can never > > > work. Is there some piece of user code that depends on the > > > successful insertion of trusted.ko? > > > > The module dependency chain relies on it. If that can be broken that > > would also be an acceptable fix. > > > > I found this through the following dependency chain: libnvdimm.ko -> > > encrypted_keys.ko -> trusted.ko. > > > > "key_type_trusted" is the symbol that encrypted_keys needs regardless > > of whether the tpm is present. > > That's a nasty dependency caused by every key type module exporting a > symbol for its key type. It really seems that key types should be > looked up by name not symbol to prevent more of these dependency issues > from spreading. Something like this (untested and definitely won't > work without doing an EXPORT_SYMBOL on key_type_lookup). > > If it does look acceptable we can also disentangle the nasty module > dependencies in the encrypted key code around masterkey which alone > should be a huge improvement because that code is too hacky to live. Looks good to me. I fired it up with the export added and also included a: MODULE_SOFTDEP("pre: trusted"); ...to encourage trusted.ko to be ready, if possible, for the lookup. ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-19 1:34 ` Dan Williams 0 siblings, 0 replies; 50+ messages in thread From: Dan Williams @ 2019-03-19 1:34 UTC (permalink / raw) To: James Bottomley Cc: linux-nvdimm, Roberto Sassu, Linux Kernel Mailing List, Mimi Zohar, David Howells, keyrings, Jarkko Sakkinen On Mon, Mar 18, 2019 at 5:56 PM James Bottomley <jejb@linux.ibm.com> wrote: > > On Mon, 2019-03-18 at 17:30 -0700, Dan Williams wrote: > > On Mon, Mar 18, 2019 at 5:24 PM James Bottomley <jejb@linux.ibm.com> > > wrote: > > > > > > On Mon, 2019-03-18 at 16:45 -0700, Dan Williams wrote: > > > > Rather than fail initialization of the trusted.ko module, arrange > > > > for the module to load, but rely on trusted_instantiate() to fail > > > > trusted-key operations. > > > > > > What actual problem is this fixing? To me it would seem like an > > > enhancement to make the trusted module fail at load time if there's > > > no TPM rather than waiting until first use to find out it can never > > > work. Is there some piece of user code that depends on the > > > successful insertion of trusted.ko? > > > > The module dependency chain relies on it. If that can be broken that > > would also be an acceptable fix. > > > > I found this through the following dependency chain: libnvdimm.ko -> > > encrypted_keys.ko -> trusted.ko. > > > > "key_type_trusted" is the symbol that encrypted_keys needs regardless > > of whether the tpm is present. > > That's a nasty dependency caused by every key type module exporting a > symbol for its key type. It really seems that key types should be > looked up by name not symbol to prevent more of these dependency issues > from spreading. Something like this (untested and definitely won't > work without doing an EXPORT_SYMBOL on key_type_lookup). > > If it does look acceptable we can also disentangle the nasty module > dependencies in the encrypted key code around masterkey which alone > should be a huge improvement because that code is too hacky to live. Looks good to me. I fired it up with the export added and also included a: MODULE_SOFTDEP("pre: trusted"); ...to encourage trusted.ko to be ready, if possible, for the lookup. ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM 2019-03-19 0:56 ` James Bottomley (?) @ 2019-03-20 1:55 ` Dan Williams -1 siblings, 0 replies; 50+ messages in thread From: Dan Williams @ 2019-03-20 1:55 UTC (permalink / raw) To: James Bottomley Cc: linux-nvdimm, Roberto Sassu, Linux Kernel Mailing List, Mimi Zohar, David Howells, keyrings, Jarkko Sakkinen On Mon, Mar 18, 2019 at 5:56 PM James Bottomley <jejb@linux.ibm.com> wrote: > > On Mon, 2019-03-18 at 17:30 -0700, Dan Williams wrote: > > On Mon, Mar 18, 2019 at 5:24 PM James Bottomley <jejb@linux.ibm.com> > > wrote: > > > > > > On Mon, 2019-03-18 at 16:45 -0700, Dan Williams wrote: > > > > Rather than fail initialization of the trusted.ko module, arrange > > > > for the module to load, but rely on trusted_instantiate() to fail > > > > trusted-key operations. > > > > > > What actual problem is this fixing? To me it would seem like an > > > enhancement to make the trusted module fail at load time if there's > > > no TPM rather than waiting until first use to find out it can never > > > work. Is there some piece of user code that depends on the > > > successful insertion of trusted.ko? > > > > The module dependency chain relies on it. If that can be broken that > > would also be an acceptable fix. > > > > I found this through the following dependency chain: libnvdimm.ko -> > > encrypted_keys.ko -> trusted.ko. > > > > "key_type_trusted" is the symbol that encrypted_keys needs regardless > > of whether the tpm is present. > > That's a nasty dependency caused by every key type module exporting a > symbol for its key type. It really seems that key types should be > looked up by name not symbol to prevent more of these dependency issues > from spreading. Something like this (untested and definitely won't > work without doing an EXPORT_SYMBOL on key_type_lookup). > > If it does look acceptable we can also disentangle the nasty module > dependencies in the encrypted key code around masterkey which alone > should be a huge improvement because that code is too hacky to live. > > James > > --- > diff --git a/security/keys/encrypted-keys/masterkey_trusted.c b/security/keys/encrypted-keys/masterkey_trusted.c > index dc3d18cae642..b98416f091e2 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 > @@ -32,8 +33,14 @@ struct key *request_trusted_key(const char *trusted_desc, > { > struct trusted_key_payload *tpayload; > struct key *tkey; > + struct key_type *type; > > - 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; This falls over with the need to pin the module while any key that needs service from the hosting key_type operations might be live in the system. I could hang a "struct module *" off of the key_type so the host module can be pinned, but that requires teaching all consumers of the key_type module lifetime. Not impossible, but I think too big for a fix, and I'd rather go with this local fixup to drop the dependency on tpm_default_chip() successfully enumerating a TPM. _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-20 1:55 ` Dan Williams 0 siblings, 0 replies; 50+ messages in thread From: Dan Williams @ 2019-03-20 1:55 UTC (permalink / raw) To: James Bottomley Cc: Jarkko Sakkinen, linux-nvdimm, Roberto Sassu, Linux Kernel Mailing List, Mimi Zohar, David Howells, keyrings On Mon, Mar 18, 2019 at 5:56 PM James Bottomley <jejb@linux.ibm.com> wrote: > > On Mon, 2019-03-18 at 17:30 -0700, Dan Williams wrote: > > On Mon, Mar 18, 2019 at 5:24 PM James Bottomley <jejb@linux.ibm.com> > > wrote: > > > > > > On Mon, 2019-03-18 at 16:45 -0700, Dan Williams wrote: > > > > Rather than fail initialization of the trusted.ko module, arrange > > > > for the module to load, but rely on trusted_instantiate() to fail > > > > trusted-key operations. > > > > > > What actual problem is this fixing? To me it would seem like an > > > enhancement to make the trusted module fail at load time if there's > > > no TPM rather than waiting until first use to find out it can never > > > work. Is there some piece of user code that depends on the > > > successful insertion of trusted.ko? > > > > The module dependency chain relies on it. If that can be broken that > > would also be an acceptable fix. > > > > I found this through the following dependency chain: libnvdimm.ko -> > > encrypted_keys.ko -> trusted.ko. > > > > "key_type_trusted" is the symbol that encrypted_keys needs regardless > > of whether the tpm is present. > > That's a nasty dependency caused by every key type module exporting a > symbol for its key type. It really seems that key types should be > looked up by name not symbol to prevent more of these dependency issues > from spreading. Something like this (untested and definitely won't > work without doing an EXPORT_SYMBOL on key_type_lookup). > > If it does look acceptable we can also disentangle the nasty module > dependencies in the encrypted key code around masterkey which alone > should be a huge improvement because that code is too hacky to live. > > James > > --- > diff --git a/security/keys/encrypted-keys/masterkey_trusted.c b/security/keys/encrypted-keys/masterkey_trusted.c > index dc3d18cae642..b98416f091e2 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 > @@ -32,8 +33,14 @@ struct key *request_trusted_key(const char *trusted_desc, > { > struct trusted_key_payload *tpayload; > struct key *tkey; > + struct key_type *type; > > - 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; This falls over with the need to pin the module while any key that needs service from the hosting key_type operations might be live in the system. I could hang a "struct module *" off of the key_type so the host module can be pinned, but that requires teaching all consumers of the key_type module lifetime. Not impossible, but I think too big for a fix, and I'd rather go with this local fixup to drop the dependency on tpm_default_chip() successfully enumerating a TPM. ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-20 1:55 ` Dan Williams 0 siblings, 0 replies; 50+ messages in thread From: Dan Williams @ 2019-03-20 1:55 UTC (permalink / raw) To: James Bottomley Cc: linux-nvdimm, Roberto Sassu, Linux Kernel Mailing List, Mimi Zohar, David Howells, keyrings, Jarkko Sakkinen On Mon, Mar 18, 2019 at 5:56 PM James Bottomley <jejb@linux.ibm.com> wrote: > > On Mon, 2019-03-18 at 17:30 -0700, Dan Williams wrote: > > On Mon, Mar 18, 2019 at 5:24 PM James Bottomley <jejb@linux.ibm.com> > > wrote: > > > > > > On Mon, 2019-03-18 at 16:45 -0700, Dan Williams wrote: > > > > Rather than fail initialization of the trusted.ko module, arrange > > > > for the module to load, but rely on trusted_instantiate() to fail > > > > trusted-key operations. > > > > > > What actual problem is this fixing? To me it would seem like an > > > enhancement to make the trusted module fail at load time if there's > > > no TPM rather than waiting until first use to find out it can never > > > work. Is there some piece of user code that depends on the > > > successful insertion of trusted.ko? > > > > The module dependency chain relies on it. If that can be broken that > > would also be an acceptable fix. > > > > I found this through the following dependency chain: libnvdimm.ko -> > > encrypted_keys.ko -> trusted.ko. > > > > "key_type_trusted" is the symbol that encrypted_keys needs regardless > > of whether the tpm is present. > > That's a nasty dependency caused by every key type module exporting a > symbol for its key type. It really seems that key types should be > looked up by name not symbol to prevent more of these dependency issues > from spreading. Something like this (untested and definitely won't > work without doing an EXPORT_SYMBOL on key_type_lookup). > > If it does look acceptable we can also disentangle the nasty module > dependencies in the encrypted key code around masterkey which alone > should be a huge improvement because that code is too hacky to live. > > James > > --- > diff --git a/security/keys/encrypted-keys/masterkey_trusted.c b/security/keys/encrypted-keys/masterkey_trusted.c > index dc3d18cae642..b98416f091e2 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 > @@ -32,8 +33,14 @@ struct key *request_trusted_key(const char *trusted_desc, > { > struct trusted_key_payload *tpayload; > struct key *tkey; > + struct key_type *type; > > - 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; This falls over with the need to pin the module while any key that needs service from the hosting key_type operations might be live in the system. I could hang a "struct module *" off of the key_type so the host module can be pinned, but that requires teaching all consumers of the key_type module lifetime. Not impossible, but I think too big for a fix, and I'd rather go with this local fixup to drop the dependency on tpm_default_chip() successfully enumerating a TPM. ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <CAPcyv4hXKQcdLnKG6rPNOJr2wjq31uCCr+16S5Yu6S5A_5UGrA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM [not found] ` <CAPcyv4hXKQcdLnKG6rPNOJr2wjq31uCCr+16S5Yu6S5A_5UGrA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2019-03-20 2:19 ` James Bottomley @ 2019-03-20 2:19 ` James Bottomley 0 siblings, 0 replies; 50+ messages in thread From: James Bottomley @ 2019-03-20 2:19 UTC (permalink / raw) To: Dan Williams Cc: linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Roberto Sassu, Linux Kernel Mailing List, Mimi Zohar, David Howells, keyrings-u79uwXL29TY76Z2rM5mHXA, Jarkko Sakkinen On Tue, 2019-03-19 at 18:55 -0700, Dan Williams wrote: > On Mon, Mar 18, 2019 at 5:56 PM James Bottomley <jejb-tEXmvtCZX7AybS5Ee8rs3A@public.gmane.org> > wrote: > > > > On Mon, 2019-03-18 at 17:30 -0700, Dan Williams wrote: > > > On Mon, Mar 18, 2019 at 5:24 PM James Bottomley > > > <jejb-tEXmvtCZX7AybS5Ee8rs3A@public.gmane.org> wrote: > > > > > > > > On Mon, 2019-03-18 at 16:45 -0700, Dan Williams wrote: > > > > > Rather than fail initialization of the trusted.ko module, > > > > > arrange for the module to load, but rely on > > > > > trusted_instantiate() to fail trusted-key operations. > > > > > > > > What actual problem is this fixing? To me it would seem like > > > > an enhancement to make the trusted module fail at load time if > > > > there's no TPM rather than waiting until first use to find out > > > > it can never work. Is there some piece of user code that > > > > depends on the successful insertion of trusted.ko? > > > > > > The module dependency chain relies on it. If that can be broken > > > that would also be an acceptable fix. > > > > > > I found this through the following dependency chain: libnvdimm.ko > > > -> encrypted_keys.ko -> trusted.ko. > > > > > > "key_type_trusted" is the symbol that encrypted_keys needs > > > regardless of whether the tpm is present. > > > > That's a nasty dependency caused by every key type module exporting > > a symbol for its key type. It really seems that key types should > > be looked up by name not symbol to prevent more of these dependency > > issues from spreading. Something like this (untested and > > definitely won't work without doing an EXPORT_SYMBOL on > > key_type_lookup). > > > > If it does look acceptable we can also disentangle the nasty module > > dependencies in the encrypted key code around masterkey which alone > > should be a huge improvement because that code is too hacky to > > live. > > > > James > > > > --- > > diff --git a/security/keys/encrypted-keys/masterkey_trusted.c > > b/security/keys/encrypted-keys/masterkey_trusted.c > > index dc3d18cae642..b98416f091e2 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 > > @@ -32,8 +33,14 @@ struct key *request_trusted_key(const char > > *trusted_desc, > > { > > struct trusted_key_payload *tpayload; > > struct key *tkey; > > + struct key_type *type; > > > > - 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; > > > This falls over with the need to pin the module while any key that > needs service from the hosting key_type operations might be live in > the system. > > I could hang a "struct module *" off of the key_type so the host > module can be pinned, but that requires teaching all consumers of the > key_type module lifetime. Not impossible, but I think too big for a > fix, and I'd rather go with this local fixup to drop the dependency > on tpm_default_chip() successfully enumerating a TPM. Heh, well this proved to be a can of worms and no mistake. Unfortunately all of this does need fixing otherwise the keyctl syscall has exactly the same problem. But I think I agree it's getting way out of scope for the bug you found. James ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-20 2:19 ` James Bottomley 0 siblings, 0 replies; 50+ messages in thread From: James Bottomley @ 2019-03-20 2:19 UTC (permalink / raw) To: Dan Williams Cc: Jarkko Sakkinen, linux-nvdimm, Roberto Sassu, Linux Kernel Mailing List, Mimi Zohar, David Howells, keyrings On Tue, 2019-03-19 at 18:55 -0700, Dan Williams wrote: > On Mon, Mar 18, 2019 at 5:56 PM James Bottomley <jejb@linux.ibm.com> > wrote: > > > > On Mon, 2019-03-18 at 17:30 -0700, Dan Williams wrote: > > > On Mon, Mar 18, 2019 at 5:24 PM James Bottomley > > > <jejb@linux.ibm.com> wrote: > > > > > > > > On Mon, 2019-03-18 at 16:45 -0700, Dan Williams wrote: > > > > > Rather than fail initialization of the trusted.ko module, > > > > > arrange for the module to load, but rely on > > > > > trusted_instantiate() to fail trusted-key operations. > > > > > > > > What actual problem is this fixing? To me it would seem like > > > > an enhancement to make the trusted module fail at load time if > > > > there's no TPM rather than waiting until first use to find out > > > > it can never work. Is there some piece of user code that > > > > depends on the successful insertion of trusted.ko? > > > > > > The module dependency chain relies on it. If that can be broken > > > that would also be an acceptable fix. > > > > > > I found this through the following dependency chain: libnvdimm.ko > > > -> encrypted_keys.ko -> trusted.ko. > > > > > > "key_type_trusted" is the symbol that encrypted_keys needs > > > regardless of whether the tpm is present. > > > > That's a nasty dependency caused by every key type module exporting > > a symbol for its key type. It really seems that key types should > > be looked up by name not symbol to prevent more of these dependency > > issues from spreading. Something like this (untested and > > definitely won't work without doing an EXPORT_SYMBOL on > > key_type_lookup). > > > > If it does look acceptable we can also disentangle the nasty module > > dependencies in the encrypted key code around masterkey which alone > > should be a huge improvement because that code is too hacky to > > live. > > > > James > > > > --- > > diff --git a/security/keys/encrypted-keys/masterkey_trusted.c > > b/security/keys/encrypted-keys/masterkey_trusted.c > > index dc3d18cae642..b98416f091e2 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 > > @@ -32,8 +33,14 @@ struct key *request_trusted_key(const char > > *trusted_desc, > > { > > struct trusted_key_payload *tpayload; > > struct key *tkey; > > + struct key_type *type; > > > > - 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; > > > This falls over with the need to pin the module while any key that > needs service from the hosting key_type operations might be live in > the system. > > I could hang a "struct module *" off of the key_type so the host > module can be pinned, but that requires teaching all consumers of the > key_type module lifetime. Not impossible, but I think too big for a > fix, and I'd rather go with this local fixup to drop the dependency > on tpm_default_chip() successfully enumerating a TPM. Heh, well this proved to be a can of worms and no mistake. Unfortunately all of this does need fixing otherwise the keyctl syscall has exactly the same problem. But I think I agree it's getting way out of scope for the bug you found. James ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-20 2:19 ` James Bottomley 0 siblings, 0 replies; 50+ messages in thread From: James Bottomley @ 2019-03-20 2:19 UTC (permalink / raw) To: Dan Williams Cc: linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Roberto Sassu, Linux Kernel Mailing List, Mimi Zohar, David Howells, keyrings-u79uwXL29TY76Z2rM5mHXA, Jarkko Sakkinen On Tue, 2019-03-19 at 18:55 -0700, Dan Williams wrote: > On Mon, Mar 18, 2019 at 5:56 PM James Bottomley <jejb@linux.ibm.com> > wrote: > > > > On Mon, 2019-03-18 at 17:30 -0700, Dan Williams wrote: > > > On Mon, Mar 18, 2019 at 5:24 PM James Bottomley > > > <jejb@linux.ibm.com> wrote: > > > > > > > > On Mon, 2019-03-18 at 16:45 -0700, Dan Williams wrote: > > > > > Rather than fail initialization of the trusted.ko module, > > > > > arrange for the module to load, but rely on > > > > > trusted_instantiate() to fail trusted-key operations. > > > > > > > > What actual problem is this fixing? To me it would seem like > > > > an enhancement to make the trusted module fail at load time if > > > > there's no TPM rather than waiting until first use to find out > > > > it can never work. Is there some piece of user code that > > > > depends on the successful insertion of trusted.ko? > > > > > > The module dependency chain relies on it. If that can be broken > > > that would also be an acceptable fix. > > > > > > I found this through the following dependency chain: libnvdimm.ko > > > -> encrypted_keys.ko -> trusted.ko. > > > > > > "key_type_trusted" is the symbol that encrypted_keys needs > > > regardless of whether the tpm is present. > > > > That's a nasty dependency caused by every key type module exporting > > a symbol for its key type. It really seems that key types should > > be looked up by name not symbol to prevent more of these dependency > > issues from spreading. Something like this (untested and > > definitely won't work without doing an EXPORT_SYMBOL on > > key_type_lookup). > > > > If it does look acceptable we can also disentangle the nasty module > > dependencies in the encrypted key code around masterkey which alone > > should be a huge improvement because that code is too hacky to > > live. > > > > James > > > > --- > > diff --git a/security/keys/encrypted-keys/masterkey_trusted.c > > b/security/keys/encrypted-keys/masterkey_trusted.c > > index dc3d18cae642..b98416f091e2 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 > > @@ -32,8 +33,14 @@ struct key *request_trusted_key(const char > > *trusted_desc, > > { > > struct trusted_key_payload *tpayload; > > struct key *tkey; > > + struct key_type *type; > > > > - 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; > > > This falls over with the need to pin the module while any key that > needs service from the hosting key_type operations might be live in > the system. > > I could hang a "struct module *" off of the key_type so the host > module can be pinned, but that requires teaching all consumers of the > key_type module lifetime. Not impossible, but I think too big for a > fix, and I'd rather go with this local fixup to drop the dependency > on tpm_default_chip() successfully enumerating a TPM. Heh, well this proved to be a can of worms and no mistake. Unfortunately all of this does need fixing otherwise the keyctl syscall has exactly the same problem. But I think I agree it's getting way out of scope for the bug you found. James ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM [not found] ` <CAA9_cmcOD2zPaaNbkYAaH5DRDRAebPkW+hwPA0zPKY4kU8R-rg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2019-03-19 0:56 ` James Bottomley @ 2019-03-19 22:56 ` Mimi Zohar 1 sibling, 0 replies; 50+ messages in thread From: Mimi Zohar @ 2019-03-19 22:56 UTC (permalink / raw) To: Dan Williams, James Bottomley Cc: linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Roberto Sassu, Linux Kernel Mailing List, Jarkko Sakkinen, David Howells, keyrings-u79uwXL29TY76Z2rM5mHXA Hi Dan, On Mon, 2019-03-18 at 17:30 -0700, Dan Williams wrote: Sorry for the late reply. > On Mon, Mar 18, 2019 at 5:24 PM James Bottomley <jejb@linux.ibm.com> wrote: > > > > On Mon, 2019-03-18 at 16:45 -0700, Dan Williams wrote: > > > Rather than fail initialization of the trusted.ko module, arrange for > > > the module to load, but rely on trusted_instantiate() to fail > > > trusted-key operations. > > > > What actual problem is this fixing? To me it would seem like an > > enhancement to make the trusted module fail at load time if there's no > > TPM rather than waiting until first use to find out it can never work. > > Is there some piece of user code that depends on the successful > > insertion of trusted.ko? > > The module dependency chain relies on it. If that can be broken that > would also be an acceptable fix. > > I found this through the following dependency chain: libnvdimm.ko -> > encrypted_keys.ko -> trusted.ko. > > "key_type_trusted" is the symbol that encrypted_keys needs regardless > of whether the tpm is present. Commit 982e617a313b ("encrypted-keys: remove trusted-keys dependency") removed the dependency on trusted keys. masterkey_trusted.c should only be included if "CONFIG_TRUSTED_KEYS" is enabled. Is CONFIG_TRUSTED_KEYS enabled? Mimi _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-19 22:56 ` Mimi Zohar 0 siblings, 0 replies; 50+ messages in thread From: Mimi Zohar @ 2019-03-19 22:56 UTC (permalink / raw) To: Dan Williams, James Bottomley Cc: Jarkko Sakkinen, linux-nvdimm, Roberto Sassu, Linux Kernel Mailing List, David Howells, keyrings Hi Dan, On Mon, 2019-03-18 at 17:30 -0700, Dan Williams wrote: Sorry for the late reply. > On Mon, Mar 18, 2019 at 5:24 PM James Bottomley <jejb@linux.ibm.com> wrote: > > > > On Mon, 2019-03-18 at 16:45 -0700, Dan Williams wrote: > > > Rather than fail initialization of the trusted.ko module, arrange for > > > the module to load, but rely on trusted_instantiate() to fail > > > trusted-key operations. > > > > What actual problem is this fixing? To me it would seem like an > > enhancement to make the trusted module fail at load time if there's no > > TPM rather than waiting until first use to find out it can never work. > > Is there some piece of user code that depends on the successful > > insertion of trusted.ko? > > The module dependency chain relies on it. If that can be broken that > would also be an acceptable fix. > > I found this through the following dependency chain: libnvdimm.ko -> > encrypted_keys.ko -> trusted.ko. > > "key_type_trusted" is the symbol that encrypted_keys needs regardless > of whether the tpm is present. Commit 982e617a313b ("encrypted-keys: remove trusted-keys dependency") removed the dependency on trusted keys. masterkey_trusted.c should only be included if "CONFIG_TRUSTED_KEYS" is enabled. Is CONFIG_TRUSTED_KEYS enabled? Mimi ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-19 22:56 ` Mimi Zohar 0 siblings, 0 replies; 50+ messages in thread From: Mimi Zohar @ 2019-03-19 22:56 UTC (permalink / raw) To: Dan Williams, James Bottomley Cc: linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Roberto Sassu, Linux Kernel Mailing List, Jarkko Sakkinen, David Howells, keyrings-u79uwXL29TY76Z2rM5mHXA Hi Dan, On Mon, 2019-03-18 at 17:30 -0700, Dan Williams wrote: Sorry for the late reply. > On Mon, Mar 18, 2019 at 5:24 PM James Bottomley <jejb@linux.ibm.com> wrote: > > > > On Mon, 2019-03-18 at 16:45 -0700, Dan Williams wrote: > > > Rather than fail initialization of the trusted.ko module, arrange for > > > the module to load, but rely on trusted_instantiate() to fail > > > trusted-key operations. > > > > What actual problem is this fixing? To me it would seem like an > > enhancement to make the trusted module fail at load time if there's no > > TPM rather than waiting until first use to find out it can never work. > > Is there some piece of user code that depends on the successful > > insertion of trusted.ko? > > The module dependency chain relies on it. If that can be broken that > would also be an acceptable fix. > > I found this through the following dependency chain: libnvdimm.ko -> > encrypted_keys.ko -> trusted.ko. > > "key_type_trusted" is the symbol that encrypted_keys needs regardless > of whether the tpm is present. Commit 982e617a313b ("encrypted-keys: remove trusted-keys dependency") removed the dependency on trusted keys. masterkey_trusted.c should only be included if "CONFIG_TRUSTED_KEYS" is enabled. Is CONFIG_TRUSTED_KEYS enabled? Mimi ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM 2019-03-19 22:56 ` Mimi Zohar (?) @ 2019-03-19 23:01 ` Dan Williams -1 siblings, 0 replies; 50+ messages in thread From: Dan Williams @ 2019-03-19 23:01 UTC (permalink / raw) To: Mimi Zohar Cc: linux-nvdimm, James Bottomley, Roberto Sassu, Linux Kernel Mailing List, Jarkko Sakkinen, David Howells, keyrings On Tue, Mar 19, 2019 at 3:56 PM Mimi Zohar <zohar@linux.ibm.com> wrote: > > Hi Dan, > > On Mon, 2019-03-18 at 17:30 -0700, Dan Williams wrote: > > Sorry for the late reply. > > > On Mon, Mar 18, 2019 at 5:24 PM James Bottomley <jejb@linux.ibm.com> wrote: > > > > > > On Mon, 2019-03-18 at 16:45 -0700, Dan Williams wrote: > > > > Rather than fail initialization of the trusted.ko module, arrange for > > > > the module to load, but rely on trusted_instantiate() to fail > > > > trusted-key operations. > > > > > > What actual problem is this fixing? To me it would seem like an > > > enhancement to make the trusted module fail at load time if there's no > > > TPM rather than waiting until first use to find out it can never work. > > > Is there some piece of user code that depends on the successful > > > insertion of trusted.ko? > > > > The module dependency chain relies on it. If that can be broken that > > would also be an acceptable fix. > > > > I found this through the following dependency chain: libnvdimm.ko -> > > encrypted_keys.ko -> trusted.ko. > > > > "key_type_trusted" is the symbol that encrypted_keys needs regardless > > of whether the tpm is present. > > Commit 982e617a313b ("encrypted-keys: remove trusted-keys dependency") > removed the dependency on trusted keys. masterkey_trusted.c should > only be included if "CONFIG_TRUSTED_KEYS" is enabled. Is > CONFIG_TRUSTED_KEYS enabled? Yes, TRUSTED_KEYS is enabled, the module is built/available, and tries to load when encrypted_keys.ko loads. The problem is that it fails to load due an error returned from init_trusted(). The error is new for v5.1. So, instead of requiring the module dependencies to resolve successfully, and init_trusted() to return 0, the proposal is to just lookup the key types by name. _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-19 23:01 ` Dan Williams 0 siblings, 0 replies; 50+ messages in thread From: Dan Williams @ 2019-03-19 23:01 UTC (permalink / raw) To: Mimi Zohar Cc: James Bottomley, Jarkko Sakkinen, linux-nvdimm, Roberto Sassu, Linux Kernel Mailing List, David Howells, keyrings On Tue, Mar 19, 2019 at 3:56 PM Mimi Zohar <zohar@linux.ibm.com> wrote: > > Hi Dan, > > On Mon, 2019-03-18 at 17:30 -0700, Dan Williams wrote: > > Sorry for the late reply. > > > On Mon, Mar 18, 2019 at 5:24 PM James Bottomley <jejb@linux.ibm.com> wrote: > > > > > > On Mon, 2019-03-18 at 16:45 -0700, Dan Williams wrote: > > > > Rather than fail initialization of the trusted.ko module, arrange for > > > > the module to load, but rely on trusted_instantiate() to fail > > > > trusted-key operations. > > > > > > What actual problem is this fixing? To me it would seem like an > > > enhancement to make the trusted module fail at load time if there's no > > > TPM rather than waiting until first use to find out it can never work. > > > Is there some piece of user code that depends on the successful > > > insertion of trusted.ko? > > > > The module dependency chain relies on it. If that can be broken that > > would also be an acceptable fix. > > > > I found this through the following dependency chain: libnvdimm.ko -> > > encrypted_keys.ko -> trusted.ko. > > > > "key_type_trusted" is the symbol that encrypted_keys needs regardless > > of whether the tpm is present. > > Commit 982e617a313b ("encrypted-keys: remove trusted-keys dependency") > removed the dependency on trusted keys. masterkey_trusted.c should > only be included if "CONFIG_TRUSTED_KEYS" is enabled. Is > CONFIG_TRUSTED_KEYS enabled? Yes, TRUSTED_KEYS is enabled, the module is built/available, and tries to load when encrypted_keys.ko loads. The problem is that it fails to load due an error returned from init_trusted(). The error is new for v5.1. So, instead of requiring the module dependencies to resolve successfully, and init_trusted() to return 0, the proposal is to just lookup the key types by name. ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-19 23:01 ` Dan Williams 0 siblings, 0 replies; 50+ messages in thread From: Dan Williams @ 2019-03-19 23:01 UTC (permalink / raw) To: Mimi Zohar Cc: linux-nvdimm, James Bottomley, Roberto Sassu, Linux Kernel Mailing List, Jarkko Sakkinen, David Howells, keyrings On Tue, Mar 19, 2019 at 3:56 PM Mimi Zohar <zohar@linux.ibm.com> wrote: > > Hi Dan, > > On Mon, 2019-03-18 at 17:30 -0700, Dan Williams wrote: > > Sorry for the late reply. > > > On Mon, Mar 18, 2019 at 5:24 PM James Bottomley <jejb@linux.ibm.com> wrote: > > > > > > On Mon, 2019-03-18 at 16:45 -0700, Dan Williams wrote: > > > > Rather than fail initialization of the trusted.ko module, arrange for > > > > the module to load, but rely on trusted_instantiate() to fail > > > > trusted-key operations. > > > > > > What actual problem is this fixing? To me it would seem like an > > > enhancement to make the trusted module fail at load time if there's no > > > TPM rather than waiting until first use to find out it can never work. > > > Is there some piece of user code that depends on the successful > > > insertion of trusted.ko? > > > > The module dependency chain relies on it. If that can be broken that > > would also be an acceptable fix. > > > > I found this through the following dependency chain: libnvdimm.ko -> > > encrypted_keys.ko -> trusted.ko. > > > > "key_type_trusted" is the symbol that encrypted_keys needs regardless > > of whether the tpm is present. > > Commit 982e617a313b ("encrypted-keys: remove trusted-keys dependency") > removed the dependency on trusted keys. masterkey_trusted.c should > only be included if "CONFIG_TRUSTED_KEYS" is enabled. Is > CONFIG_TRUSTED_KEYS enabled? Yes, TRUSTED_KEYS is enabled, the module is built/available, and tries to load when encrypted_keys.ko loads. The problem is that it fails to load due an error returned from init_trusted(). The error is new for v5.1. So, instead of requiring the module dependencies to resolve successfully, and init_trusted() to return 0, the proposal is to just lookup the key types by name. ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM 2019-03-18 23:45 ` Dan Williams (?) @ 2019-03-21 13:54 ` Jarkko Sakkinen -1 siblings, 0 replies; 50+ messages in thread From: Jarkko Sakkinen @ 2019-03-21 13:54 UTC (permalink / raw) To: Dan Williams Cc: linux-nvdimm, James Bottomley, Roberto Sassu, linux-kernel, Mimi Zohar, David Howells, keyrings On Mon, Mar 18, 2019 at 04:45:13PM -0700, Dan Williams wrote: > Rather than fail initialization of the trusted.ko module, arrange for > the module to load, but rely on trusted_instantiate() to fail > trusted-key operations. > > Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...") > Cc: Roberto Sassu <roberto.sassu@huawei.com> > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > Cc: James Bottomley <jejb@linux.ibm.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> It should check for chip in each function that uses TPM now that the code does not rely on default chip. Otherwise, the semantics are kind of inconsistent. /Jarkko _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-21 13:54 ` Jarkko Sakkinen 0 siblings, 0 replies; 50+ messages in thread From: Jarkko Sakkinen @ 2019-03-21 13:54 UTC (permalink / raw) To: Dan Williams Cc: Roberto Sassu, James Bottomley, Mimi Zohar, David Howells, keyrings, linux-nvdimm, linux-kernel On Mon, Mar 18, 2019 at 04:45:13PM -0700, Dan Williams wrote: > Rather than fail initialization of the trusted.ko module, arrange for > the module to load, but rely on trusted_instantiate() to fail > trusted-key operations. > > Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...") > Cc: Roberto Sassu <roberto.sassu@huawei.com> > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > Cc: James Bottomley <jejb@linux.ibm.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> It should check for chip in each function that uses TPM now that the code does not rely on default chip. Otherwise, the semantics are kind of inconsistent. /Jarkko ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-21 13:54 ` Jarkko Sakkinen 0 siblings, 0 replies; 50+ messages in thread From: Jarkko Sakkinen @ 2019-03-21 13:54 UTC (permalink / raw) To: Dan Williams Cc: linux-nvdimm, James Bottomley, Roberto Sassu, linux-kernel, Mimi Zohar, David Howells, keyrings On Mon, Mar 18, 2019 at 04:45:13PM -0700, Dan Williams wrote: > Rather than fail initialization of the trusted.ko module, arrange for > the module to load, but rely on trusted_instantiate() to fail > trusted-key operations. > > Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...") > Cc: Roberto Sassu <roberto.sassu@huawei.com> > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > Cc: James Bottomley <jejb@linux.ibm.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> It should check for chip in each function that uses TPM now that the code does not rely on default chip. Otherwise, the semantics are kind of inconsistent. /Jarkko ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM 2019-03-21 13:54 ` Jarkko Sakkinen (?) @ 2019-03-21 14:26 ` Roberto Sassu -1 siblings, 0 replies; 50+ messages in thread From: Roberto Sassu @ 2019-03-21 14:26 UTC (permalink / raw) To: Jarkko Sakkinen, Dan Williams Cc: James Bottomley, Mimi Zohar, David Howells, keyrings, linux-nvdimm, linux-kernel On 3/21/2019 2:54 PM, Jarkko Sakkinen wrote: > On Mon, Mar 18, 2019 at 04:45:13PM -0700, Dan Williams wrote: >> Rather than fail initialization of the trusted.ko module, arrange for >> the module to load, but rely on trusted_instantiate() to fail >> trusted-key operations. >> >> Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...") >> Cc: Roberto Sassu <roberto.sassu@huawei.com> >> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >> Cc: James Bottomley <jejb@linux.ibm.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> > > It should check for chip in each function that uses TPM now that > the code does not rely on default chip. Otherwise, the semantics > are kind of inconsistent. If no other TPM function can be used before a successful key instantiate, checking for a chip only in trusted_instantiate() seems sufficient. Then, the same chip will be used by all TPM functions until module unloading, since we incremented the reference count. I would suggest to move the tpm_default_chip() and init_digests() calls to trusted_instantiate() to restore the old behavior of init_trusted(). trusted_instantiate() should look like: --- if (!chip) { chip = tpm_default_chip(); if (!chip) return -ENODEV; } if (!digests) { ret = init_digests(); if (ret < 0) return ret; } --- Roberto > /Jarkko > -- HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Bo PENG, Jian LI, Yanli SHI ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-21 14:26 ` Roberto Sassu 0 siblings, 0 replies; 50+ messages in thread From: Roberto Sassu @ 2019-03-21 14:26 UTC (permalink / raw) To: Jarkko Sakkinen, Dan Williams Cc: James Bottomley, Mimi Zohar, David Howells, keyrings, linux-nvdimm, linux-kernel On 3/21/2019 2:54 PM, Jarkko Sakkinen wrote: > On Mon, Mar 18, 2019 at 04:45:13PM -0700, Dan Williams wrote: >> Rather than fail initialization of the trusted.ko module, arrange for >> the module to load, but rely on trusted_instantiate() to fail >> trusted-key operations. >> >> Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...") >> Cc: Roberto Sassu <roberto.sassu@huawei.com> >> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >> Cc: James Bottomley <jejb@linux.ibm.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> > > It should check for chip in each function that uses TPM now that > the code does not rely on default chip. Otherwise, the semantics > are kind of inconsistent. If no other TPM function can be used before a successful key instantiate, checking for a chip only in trusted_instantiate() seems sufficient. Then, the same chip will be used by all TPM functions until module unloading, since we incremented the reference count. I would suggest to move the tpm_default_chip() and init_digests() calls to trusted_instantiate() to restore the old behavior of init_trusted(). trusted_instantiate() should look like: --- if (!chip) { chip = tpm_default_chip(); if (!chip) return -ENODEV; } if (!digests) { ret = init_digests(); if (ret < 0) return ret; } --- Roberto > /Jarkko > -- HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Bo PENG, Jian LI, Yanli SHI ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-21 14:26 ` Roberto Sassu 0 siblings, 0 replies; 50+ messages in thread From: Roberto Sassu @ 2019-03-21 14:26 UTC (permalink / raw) To: Jarkko Sakkinen, Dan Williams Cc: James Bottomley, Mimi Zohar, David Howells, keyrings, linux-nvdimm, linux-kernel On 3/21/2019 2:54 PM, Jarkko Sakkinen wrote: > On Mon, Mar 18, 2019 at 04:45:13PM -0700, Dan Williams wrote: >> Rather than fail initialization of the trusted.ko module, arrange for >> the module to load, but rely on trusted_instantiate() to fail >> trusted-key operations. >> >> Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...") >> Cc: Roberto Sassu <roberto.sassu@huawei.com> >> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >> Cc: James Bottomley <jejb@linux.ibm.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> > > It should check for chip in each function that uses TPM now that > the code does not rely on default chip. Otherwise, the semantics > are kind of inconsistent. If no other TPM function can be used before a successful key instantiate, checking for a chip only in trusted_instantiate() seems sufficient. Then, the same chip will be used by all TPM functions until module unloading, since we incremented the reference count. I would suggest to move the tpm_default_chip() and init_digests() calls to trusted_instantiate() to restore the old behavior of init_trusted(). trusted_instantiate() should look like: --- if (!chip) { chip = tpm_default_chip(); if (!chip) return -ENODEV; } if (!digests) { ret = init_digests(); if (ret < 0) return ret; } --- Roberto > /Jarkko > -- HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Bo PENG, Jian LI, Yanli SHI ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM 2019-03-21 14:26 ` Roberto Sassu (?) @ 2019-03-21 16:30 ` Dan Williams -1 siblings, 0 replies; 50+ messages in thread From: Dan Williams @ 2019-03-21 16:30 UTC (permalink / raw) To: Roberto Sassu Cc: linux-nvdimm, James Bottomley, Linux Kernel Mailing List, Mimi Zohar, David Howells, keyrings, Jarkko Sakkinen On Thu, Mar 21, 2019 at 7:27 AM Roberto Sassu <roberto.sassu@huawei.com> wrote: > > On 3/21/2019 2:54 PM, Jarkko Sakkinen wrote: > > On Mon, Mar 18, 2019 at 04:45:13PM -0700, Dan Williams wrote: > >> Rather than fail initialization of the trusted.ko module, arrange for > >> the module to load, but rely on trusted_instantiate() to fail > >> trusted-key operations. > >> > >> Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...") > >> Cc: Roberto Sassu <roberto.sassu@huawei.com> > >> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > >> Cc: James Bottomley <jejb@linux.ibm.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> > > > > It should check for chip in each function that uses TPM now that > > the code does not rely on default chip. Otherwise, the semantics > > are kind of inconsistent. > > If no other TPM function can be used before a successful key > instantiate, checking for a chip only in trusted_instantiate() seems > sufficient. Then, the same chip will be used by all TPM functions until > module unloading, since we incremented the reference count. > > I would suggest to move the tpm_default_chip() and init_digests() calls > to trusted_instantiate() to restore the old behavior of init_trusted(). > > trusted_instantiate() should look like: > --- > if (!chip) { > chip = tpm_default_chip(); > if (!chip) > return -ENODEV; > } > > if (!digests) { > ret = init_digests(); > if (ret < 0) > return ret; > } This patch already achieves that because tpm_find_get_ops() will fail and cause tpm_is_tpm2() to return NULL. _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-21 16:30 ` Dan Williams 0 siblings, 0 replies; 50+ messages in thread From: Dan Williams @ 2019-03-21 16:30 UTC (permalink / raw) To: Roberto Sassu Cc: Jarkko Sakkinen, James Bottomley, Mimi Zohar, David Howells, keyrings, linux-nvdimm, Linux Kernel Mailing List On Thu, Mar 21, 2019 at 7:27 AM Roberto Sassu <roberto.sassu@huawei.com> wrote: > > On 3/21/2019 2:54 PM, Jarkko Sakkinen wrote: > > On Mon, Mar 18, 2019 at 04:45:13PM -0700, Dan Williams wrote: > >> Rather than fail initialization of the trusted.ko module, arrange for > >> the module to load, but rely on trusted_instantiate() to fail > >> trusted-key operations. > >> > >> Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...") > >> Cc: Roberto Sassu <roberto.sassu@huawei.com> > >> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > >> Cc: James Bottomley <jejb@linux.ibm.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> > > > > It should check for chip in each function that uses TPM now that > > the code does not rely on default chip. Otherwise, the semantics > > are kind of inconsistent. > > If no other TPM function can be used before a successful key > instantiate, checking for a chip only in trusted_instantiate() seems > sufficient. Then, the same chip will be used by all TPM functions until > module unloading, since we incremented the reference count. > > I would suggest to move the tpm_default_chip() and init_digests() calls > to trusted_instantiate() to restore the old behavior of init_trusted(). > > trusted_instantiate() should look like: > --- > if (!chip) { > chip = tpm_default_chip(); > if (!chip) > return -ENODEV; > } > > if (!digests) { > ret = init_digests(); > if (ret < 0) > return ret; > } This patch already achieves that because tpm_find_get_ops() will fail and cause tpm_is_tpm2() to return NULL. ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-21 16:30 ` Dan Williams 0 siblings, 0 replies; 50+ messages in thread From: Dan Williams @ 2019-03-21 16:30 UTC (permalink / raw) To: Roberto Sassu Cc: linux-nvdimm, James Bottomley, Linux Kernel Mailing List, Mimi Zohar, David Howells, keyrings, Jarkko Sakkinen On Thu, Mar 21, 2019 at 7:27 AM Roberto Sassu <roberto.sassu@huawei.com> wrote: > > On 3/21/2019 2:54 PM, Jarkko Sakkinen wrote: > > On Mon, Mar 18, 2019 at 04:45:13PM -0700, Dan Williams wrote: > >> Rather than fail initialization of the trusted.ko module, arrange for > >> the module to load, but rely on trusted_instantiate() to fail > >> trusted-key operations. > >> > >> Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...") > >> Cc: Roberto Sassu <roberto.sassu@huawei.com> > >> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > >> Cc: James Bottomley <jejb@linux.ibm.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> > > > > It should check for chip in each function that uses TPM now that > > the code does not rely on default chip. Otherwise, the semantics > > are kind of inconsistent. > > If no other TPM function can be used before a successful key > instantiate, checking for a chip only in trusted_instantiate() seems > sufficient. Then, the same chip will be used by all TPM functions until > module unloading, since we incremented the reference count. > > I would suggest to move the tpm_default_chip() and init_digests() calls > to trusted_instantiate() to restore the old behavior of init_trusted(). > > trusted_instantiate() should look like: > --- > if (!chip) { > chip = tpm_default_chip(); > if (!chip) > return -ENODEV; > } > > if (!digests) { > ret = init_digests(); > if (ret < 0) > return ret; > } This patch already achieves that because tpm_find_get_ops() will fail and cause tpm_is_tpm2() to return NULL. ^ permalink raw reply [flat|nested] 50+ messages in thread
[parent not found: <CAPcyv4gc_KWedC12bTcX24KwjKiBCF=yvouRNzCbJKaLrgJdDg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM [not found] ` <CAPcyv4gc_KWedC12bTcX24KwjKiBCF=yvouRNzCbJKaLrgJdDg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2019-03-21 17:45 ` Roberto Sassu @ 2019-03-21 17:45 ` Roberto Sassu 0 siblings, 0 replies; 50+ messages in thread From: Roberto Sassu @ 2019-03-21 17:45 UTC (permalink / raw) To: Dan Williams, Jarkko Sakkinen Cc: Silviu Vlasceanu, linux-nvdimm, James Bottomley, Linux Kernel Mailing List, Mimi Zohar, David Howells, keyrings-u79uwXL29TY76Z2rM5mHXA On 3/21/2019 5:30 PM, Dan Williams wrote: > On Thu, Mar 21, 2019 at 7:27 AM Roberto Sassu <roberto.sassu-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> wrote: >> >> On 3/21/2019 2:54 PM, Jarkko Sakkinen wrote: >>> On Mon, Mar 18, 2019 at 04:45:13PM -0700, Dan Williams wrote: >>>> Rather than fail initialization of the trusted.ko module, arrange for >>>> the module to load, but rely on trusted_instantiate() to fail >>>> trusted-key operations. >>>> >>>> Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...") >>>> Cc: Roberto Sassu <roberto.sassu-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> >>>> Cc: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> >>>> Cc: James Bottomley <jejb-tEXmvtCZX7AybS5Ee8rs3A@public.gmane.org> >>>> Cc: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> >>>> Cc: Mimi Zohar <zohar-tEXmvtCZX7AybS5Ee8rs3A@public.gmane.org> >>>> Cc: David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> >>>> Signed-off-by: Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> >>> >>> It should check for chip in each function that uses TPM now that >>> the code does not rely on default chip. Otherwise, the semantics >>> are kind of inconsistent. >> >> If no other TPM function can be used before a successful key >> instantiate, checking for a chip only in trusted_instantiate() seems >> sufficient. Then, the same chip will be used by all TPM functions until >> module unloading, since we incremented the reference count. >> >> I would suggest to move the tpm_default_chip() and init_digests() calls >> to trusted_instantiate() to restore the old behavior of init_trusted(). >> >> trusted_instantiate() should look like: >> --- >> if (!chip) { >> chip = tpm_default_chip(); >> if (!chip) >> return -ENODEV; >> } >> >> if (!digests) { >> ret = init_digests(); >> if (ret < 0) >> return ret; >> } > > This patch already achieves that because tpm_find_get_ops() will fail > and cause tpm_is_tpm2() to return NULL. In addition, the changes I proposed would allow users to create trusted keys if a TPM is added later. CONFIG_TRUSTED_KEYS=y and CONFIG_TCG_TPM=m is a valid configuration. Jarkko, Dan's patch seems sufficient to fix the issue. He could include the changes I proposed in his patch. What is your opinion? Thanks Roberto -- HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Bo PENG, Jian LI, Yanli SHI ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-21 17:45 ` Roberto Sassu 0 siblings, 0 replies; 50+ messages in thread From: Roberto Sassu @ 2019-03-21 17:45 UTC (permalink / raw) To: Dan Williams, Jarkko Sakkinen Cc: James Bottomley, Mimi Zohar, David Howells, keyrings, linux-nvdimm, Linux Kernel Mailing List, Silviu Vlasceanu On 3/21/2019 5:30 PM, Dan Williams wrote: > On Thu, Mar 21, 2019 at 7:27 AM Roberto Sassu <roberto.sassu@huawei.com> wrote: >> >> On 3/21/2019 2:54 PM, Jarkko Sakkinen wrote: >>> On Mon, Mar 18, 2019 at 04:45:13PM -0700, Dan Williams wrote: >>>> Rather than fail initialization of the trusted.ko module, arrange for >>>> the module to load, but rely on trusted_instantiate() to fail >>>> trusted-key operations. >>>> >>>> Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...") >>>> Cc: Roberto Sassu <roberto.sassu@huawei.com> >>>> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >>>> Cc: James Bottomley <jejb@linux.ibm.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> >>> >>> It should check for chip in each function that uses TPM now that >>> the code does not rely on default chip. Otherwise, the semantics >>> are kind of inconsistent. >> >> If no other TPM function can be used before a successful key >> instantiate, checking for a chip only in trusted_instantiate() seems >> sufficient. Then, the same chip will be used by all TPM functions until >> module unloading, since we incremented the reference count. >> >> I would suggest to move the tpm_default_chip() and init_digests() calls >> to trusted_instantiate() to restore the old behavior of init_trusted(). >> >> trusted_instantiate() should look like: >> --- >> if (!chip) { >> chip = tpm_default_chip(); >> if (!chip) >> return -ENODEV; >> } >> >> if (!digests) { >> ret = init_digests(); >> if (ret < 0) >> return ret; >> } > > This patch already achieves that because tpm_find_get_ops() will fail > and cause tpm_is_tpm2() to return NULL. In addition, the changes I proposed would allow users to create trusted keys if a TPM is added later. CONFIG_TRUSTED_KEYS=y and CONFIG_TCG_TPM=m is a valid configuration. Jarkko, Dan's patch seems sufficient to fix the issue. He could include the changes I proposed in his patch. What is your opinion? Thanks Roberto -- HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Bo PENG, Jian LI, Yanli SHI ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-21 17:45 ` Roberto Sassu 0 siblings, 0 replies; 50+ messages in thread From: Roberto Sassu @ 2019-03-21 17:45 UTC (permalink / raw) To: Dan Williams, Jarkko Sakkinen Cc: Silviu Vlasceanu, linux-nvdimm, James Bottomley, Linux Kernel Mailing List, Mimi Zohar, David Howells, keyrings-u79uwXL29TY76Z2rM5mHXA On 3/21/2019 5:30 PM, Dan Williams wrote: > On Thu, Mar 21, 2019 at 7:27 AM Roberto Sassu <roberto.sassu@huawei.com> wrote: >> >> On 3/21/2019 2:54 PM, Jarkko Sakkinen wrote: >>> On Mon, Mar 18, 2019 at 04:45:13PM -0700, Dan Williams wrote: >>>> Rather than fail initialization of the trusted.ko module, arrange for >>>> the module to load, but rely on trusted_instantiate() to fail >>>> trusted-key operations. >>>> >>>> Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...") >>>> Cc: Roberto Sassu <roberto.sassu@huawei.com> >>>> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >>>> Cc: James Bottomley <jejb@linux.ibm.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> >>> >>> It should check for chip in each function that uses TPM now that >>> the code does not rely on default chip. Otherwise, the semantics >>> are kind of inconsistent. >> >> If no other TPM function can be used before a successful key >> instantiate, checking for a chip only in trusted_instantiate() seems >> sufficient. Then, the same chip will be used by all TPM functions until >> module unloading, since we incremented the reference count. >> >> I would suggest to move the tpm_default_chip() and init_digests() calls >> to trusted_instantiate() to restore the old behavior of init_trusted(). >> >> trusted_instantiate() should look like: >> --- >> if (!chip) { >> chip = tpm_default_chip(); >> if (!chip) >> return -ENODEV; >> } >> >> if (!digests) { >> ret = init_digests(); >> if (ret < 0) >> return ret; >> } > > This patch already achieves that because tpm_find_get_ops() will fail > and cause tpm_is_tpm2() to return NULL. In addition, the changes I proposed would allow users to create trusted keys if a TPM is added later. CONFIG_TRUSTED_KEYS=y and CONFIG_TCG_TPM=m is a valid configuration. Jarkko, Dan's patch seems sufficient to fix the issue. He could include the changes I proposed in his patch. What is your opinion? Thanks Roberto -- HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Bo PENG, Jian LI, Yanli SHI ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM 2019-03-21 17:45 ` Roberto Sassu @ 2019-03-22 10:12 ` Jarkko Sakkinen -1 siblings, 0 replies; 50+ messages in thread From: Jarkko Sakkinen @ 2019-03-22 10:12 UTC (permalink / raw) To: Roberto Sassu Cc: Dan Williams, James Bottomley, Mimi Zohar, David Howells, keyrings, linux-nvdimm, Linux Kernel Mailing List, Silviu Vlasceanu On Thu, Mar 21, 2019 at 06:45:47PM +0100, Roberto Sassu wrote: > On 3/21/2019 5:30 PM, Dan Williams wrote: > > On Thu, Mar 21, 2019 at 7:27 AM Roberto Sassu <roberto.sassu@huawei.com> wrote: > > > > > > On 3/21/2019 2:54 PM, Jarkko Sakkinen wrote: > > > > On Mon, Mar 18, 2019 at 04:45:13PM -0700, Dan Williams wrote: > > > > > Rather than fail initialization of the trusted.ko module, arrange for > > > > > the module to load, but rely on trusted_instantiate() to fail > > > > > trusted-key operations. > > > > > > > > > > Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...") > > > > > Cc: Roberto Sassu <roberto.sassu@huawei.com> > > > > > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > > > > Cc: James Bottomley <jejb@linux.ibm.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> > > > > > > > > It should check for chip in each function that uses TPM now that > > > > the code does not rely on default chip. Otherwise, the semantics > > > > are kind of inconsistent. > > > > > > If no other TPM function can be used before a successful key > > > instantiate, checking for a chip only in trusted_instantiate() seems > > > sufficient. Then, the same chip will be used by all TPM functions until > > > module unloading, since we incremented the reference count. > > > > > > I would suggest to move the tpm_default_chip() and init_digests() calls > > > to trusted_instantiate() to restore the old behavior of init_trusted(). > > > > > > trusted_instantiate() should look like: > > > --- > > > if (!chip) { > > > chip = tpm_default_chip(); > > > if (!chip) > > > return -ENODEV; > > > } > > > > > > if (!digests) { > > > ret = init_digests(); > > > if (ret < 0) > > > return ret; > > > } > > > > This patch already achieves that because tpm_find_get_ops() will fail > > and cause tpm_is_tpm2() to return NULL. > > In addition, the changes I proposed would allow users to create trusted > keys if a TPM is added later. CONFIG_TRUSTED_KEYS=y and > CONFIG_TCG_TPM=m is a valid configuration. > > Jarkko, Dan's patch seems sufficient to fix the issue. He could include > the changes I proposed in his patch. What is your opinion? Agreed. /Jarkko ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-22 10:12 ` Jarkko Sakkinen 0 siblings, 0 replies; 50+ messages in thread From: Jarkko Sakkinen @ 2019-03-22 10:12 UTC (permalink / raw) To: Roberto Sassu Cc: Dan Williams, James Bottomley, Mimi Zohar, David Howells, keyrings, linux-nvdimm, Linux Kernel Mailing List, Silviu Vlasceanu On Thu, Mar 21, 2019 at 06:45:47PM +0100, Roberto Sassu wrote: > On 3/21/2019 5:30 PM, Dan Williams wrote: > > On Thu, Mar 21, 2019 at 7:27 AM Roberto Sassu <roberto.sassu@huawei.com> wrote: > > > > > > On 3/21/2019 2:54 PM, Jarkko Sakkinen wrote: > > > > On Mon, Mar 18, 2019 at 04:45:13PM -0700, Dan Williams wrote: > > > > > Rather than fail initialization of the trusted.ko module, arrange for > > > > > the module to load, but rely on trusted_instantiate() to fail > > > > > trusted-key operations. > > > > > > > > > > Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...") > > > > > Cc: Roberto Sassu <roberto.sassu@huawei.com> > > > > > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > > > > Cc: James Bottomley <jejb@linux.ibm.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> > > > > > > > > It should check for chip in each function that uses TPM now that > > > > the code does not rely on default chip. Otherwise, the semantics > > > > are kind of inconsistent. > > > > > > If no other TPM function can be used before a successful key > > > instantiate, checking for a chip only in trusted_instantiate() seems > > > sufficient. Then, the same chip will be used by all TPM functions until > > > module unloading, since we incremented the reference count. > > > > > > I would suggest to move the tpm_default_chip() and init_digests() calls > > > to trusted_instantiate() to restore the old behavior of init_trusted(). > > > > > > trusted_instantiate() should look like: > > > --- > > > if (!chip) { > > > chip = tpm_default_chip(); > > > if (!chip) > > > return -ENODEV; > > > } > > > > > > if (!digests) { > > > ret = init_digests(); > > > if (ret < 0) > > > return ret; > > > } > > > > This patch already achieves that because tpm_find_get_ops() will fail > > and cause tpm_is_tpm2() to return NULL. > > In addition, the changes I proposed would allow users to create trusted > keys if a TPM is added later. CONFIG_TRUSTED_KEYS=y and > CONFIG_TCG_TPM=m is a valid configuration. > > Jarkko, Dan's patch seems sufficient to fix the issue. He could include > the changes I proposed in his patch. What is your opinion? Agreed. /Jarkko ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM 2019-03-22 10:12 ` Jarkko Sakkinen (?) @ 2019-03-22 15:24 ` Dan Williams -1 siblings, 0 replies; 50+ messages in thread From: Dan Williams @ 2019-03-22 15:24 UTC (permalink / raw) To: Jarkko Sakkinen Cc: Silviu Vlasceanu, linux-nvdimm, James Bottomley, Roberto Sassu, Linux Kernel Mailing List, Mimi Zohar, David Howells, keyrings On Fri, Mar 22, 2019 at 3:12 AM Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote: > > On Thu, Mar 21, 2019 at 06:45:47PM +0100, Roberto Sassu wrote: > > On 3/21/2019 5:30 PM, Dan Williams wrote: > > > On Thu, Mar 21, 2019 at 7:27 AM Roberto Sassu <roberto.sassu@huawei.com> wrote: > > > > > > > > On 3/21/2019 2:54 PM, Jarkko Sakkinen wrote: > > > > > On Mon, Mar 18, 2019 at 04:45:13PM -0700, Dan Williams wrote: > > > > > > Rather than fail initialization of the trusted.ko module, arrange for > > > > > > the module to load, but rely on trusted_instantiate() to fail > > > > > > trusted-key operations. > > > > > > > > > > > > Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...") > > > > > > Cc: Roberto Sassu <roberto.sassu@huawei.com> > > > > > > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > > > > > Cc: James Bottomley <jejb@linux.ibm.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> > > > > > > > > > > It should check for chip in each function that uses TPM now that > > > > > the code does not rely on default chip. Otherwise, the semantics > > > > > are kind of inconsistent. > > > > > > > > If no other TPM function can be used before a successful key > > > > instantiate, checking for a chip only in trusted_instantiate() seems > > > > sufficient. Then, the same chip will be used by all TPM functions until > > > > module unloading, since we incremented the reference count. > > > > > > > > I would suggest to move the tpm_default_chip() and init_digests() calls > > > > to trusted_instantiate() to restore the old behavior of init_trusted(). > > > > > > > > trusted_instantiate() should look like: > > > > --- > > > > if (!chip) { > > > > chip = tpm_default_chip(); > > > > if (!chip) > > > > return -ENODEV; > > > > } > > > > > > > > if (!digests) { > > > > ret = init_digests(); > > > > if (ret < 0) > > > > return ret; > > > > } > > > > > > This patch already achieves that because tpm_find_get_ops() will fail > > > and cause tpm_is_tpm2() to return NULL. > > > > In addition, the changes I proposed would allow users to create trusted > > keys if a TPM is added later. CONFIG_TRUSTED_KEYS=y and > > CONFIG_TCG_TPM=m is a valid configuration. > > > > Jarkko, Dan's patch seems sufficient to fix the issue. He could include > > the changes I proposed in his patch. What is your opinion? > > Agreed. What changes? Robert, please feel free to re-author the proposed patch however you see fit, I just want whatever will get libnvdimm operational again in the shortest amount of time. _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-22 15:24 ` Dan Williams 0 siblings, 0 replies; 50+ messages in thread From: Dan Williams @ 2019-03-22 15:24 UTC (permalink / raw) To: Jarkko Sakkinen Cc: Roberto Sassu, James Bottomley, Mimi Zohar, David Howells, keyrings, linux-nvdimm, Linux Kernel Mailing List, Silviu Vlasceanu On Fri, Mar 22, 2019 at 3:12 AM Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote: > > On Thu, Mar 21, 2019 at 06:45:47PM +0100, Roberto Sassu wrote: > > On 3/21/2019 5:30 PM, Dan Williams wrote: > > > On Thu, Mar 21, 2019 at 7:27 AM Roberto Sassu <roberto.sassu@huawei.com> wrote: > > > > > > > > On 3/21/2019 2:54 PM, Jarkko Sakkinen wrote: > > > > > On Mon, Mar 18, 2019 at 04:45:13PM -0700, Dan Williams wrote: > > > > > > Rather than fail initialization of the trusted.ko module, arrange for > > > > > > the module to load, but rely on trusted_instantiate() to fail > > > > > > trusted-key operations. > > > > > > > > > > > > Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...") > > > > > > Cc: Roberto Sassu <roberto.sassu@huawei.com> > > > > > > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > > > > > Cc: James Bottomley <jejb@linux.ibm.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> > > > > > > > > > > It should check for chip in each function that uses TPM now that > > > > > the code does not rely on default chip. Otherwise, the semantics > > > > > are kind of inconsistent. > > > > > > > > If no other TPM function can be used before a successful key > > > > instantiate, checking for a chip only in trusted_instantiate() seems > > > > sufficient. Then, the same chip will be used by all TPM functions until > > > > module unloading, since we incremented the reference count. > > > > > > > > I would suggest to move the tpm_default_chip() and init_digests() calls > > > > to trusted_instantiate() to restore the old behavior of init_trusted(). > > > > > > > > trusted_instantiate() should look like: > > > > --- > > > > if (!chip) { > > > > chip = tpm_default_chip(); > > > > if (!chip) > > > > return -ENODEV; > > > > } > > > > > > > > if (!digests) { > > > > ret = init_digests(); > > > > if (ret < 0) > > > > return ret; > > > > } > > > > > > This patch already achieves that because tpm_find_get_ops() will fail > > > and cause tpm_is_tpm2() to return NULL. > > > > In addition, the changes I proposed would allow users to create trusted > > keys if a TPM is added later. CONFIG_TRUSTED_KEYS=y and > > CONFIG_TCG_TPM=m is a valid configuration. > > > > Jarkko, Dan's patch seems sufficient to fix the issue. He could include > > the changes I proposed in his patch. What is your opinion? > > Agreed. What changes? Robert, please feel free to re-author the proposed patch however you see fit, I just want whatever will get libnvdimm operational again in the shortest amount of time. ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-22 15:24 ` Dan Williams 0 siblings, 0 replies; 50+ messages in thread From: Dan Williams @ 2019-03-22 15:24 UTC (permalink / raw) To: Jarkko Sakkinen Cc: Silviu Vlasceanu, linux-nvdimm, James Bottomley, Roberto Sassu, Linux Kernel Mailing List, Mimi Zohar, David Howells, keyrings On Fri, Mar 22, 2019 at 3:12 AM Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote: > > On Thu, Mar 21, 2019 at 06:45:47PM +0100, Roberto Sassu wrote: > > On 3/21/2019 5:30 PM, Dan Williams wrote: > > > On Thu, Mar 21, 2019 at 7:27 AM Roberto Sassu <roberto.sassu@huawei.com> wrote: > > > > > > > > On 3/21/2019 2:54 PM, Jarkko Sakkinen wrote: > > > > > On Mon, Mar 18, 2019 at 04:45:13PM -0700, Dan Williams wrote: > > > > > > Rather than fail initialization of the trusted.ko module, arrange for > > > > > > the module to load, but rely on trusted_instantiate() to fail > > > > > > trusted-key operations. > > > > > > > > > > > > Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...") > > > > > > Cc: Roberto Sassu <roberto.sassu@huawei.com> > > > > > > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > > > > > Cc: James Bottomley <jejb@linux.ibm.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> > > > > > > > > > > It should check for chip in each function that uses TPM now that > > > > > the code does not rely on default chip. Otherwise, the semantics > > > > > are kind of inconsistent. > > > > > > > > If no other TPM function can be used before a successful key > > > > instantiate, checking for a chip only in trusted_instantiate() seems > > > > sufficient. Then, the same chip will be used by all TPM functions until > > > > module unloading, since we incremented the reference count. > > > > > > > > I would suggest to move the tpm_default_chip() and init_digests() calls > > > > to trusted_instantiate() to restore the old behavior of init_trusted(). > > > > > > > > trusted_instantiate() should look like: > > > > --- > > > > if (!chip) { > > > > chip = tpm_default_chip(); > > > > if (!chip) > > > > return -ENODEV; > > > > } > > > > > > > > if (!digests) { > > > > ret = init_digests(); > > > > if (ret < 0) > > > > return ret; > > > > } > > > > > > This patch already achieves that because tpm_find_get_ops() will fail > > > and cause tpm_is_tpm2() to return NULL. > > > > In addition, the changes I proposed would allow users to create trusted > > keys if a TPM is added later. CONFIG_TRUSTED_KEYS=y and > > CONFIG_TCG_TPM=m is a valid configuration. > > > > Jarkko, Dan's patch seems sufficient to fix the issue. He could include > > the changes I proposed in his patch. What is your opinion? > > Agreed. What changes? Robert, please feel free to re-author the proposed patch however you see fit, I just want whatever will get libnvdimm operational again in the shortest amount of time. ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM 2019-03-22 15:24 ` Dan Williams (?) @ 2019-03-25 14:12 ` Jarkko Sakkinen -1 siblings, 0 replies; 50+ messages in thread From: Jarkko Sakkinen @ 2019-03-25 14:12 UTC (permalink / raw) To: Dan Williams Cc: Silviu Vlasceanu, linux-nvdimm, James Bottomley, Roberto Sassu, Linux Kernel Mailing List, Mimi Zohar, David Howells, keyrings On Fri, Mar 22, 2019 at 08:24:01AM -0700, Dan Williams wrote: > On Fri, Mar 22, 2019 at 3:12 AM Jarkko Sakkinen > <jarkko.sakkinen@linux.intel.com> wrote: > > > > On Thu, Mar 21, 2019 at 06:45:47PM +0100, Roberto Sassu wrote: > > > On 3/21/2019 5:30 PM, Dan Williams wrote: > > > > On Thu, Mar 21, 2019 at 7:27 AM Roberto Sassu <roberto.sassu@huawei.com> wrote: > > > > > > > > > > On 3/21/2019 2:54 PM, Jarkko Sakkinen wrote: > > > > > > On Mon, Mar 18, 2019 at 04:45:13PM -0700, Dan Williams wrote: > > > > > > > Rather than fail initialization of the trusted.ko module, arrange for > > > > > > > the module to load, but rely on trusted_instantiate() to fail > > > > > > > trusted-key operations. > > > > > > > > > > > > > > Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...") > > > > > > > Cc: Roberto Sassu <roberto.sassu@huawei.com> > > > > > > > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > > > > > > Cc: James Bottomley <jejb@linux.ibm.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> > > > > > > > > > > > > It should check for chip in each function that uses TPM now that > > > > > > the code does not rely on default chip. Otherwise, the semantics > > > > > > are kind of inconsistent. > > > > > > > > > > If no other TPM function can be used before a successful key > > > > > instantiate, checking for a chip only in trusted_instantiate() seems > > > > > sufficient. Then, the same chip will be used by all TPM functions until > > > > > module unloading, since we incremented the reference count. > > > > > > > > > > I would suggest to move the tpm_default_chip() and init_digests() calls > > > > > to trusted_instantiate() to restore the old behavior of init_trusted(). > > > > > > > > > > trusted_instantiate() should look like: > > > > > --- > > > > > if (!chip) { > > > > > chip = tpm_default_chip(); > > > > > if (!chip) > > > > > return -ENODEV; > > > > > } > > > > > > > > > > if (!digests) { > > > > > ret = init_digests(); > > > > > if (ret < 0) > > > > > return ret; > > > > > } > > > > > > > > This patch already achieves that because tpm_find_get_ops() will fail > > > > and cause tpm_is_tpm2() to return NULL. > > > > > > In addition, the changes I proposed would allow users to create trusted > > > keys if a TPM is added later. CONFIG_TRUSTED_KEYS=y and > > > CONFIG_TCG_TPM=m is a valid configuration. > > > > > > Jarkko, Dan's patch seems sufficient to fix the issue. He could include > > > the changes I proposed in his patch. What is your opinion? > > > > Agreed. > > What changes? > > Robert, please feel free to re-author the proposed patch however you > see fit, I just want whatever will get libnvdimm operational again in > the shortest amount of time. I've decided go with a patch of least innovation i.e. one that simply reverts the old behavior. Sending patch soon. /Jarkko _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-25 14:12 ` Jarkko Sakkinen 0 siblings, 0 replies; 50+ messages in thread From: Jarkko Sakkinen @ 2019-03-25 14:12 UTC (permalink / raw) To: Dan Williams Cc: Roberto Sassu, James Bottomley, Mimi Zohar, David Howells, keyrings, linux-nvdimm, Linux Kernel Mailing List, Silviu Vlasceanu On Fri, Mar 22, 2019 at 08:24:01AM -0700, Dan Williams wrote: > On Fri, Mar 22, 2019 at 3:12 AM Jarkko Sakkinen > <jarkko.sakkinen@linux.intel.com> wrote: > > > > On Thu, Mar 21, 2019 at 06:45:47PM +0100, Roberto Sassu wrote: > > > On 3/21/2019 5:30 PM, Dan Williams wrote: > > > > On Thu, Mar 21, 2019 at 7:27 AM Roberto Sassu <roberto.sassu@huawei.com> wrote: > > > > > > > > > > On 3/21/2019 2:54 PM, Jarkko Sakkinen wrote: > > > > > > On Mon, Mar 18, 2019 at 04:45:13PM -0700, Dan Williams wrote: > > > > > > > Rather than fail initialization of the trusted.ko module, arrange for > > > > > > > the module to load, but rely on trusted_instantiate() to fail > > > > > > > trusted-key operations. > > > > > > > > > > > > > > Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...") > > > > > > > Cc: Roberto Sassu <roberto.sassu@huawei.com> > > > > > > > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > > > > > > Cc: James Bottomley <jejb@linux.ibm.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> > > > > > > > > > > > > It should check for chip in each function that uses TPM now that > > > > > > the code does not rely on default chip. Otherwise, the semantics > > > > > > are kind of inconsistent. > > > > > > > > > > If no other TPM function can be used before a successful key > > > > > instantiate, checking for a chip only in trusted_instantiate() seems > > > > > sufficient. Then, the same chip will be used by all TPM functions until > > > > > module unloading, since we incremented the reference count. > > > > > > > > > > I would suggest to move the tpm_default_chip() and init_digests() calls > > > > > to trusted_instantiate() to restore the old behavior of init_trusted(). > > > > > > > > > > trusted_instantiate() should look like: > > > > > --- > > > > > if (!chip) { > > > > > chip = tpm_default_chip(); > > > > > if (!chip) > > > > > return -ENODEV; > > > > > } > > > > > > > > > > if (!digests) { > > > > > ret = init_digests(); > > > > > if (ret < 0) > > > > > return ret; > > > > > } > > > > > > > > This patch already achieves that because tpm_find_get_ops() will fail > > > > and cause tpm_is_tpm2() to return NULL. > > > > > > In addition, the changes I proposed would allow users to create trusted > > > keys if a TPM is added later. CONFIG_TRUSTED_KEYS=y and > > > CONFIG_TCG_TPM=m is a valid configuration. > > > > > > Jarkko, Dan's patch seems sufficient to fix the issue. He could include > > > the changes I proposed in his patch. What is your opinion? > > > > Agreed. > > What changes? > > Robert, please feel free to re-author the proposed patch however you > see fit, I just want whatever will get libnvdimm operational again in > the shortest amount of time. I've decided go with a patch of least innovation i.e. one that simply reverts the old behavior. Sending patch soon. /Jarkko ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-25 14:12 ` Jarkko Sakkinen 0 siblings, 0 replies; 50+ messages in thread From: Jarkko Sakkinen @ 2019-03-25 14:12 UTC (permalink / raw) To: Dan Williams Cc: Silviu Vlasceanu, linux-nvdimm, James Bottomley, Roberto Sassu, Linux Kernel Mailing List, Mimi Zohar, David Howells, keyrings On Fri, Mar 22, 2019 at 08:24:01AM -0700, Dan Williams wrote: > On Fri, Mar 22, 2019 at 3:12 AM Jarkko Sakkinen > <jarkko.sakkinen@linux.intel.com> wrote: > > > > On Thu, Mar 21, 2019 at 06:45:47PM +0100, Roberto Sassu wrote: > > > On 3/21/2019 5:30 PM, Dan Williams wrote: > > > > On Thu, Mar 21, 2019 at 7:27 AM Roberto Sassu <roberto.sassu@huawei.com> wrote: > > > > > > > > > > On 3/21/2019 2:54 PM, Jarkko Sakkinen wrote: > > > > > > On Mon, Mar 18, 2019 at 04:45:13PM -0700, Dan Williams wrote: > > > > > > > Rather than fail initialization of the trusted.ko module, arrange for > > > > > > > the module to load, but rely on trusted_instantiate() to fail > > > > > > > trusted-key operations. > > > > > > > > > > > > > > Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...") > > > > > > > Cc: Roberto Sassu <roberto.sassu@huawei.com> > > > > > > > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > > > > > > Cc: James Bottomley <jejb@linux.ibm.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> > > > > > > > > > > > > It should check for chip in each function that uses TPM now that > > > > > > the code does not rely on default chip. Otherwise, the semantics > > > > > > are kind of inconsistent. > > > > > > > > > > If no other TPM function can be used before a successful key > > > > > instantiate, checking for a chip only in trusted_instantiate() seems > > > > > sufficient. Then, the same chip will be used by all TPM functions until > > > > > module unloading, since we incremented the reference count. > > > > > > > > > > I would suggest to move the tpm_default_chip() and init_digests() calls > > > > > to trusted_instantiate() to restore the old behavior of init_trusted(). > > > > > > > > > > trusted_instantiate() should look like: > > > > > --- > > > > > if (!chip) { > > > > > chip = tpm_default_chip(); > > > > > if (!chip) > > > > > return -ENODEV; > > > > > } > > > > > > > > > > if (!digests) { > > > > > ret = init_digests(); > > > > > if (ret < 0) > > > > > return ret; > > > > > } > > > > > > > > This patch already achieves that because tpm_find_get_ops() will fail > > > > and cause tpm_is_tpm2() to return NULL. > > > > > > In addition, the changes I proposed would allow users to create trusted > > > keys if a TPM is added later. CONFIG_TRUSTED_KEYS=y and > > > CONFIG_TCG_TPM=m is a valid configuration. > > > > > > Jarkko, Dan's patch seems sufficient to fix the issue. He could include > > > the changes I proposed in his patch. What is your opinion? > > > > Agreed. > > What changes? > > Robert, please feel free to re-author the proposed patch however you > see fit, I just want whatever will get libnvdimm operational again in > the shortest amount of time. I've decided go with a patch of least innovation i.e. one that simply reverts the old behavior. Sending patch soon. /Jarkko ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM 2019-03-25 14:12 ` Jarkko Sakkinen (?) @ 2019-03-25 14:50 ` Jarkko Sakkinen -1 siblings, 0 replies; 50+ messages in thread From: Jarkko Sakkinen @ 2019-03-25 14:50 UTC (permalink / raw) To: Dan Williams Cc: Silviu Vlasceanu, linux-nvdimm, James Bottomley, Roberto Sassu, Linux Kernel Mailing List, Mimi Zohar, David Howells, keyrings On Mon, Mar 25, 2019 at 04:12:23PM +0200, Jarkko Sakkinen wrote: > > Robert, please feel free to re-author the proposed patch however you > > see fit, I just want whatever will get libnvdimm operational again in > > the shortest amount of time. > > I've decided go with a patch of least innovation i.e. one that simply reverts > the old behavior. Sending patch soon. I.e. https://www.lkml.org/lkml/2019/3/25/674 I think fixing the actual dependency issue would be unorthodox at this point of release cycle as adding dependencies was not the root cause for the bug even if existing somewhat unorthodox were. Better to just the problem by reverting the semantics near the same as in v5.0 and take this dependency discussion "offline" for the release work... /Jarkko _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-25 14:50 ` Jarkko Sakkinen 0 siblings, 0 replies; 50+ messages in thread From: Jarkko Sakkinen @ 2019-03-25 14:50 UTC (permalink / raw) To: Dan Williams Cc: Roberto Sassu, James Bottomley, Mimi Zohar, David Howells, keyrings, linux-nvdimm, Linux Kernel Mailing List, Silviu Vlasceanu On Mon, Mar 25, 2019 at 04:12:23PM +0200, Jarkko Sakkinen wrote: > > Robert, please feel free to re-author the proposed patch however you > > see fit, I just want whatever will get libnvdimm operational again in > > the shortest amount of time. > > I've decided go with a patch of least innovation i.e. one that simply reverts > the old behavior. Sending patch soon. I.e. https://www.lkml.org/lkml/2019/3/25/674 I think fixing the actual dependency issue would be unorthodox at this point of release cycle as adding dependencies was not the root cause for the bug even if existing somewhat unorthodox were. Better to just the problem by reverting the semantics near the same as in v5.0 and take this dependency discussion "offline" for the release work... /Jarkko ^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM @ 2019-03-25 14:50 ` Jarkko Sakkinen 0 siblings, 0 replies; 50+ messages in thread From: Jarkko Sakkinen @ 2019-03-25 14:50 UTC (permalink / raw) To: Dan Williams Cc: Silviu Vlasceanu, linux-nvdimm, James Bottomley, Roberto Sassu, Linux Kernel Mailing List, Mimi Zohar, David Howells, keyrings On Mon, Mar 25, 2019 at 04:12:23PM +0200, Jarkko Sakkinen wrote: > > Robert, please feel free to re-author the proposed patch however you > > see fit, I just want whatever will get libnvdimm operational again in > > the shortest amount of time. > > I've decided go with a patch of least innovation i.e. one that simply reverts > the old behavior. Sending patch soon. I.e. https://www.lkml.org/lkml/2019/3/25/674 I think fixing the actual dependency issue would be unorthodox at this point of release cycle as adding dependencies was not the root cause for the bug even if existing somewhat unorthodox were. Better to just the problem by reverting the semantics near the same as in v5.0 and take this dependency discussion "offline" for the release work... /Jarkko ^ permalink raw reply [flat|nested] 50+ messages in thread
end of thread, other threads:[~2019-03-25 14:50 UTC | newest] Thread overview: 50+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-18 23:45 [PATCH] security/keys/trusted: Allow operation without hardware TPM Dan Williams 2019-03-18 23:45 ` Dan Williams 2019-03-18 23:45 ` Dan Williams [not found] ` <155295271345.1945351.6465460744078693578.stgit-p8uTFz9XbKj2zm6wflaqv1nYeNYlB/vhral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2019-03-19 0:24 ` James Bottomley 2019-03-19 0:24 ` James Bottomley 2019-03-19 0:24 ` James Bottomley 2019-03-19 0:30 ` Dan Williams 2019-03-19 0:30 ` Dan Williams 2019-03-19 0:30 ` Dan Williams [not found] ` <CAA9_cmcOD2zPaaNbkYAaH5DRDRAebPkW+hwPA0zPKY4kU8R-rg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2019-03-19 0:56 ` James Bottomley 2019-03-19 0:56 ` James Bottomley 2019-03-19 0:56 ` James Bottomley 2019-03-19 1:34 ` Dan Williams 2019-03-19 1:34 ` Dan Williams 2019-03-19 1:34 ` Dan Williams 2019-03-20 1:55 ` Dan Williams 2019-03-20 1:55 ` Dan Williams 2019-03-20 1:55 ` Dan Williams [not found] ` <CAPcyv4hXKQcdLnKG6rPNOJr2wjq31uCCr+16S5Yu6S5A_5UGrA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2019-03-20 2:19 ` James Bottomley 2019-03-20 2:19 ` James Bottomley 2019-03-20 2:19 ` James Bottomley 2019-03-19 22:56 ` Mimi Zohar 2019-03-19 22:56 ` Mimi Zohar 2019-03-19 22:56 ` Mimi Zohar 2019-03-19 23:01 ` Dan Williams 2019-03-19 23:01 ` Dan Williams 2019-03-19 23:01 ` Dan Williams 2019-03-21 13:54 ` Jarkko Sakkinen 2019-03-21 13:54 ` Jarkko Sakkinen 2019-03-21 13:54 ` Jarkko Sakkinen 2019-03-21 14:26 ` Roberto Sassu 2019-03-21 14:26 ` Roberto Sassu 2019-03-21 14:26 ` Roberto Sassu 2019-03-21 16:30 ` Dan Williams 2019-03-21 16:30 ` Dan Williams 2019-03-21 16:30 ` Dan Williams [not found] ` <CAPcyv4gc_KWedC12bTcX24KwjKiBCF=yvouRNzCbJKaLrgJdDg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2019-03-21 17:45 ` Roberto Sassu 2019-03-21 17:45 ` Roberto Sassu 2019-03-21 17:45 ` Roberto Sassu 2019-03-22 10:12 ` Jarkko Sakkinen 2019-03-22 10:12 ` Jarkko Sakkinen 2019-03-22 15:24 ` Dan Williams 2019-03-22 15:24 ` Dan Williams 2019-03-22 15:24 ` Dan Williams 2019-03-25 14:12 ` Jarkko Sakkinen 2019-03-25 14:12 ` Jarkko Sakkinen 2019-03-25 14:12 ` Jarkko Sakkinen 2019-03-25 14:50 ` Jarkko Sakkinen 2019-03-25 14:50 ` Jarkko Sakkinen 2019-03-25 14:50 ` 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.