diff mbox series

[v5,3/3] KEYS: trusted: Reserve TPM for seal and unseal operations

Message ID 20210128235621.127925-4-jarkko@kernel.org
State Accepted
Commit 8c657a0590de585b1115847c17b34a58025f2f4b
Headers show
Series [v5,1/3] KEYS: trusted: Fix incorrect handling of tpm_get_random() | expand

Commit Message

Jarkko Sakkinen Jan. 28, 2021, 11:56 p.m. UTC 
From: Jarkko Sakkinen <jarkko@kernel.org>

When TPM 2.0 trusted keys code was moved to the trusted keys subsystem,
the operations were unwrapped from tpm_try_get_ops() and tpm_put_ops(),
which are used to take temporarily the ownership of the TPM chip. The
ownership is only taken inside tpm_send(), but this is not sufficient,
as in the key load TPM2_CC_LOAD, TPM2_CC_UNSEAL and TPM2_FLUSH_CONTEXT
need to be done as a one single atom.

Take the TPM chip ownership before sending anything with
tpm_try_get_ops() and tpm_put_ops(), and use tpm_transmit_cmd() to send
TPM commands instead of tpm_send(), reverting back to the old behaviour.

Fixes: 2e19e10131a0 ("KEYS: trusted: Move TPM2 trusted keys code")
Reported-by: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: stable@vger.kernel.org
Cc: David Howells <dhowells@redhat.com>
Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: Sumit Garg <sumit.garg@linaro.org>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 drivers/char/tpm/tpm.h                    |  4 ----
 include/linux/tpm.h                       |  5 ++++-
 security/keys/trusted-keys/trusted_tpm2.c | 24 ++++++++++++++++++-----
 3 files changed, 23 insertions(+), 10 deletions(-)

Comments

Jarkko Sakkinen Jan. 30, 2021, 9:27 p.m. UTC | #1 
On Fri, 2021-01-29 at 14:44 +0530, Sumit Garg wrote:
> On Fri, 29 Jan 2021 at 05:26, <jarkko@kernel.org> wrote:

> > 

> > From: Jarkko Sakkinen <jarkko@kernel.org>

> > 

> > When TPM 2.0 trusted keys code was moved to the trusted keys subsystem,

> > the operations were unwrapped from tpm_try_get_ops() and tpm_put_ops(),

> > which are used to take temporarily the ownership of the TPM chip. The

> > ownership is only taken inside tpm_send(), but this is not sufficient,

> > as in the key load TPM2_CC_LOAD, TPM2_CC_UNSEAL and TPM2_FLUSH_CONTEXT

> > need to be done as a one single atom.

> > 

> > Take the TPM chip ownership before sending anything with

> > tpm_try_get_ops() and tpm_put_ops(), and use tpm_transmit_cmd() to send

> > TPM commands instead of tpm_send(), reverting back to the old behaviour.

> > 

> > Fixes: 2e19e10131a0 ("KEYS: trusted: Move TPM2 trusted keys code")

> > Reported-by: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>

> > Cc: stable@vger.kernel.org

> > Cc: David Howells <dhowells@redhat.com>

> > Cc: Mimi Zohar <zohar@linux.ibm.com>

> > Cc: Sumit Garg <sumit.garg@linaro.org>

> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>

> > ---

> >  drivers/char/tpm/tpm.h                    |  4 ----

> >  include/linux/tpm.h                       |  5 ++++-

> >  security/keys/trusted-keys/trusted_tpm2.c | 24 ++++++++++++++++++-----

> >  3 files changed, 23 insertions(+), 10 deletions(-)

> > 

> 

> Acked-by: Sumit Garg <sumit.garg@linaro.org>


Thanks.

/Jarkko
Jarkko Sakkinen Jan. 30, 2021, 9:28 p.m. UTC | #2 
On Fri, 2021-01-29 at 08:58 -0500, Mimi Zohar wrote:
> On Fri, 2021-01-29 at 01:56 +0200, jarkko@kernel.org wrote:

> > From: Jarkko Sakkinen <jarkko@kernel.org>

> > 

> > When TPM 2.0 trusted keys code was moved to the trusted keys subsystem,

> > the operations were unwrapped from tpm_try_get_ops() and tpm_put_ops(),

> > which are used to take temporarily the ownership of the TPM chip. The

> > ownership is only taken inside tpm_send(), but this is not sufficient,

> > as in the key load TPM2_CC_LOAD, TPM2_CC_UNSEAL and TPM2_FLUSH_CONTEXT

> > need to be done as a one single atom.

> > 

> > Take the TPM chip ownership before sending anything with

> > tpm_try_get_ops() and tpm_put_ops(), and use tpm_transmit_cmd() to send

> > TPM commands instead of tpm_send(), reverting back to the old behaviour.

> > 

> > Fixes: 2e19e10131a0 ("KEYS: trusted: Move TPM2 trusted keys code")

> > Reported-by: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>

> > Cc: stable@vger.kernel.org

> > Cc: David Howells <dhowells@redhat.com>

> > Cc: Mimi Zohar <zohar@linux.ibm.com>

> > Cc: Sumit Garg <sumit.garg@linaro.org>

> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>

> 

> Tested-by: Mimi Zohar <zohar@linux.ibm.com> (on TPM 1.2 & PTT, discrete

> TPM 2.0)


Thanks, is it OK to apply the whole series?

/Jarkko
Mimi Zohar Jan. 31, 2021, 12:52 p.m. UTC | #3 
On Sat, 2021-01-30 at 23:28 +0200, Jarkko Sakkinen wrote:
> On Fri, 2021-01-29 at 08:58 -0500, Mimi Zohar wrote:

> > On Fri, 2021-01-29 at 01:56 +0200, jarkko@kernel.org wrote:

> > > From: Jarkko Sakkinen <jarkko@kernel.org>

> > > 

> > > When TPM 2.0 trusted keys code was moved to the trusted keys subsystem,

> > > the operations were unwrapped from tpm_try_get_ops() and tpm_put_ops(),

> > > which are used to take temporarily the ownership of the TPM chip. The

> > > ownership is only taken inside tpm_send(), but this is not sufficient,

> > > as in the key load TPM2_CC_LOAD, TPM2_CC_UNSEAL and TPM2_FLUSH_CONTEXT

> > > need to be done as a one single atom.

> > > 

> > > Take the TPM chip ownership before sending anything with

> > > tpm_try_get_ops() and tpm_put_ops(), and use tpm_transmit_cmd() to send

> > > TPM commands instead of tpm_send(), reverting back to the old behaviour.

> > > 

> > > Fixes: 2e19e10131a0 ("KEYS: trusted: Move TPM2 trusted keys code")

> > > Reported-by: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>

> > > Cc: stable@vger.kernel.org

> > > Cc: David Howells <dhowells@redhat.com>

> > > Cc: Mimi Zohar <zohar@linux.ibm.com>

> > > Cc: Sumit Garg <sumit.garg@linaro.org>

> > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>

> > 

> > Tested-by: Mimi Zohar <zohar@linux.ibm.com> (on TPM 1.2 & PTT, discrete

> > TPM 2.0)

> 

> Thanks, is it OK to apply the whole series?


Yes.  The testing was with the entire patch set, but I didn't
explicitly test each change.  For the other two patches, please add my
Reviewed-by.

Mimi
Jarkko Sakkinen Feb. 2, 2021, 4:29 p.m. UTC | #4 
On Sun, Jan 31, 2021 at 07:52:42AM -0500, Mimi Zohar wrote:
> On Sat, 2021-01-30 at 23:28 +0200, Jarkko Sakkinen wrote:

> > On Fri, 2021-01-29 at 08:58 -0500, Mimi Zohar wrote:

> > > On Fri, 2021-01-29 at 01:56 +0200, jarkko@kernel.org wrote:

> > > > From: Jarkko Sakkinen <jarkko@kernel.org>

> > > > 

> > > > When TPM 2.0 trusted keys code was moved to the trusted keys subsystem,

> > > > the operations were unwrapped from tpm_try_get_ops() and tpm_put_ops(),

> > > > which are used to take temporarily the ownership of the TPM chip. The

> > > > ownership is only taken inside tpm_send(), but this is not sufficient,

> > > > as in the key load TPM2_CC_LOAD, TPM2_CC_UNSEAL and TPM2_FLUSH_CONTEXT

> > > > need to be done as a one single atom.

> > > > 

> > > > Take the TPM chip ownership before sending anything with

> > > > tpm_try_get_ops() and tpm_put_ops(), and use tpm_transmit_cmd() to send

> > > > TPM commands instead of tpm_send(), reverting back to the old behaviour.

> > > > 

> > > > Fixes: 2e19e10131a0 ("KEYS: trusted: Move TPM2 trusted keys code")

> > > > Reported-by: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>

> > > > Cc: stable@vger.kernel.org

> > > > Cc: David Howells <dhowells@redhat.com>

> > > > Cc: Mimi Zohar <zohar@linux.ibm.com>

> > > > Cc: Sumit Garg <sumit.garg@linaro.org>

> > > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>

> > > 

> > > Tested-by: Mimi Zohar <zohar@linux.ibm.com> (on TPM 1.2 & PTT, discrete

> > > TPM 2.0)

> > 

> > Thanks, is it OK to apply the whole series?

> 

> Yes.  The testing was with the entire patch set, but I didn't

> explicitly test each change.  For the other two patches, please add my

> Reviewed-by.

> 

> Mimi


Thank you. I will do that.

/Jarkko
diff mbox series

Patch

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 947d1db0a5cc..283f78211c3a 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -164,8 +164,6 @@  extern const struct file_operations tpmrm_fops;
 extern struct idr dev_nums_idr;
 
 ssize_t tpm_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz);
-ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_buf *buf,
-			 size_t min_rsp_body_length, const char *desc);
 int tpm_get_timeouts(struct tpm_chip *);
 int tpm_auto_startup(struct tpm_chip *chip);
 
@@ -194,8 +192,6 @@  static inline void tpm_msleep(unsigned int delay_msec)
 int tpm_chip_start(struct tpm_chip *chip);
 void tpm_chip_stop(struct tpm_chip *chip);
 struct tpm_chip *tpm_find_get_ops(struct tpm_chip *chip);
-__must_check int tpm_try_get_ops(struct tpm_chip *chip);
-void tpm_put_ops(struct tpm_chip *chip);
 
 struct tpm_chip *tpm_chip_alloc(struct device *dev,
 				const struct tpm_class_ops *ops);
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index ae2482510f8c..543aa3b1dedc 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -404,6 +404,10 @@  static inline u32 tpm2_rc_value(u32 rc)
 #if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)
 
 extern int tpm_is_tpm2(struct tpm_chip *chip);
+extern __must_check int tpm_try_get_ops(struct tpm_chip *chip);
+extern void tpm_put_ops(struct tpm_chip *chip);
+extern ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_buf *buf,
+				size_t min_rsp_body_length, const char *desc);
 extern int tpm_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
 			struct tpm_digest *digest);
 extern int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
@@ -417,7 +421,6 @@  static inline int tpm_is_tpm2(struct tpm_chip *chip)
 {
 	return -ENODEV;
 }
-
 static inline int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx,
 			       struct tpm_digest *digest)
 {
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 08ec7f48f01d..c87c4df8703d 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -79,10 +79,16 @@  int tpm2_seal_trusted(struct tpm_chip *chip,
 	if (i == ARRAY_SIZE(tpm2_hash_map))
 		return -EINVAL;
 
-	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
+	rc = tpm_try_get_ops(chip);
 	if (rc)
 		return rc;
 
+	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
+	if (rc) {
+		tpm_put_ops(chip);
+		return rc;
+	}
+
 	tpm_buf_append_u32(&buf, options->keyhandle);
 	tpm2_buf_append_auth(&buf, TPM2_RS_PW,
 			     NULL /* nonce */, 0,
@@ -130,7 +136,7 @@  int tpm2_seal_trusted(struct tpm_chip *chip,
 		goto out;
 	}
 
-	rc = tpm_send(chip, buf.data, tpm_buf_length(&buf));
+	rc = tpm_transmit_cmd(chip, &buf, 4, "sealing data");
 	if (rc)
 		goto out;
 
@@ -157,6 +163,7 @@  int tpm2_seal_trusted(struct tpm_chip *chip,
 			rc = -EPERM;
 	}
 
+	tpm_put_ops(chip);
 	return rc;
 }
 
@@ -211,7 +218,7 @@  static int tpm2_load_cmd(struct tpm_chip *chip,
 		goto out;
 	}
 
-	rc = tpm_send(chip, buf.data, tpm_buf_length(&buf));
+	rc = tpm_transmit_cmd(chip, &buf, 4, "loading blob");
 	if (!rc)
 		*blob_handle = be32_to_cpup(
 			(__be32 *) &buf.data[TPM_HEADER_SIZE]);
@@ -260,7 +267,7 @@  static int tpm2_unseal_cmd(struct tpm_chip *chip,
 			     options->blobauth /* hmac */,
 			     TPM_DIGEST_SIZE);
 
-	rc = tpm_send(chip, buf.data, tpm_buf_length(&buf));
+	rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing");
 	if (rc > 0)
 		rc = -EPERM;
 
@@ -304,12 +311,19 @@  int tpm2_unseal_trusted(struct tpm_chip *chip,
 	u32 blob_handle;
 	int rc;
 
-	rc = tpm2_load_cmd(chip, payload, options, &blob_handle);
+	rc = tpm_try_get_ops(chip);
 	if (rc)
 		return rc;
 
+	rc = tpm2_load_cmd(chip, payload, options, &blob_handle);
+	if (rc)
+		goto out;
+
 	rc = tpm2_unseal_cmd(chip, payload, options, blob_handle);
 	tpm2_flush_context(chip, blob_handle);
 
+out:
+	tpm_put_ops(chip);
+
 	return rc;
 }