All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
To: Peter Huewe <peterhuewe@gmx.de>
Cc: linux-security-module@vger.kernel.org,
	Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	stable@vger.kernel.org, Marcel Selhorst <tpmdd@selhorst.net>,
	Jason Gunthorpe <jgunthorpe@obsidianresearch.com>,
	tpmdd-devel@lists.sourceforge.net (moderated list:TPM DEVICE
	DRIVER), linux-kernel@vger.kernel.org (open list)
Subject: [PATCH] tpm: fix a race condition tpm2_unseal_trusted()
Date: Tue, 23 Aug 2016 20:57:22 -0400	[thread overview]
Message-ID: <1472000243-7088-1-git-send-email-jarkko.sakkinen@linux.intel.com> (raw)

Unseal and load operations should be done as an atomic operation. This
commit introduces unlocked tpm_transmit() so that tpm2_unseal_trusted()
can do the locking by itself.

v2: Introduced an unlocked unseal operation instead of changing locking
    strategy in order to make less intrusive bug fix and thus more
    backportable.

v3: Have also separate __tpm_transmit() that takes 'flags' in order to
    better localize the bug fix and make it easier to backport.

v4: Cleaned up the control flow in tpm2_unseal_trusted. Added the
    missing 'Fixes' line.

Fixes: 0fe5480303a1 ("keys, trusted: seal/unseal with TPM 2.0 chips")
CC: stable@vger.kernel.org
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-interface.c | 16 +++++++++-------
 drivers/char/tpm/tpm.h           | 25 +++++++++++++++++++++----
 drivers/char/tpm/tpm2-cmd.c      | 12 ++++++++----
 3 files changed, 38 insertions(+), 15 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 43ef0ef..80e702a 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -330,8 +330,8 @@ EXPORT_SYMBOL_GPL(tpm_calc_ordinal_duration);
 /*
  * Internal kernel interface to transmit TPM commands
  */
-ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
-		     size_t bufsiz)
+ssize_t __tpm_transmit(struct tpm_chip *chip, const char *buf,
+		       size_t bufsiz, unsigned int flags)
 {
 	ssize_t rc;
 	u32 count, ordinal;
@@ -350,7 +350,8 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
 		return -E2BIG;
 	}
 
-	mutex_lock(&chip->tpm_mutex);
+	if (flags & TPM_TRANSMIT_LOCK)
+		mutex_lock(&chip->tpm_mutex);
 
 	rc = chip->ops->send(chip, (u8 *) buf, count);
 	if (rc < 0) {
@@ -393,20 +394,21 @@ out_recv:
 		dev_err(&chip->dev,
 			"tpm_transmit: tpm_recv: error %zd\n", rc);
 out:
-	mutex_unlock(&chip->tpm_mutex);
+	if (flags & TPM_TRANSMIT_LOCK)
+		mutex_unlock(&chip->tpm_mutex);
 	return rc;
 }
 
 #define TPM_DIGEST_SIZE 20
 #define TPM_RET_CODE_IDX 6
 
-ssize_t tpm_transmit_cmd(struct tpm_chip *chip, void *cmd,
-			 int len, const char *desc)
+ssize_t __tpm_transmit_cmd(struct tpm_chip *chip, void *cmd,
+			   int len, const char *desc, unsigned int flags)
 {
 	struct tpm_output_header *header;
 	int err;
 
-	len = tpm_transmit(chip, (u8 *) cmd, len);
+	len = __tpm_transmit(chip, cmd, len, flags);
 	if (len <  0)
 		return len;
 	else if (len < TPM_HEADER_SIZE)
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 6e002c4..0a4abf0 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -476,12 +476,29 @@ extern dev_t tpm_devt;
 extern const struct file_operations tpm_fops;
 extern struct idr dev_nums_idr;
 
+enum tpm_transmit_flags {
+	TPM_TRANSMIT_LOCK,
+};
+
+ssize_t __tpm_transmit(struct tpm_chip *chip, const char *buf,
+		       size_t bufsiz, unsigned int flags);
+ssize_t __tpm_transmit_cmd(struct tpm_chip *chip, void *cmd, int len,
+			   const char *desc, unsigned int flags);
+
+static inline ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
+				   size_t bufsiz)
+{
+	return __tpm_transmit(chip, buf, bufsiz, TPM_TRANSMIT_LOCK);
+}
+
+static inline ssize_t tpm_transmit_cmd(struct tpm_chip *chip, void *cmd,
+				       int len, const char *desc)
+{
+	return __tpm_transmit_cmd(chip, cmd, len, desc, TPM_TRANSMIT_LOCK);
+}
+
 ssize_t tpm_getcap(struct tpm_chip *chip, __be32 subcap_id, cap_t *cap,
 		   const char *desc);
-ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
-		     size_t bufsiz);
-ssize_t tpm_transmit_cmd(struct tpm_chip *chip, void *cmd, int len,
-			 const char *desc);
 int tpm_get_timeouts(struct tpm_chip *chip);
 int tpm1_auto_startup(struct tpm_chip *chip);
 int tpm_do_selftest(struct tpm_chip *chip);
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 499f405..a2a0314 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -576,7 +576,7 @@ static int tpm2_load(struct tpm_chip *chip,
 		goto out;
 	}
 
-	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "loading blob");
+	rc = __tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "loading blob", 0);
 	if (!rc)
 		*blob_handle = be32_to_cpup(
 			(__be32 *) &buf.data[TPM_HEADER_SIZE]);
@@ -604,7 +604,8 @@ static void tpm2_flush_context(struct tpm_chip *chip, u32 handle)
 
 	tpm_buf_append_u32(&buf, handle);
 
-	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "flushing context");
+	rc = __tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "flushing context",
+				0);
 	if (rc)
 		dev_warn(&chip->dev, "0x%08x was not flushed, rc=%d\n", handle,
 			 rc);
@@ -635,7 +636,7 @@ static int tpm2_unseal(struct tpm_chip *chip,
 			     options->blobauth /* hmac */,
 			     TPM_DIGEST_SIZE);
 
-	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "unsealing");
+	rc = __tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "unsealing", 0);
 	if (rc > 0)
 		rc = -EPERM;
 
@@ -668,14 +669,17 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
 	u32 blob_handle;
 	int rc;
 
+	mutex_lock(&chip->tpm_mutex);
 	rc = tpm2_load(chip, payload, options, &blob_handle);
 	if (rc)
-		return rc;
+		goto out;
 
 	rc = tpm2_unseal(chip, payload, options, blob_handle);
 
 	tpm2_flush_context(chip, blob_handle);
 
+out:
+	mutex_unlock(&chip->tpm_mutex);
 	return rc;
 }
 
-- 
2.7.4

WARNING: multiple messages have this Message-ID (diff)
From: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
To: Peter Huewe <peterhuewe-Mmb7MZpHnFY@public.gmane.org>
Cc: open list <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	"moderated list:TPM DEVICE DRIVER"
	<tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
Subject: [PATCH] tpm: fix a race condition tpm2_unseal_trusted()
Date: Tue, 23 Aug 2016 20:57:22 -0400	[thread overview]
Message-ID: <1472000243-7088-1-git-send-email-jarkko.sakkinen@linux.intel.com> (raw)

Unseal and load operations should be done as an atomic operation. This
commit introduces unlocked tpm_transmit() so that tpm2_unseal_trusted()
can do the locking by itself.

v2: Introduced an unlocked unseal operation instead of changing locking
    strategy in order to make less intrusive bug fix and thus more
    backportable.

v3: Have also separate __tpm_transmit() that takes 'flags' in order to
    better localize the bug fix and make it easier to backport.

v4: Cleaned up the control flow in tpm2_unseal_trusted. Added the
    missing 'Fixes' line.

Fixes: 0fe5480303a1 ("keys, trusted: seal/unseal with TPM 2.0 chips")
CC: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/char/tpm/tpm-interface.c | 16 +++++++++-------
 drivers/char/tpm/tpm.h           | 25 +++++++++++++++++++++----
 drivers/char/tpm/tpm2-cmd.c      | 12 ++++++++----
 3 files changed, 38 insertions(+), 15 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 43ef0ef..80e702a 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -330,8 +330,8 @@ EXPORT_SYMBOL_GPL(tpm_calc_ordinal_duration);
 /*
  * Internal kernel interface to transmit TPM commands
  */
-ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
-		     size_t bufsiz)
+ssize_t __tpm_transmit(struct tpm_chip *chip, const char *buf,
+		       size_t bufsiz, unsigned int flags)
 {
 	ssize_t rc;
 	u32 count, ordinal;
@@ -350,7 +350,8 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
 		return -E2BIG;
 	}
 
-	mutex_lock(&chip->tpm_mutex);
+	if (flags & TPM_TRANSMIT_LOCK)
+		mutex_lock(&chip->tpm_mutex);
 
 	rc = chip->ops->send(chip, (u8 *) buf, count);
 	if (rc < 0) {
@@ -393,20 +394,21 @@ out_recv:
 		dev_err(&chip->dev,
 			"tpm_transmit: tpm_recv: error %zd\n", rc);
 out:
-	mutex_unlock(&chip->tpm_mutex);
+	if (flags & TPM_TRANSMIT_LOCK)
+		mutex_unlock(&chip->tpm_mutex);
 	return rc;
 }
 
 #define TPM_DIGEST_SIZE 20
 #define TPM_RET_CODE_IDX 6
 
-ssize_t tpm_transmit_cmd(struct tpm_chip *chip, void *cmd,
-			 int len, const char *desc)
+ssize_t __tpm_transmit_cmd(struct tpm_chip *chip, void *cmd,
+			   int len, const char *desc, unsigned int flags)
 {
 	struct tpm_output_header *header;
 	int err;
 
-	len = tpm_transmit(chip, (u8 *) cmd, len);
+	len = __tpm_transmit(chip, cmd, len, flags);
 	if (len <  0)
 		return len;
 	else if (len < TPM_HEADER_SIZE)
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 6e002c4..0a4abf0 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -476,12 +476,29 @@ extern dev_t tpm_devt;
 extern const struct file_operations tpm_fops;
 extern struct idr dev_nums_idr;
 
+enum tpm_transmit_flags {
+	TPM_TRANSMIT_LOCK,
+};
+
+ssize_t __tpm_transmit(struct tpm_chip *chip, const char *buf,
+		       size_t bufsiz, unsigned int flags);
+ssize_t __tpm_transmit_cmd(struct tpm_chip *chip, void *cmd, int len,
+			   const char *desc, unsigned int flags);
+
+static inline ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
+				   size_t bufsiz)
+{
+	return __tpm_transmit(chip, buf, bufsiz, TPM_TRANSMIT_LOCK);
+}
+
+static inline ssize_t tpm_transmit_cmd(struct tpm_chip *chip, void *cmd,
+				       int len, const char *desc)
+{
+	return __tpm_transmit_cmd(chip, cmd, len, desc, TPM_TRANSMIT_LOCK);
+}
+
 ssize_t tpm_getcap(struct tpm_chip *chip, __be32 subcap_id, cap_t *cap,
 		   const char *desc);
-ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
-		     size_t bufsiz);
-ssize_t tpm_transmit_cmd(struct tpm_chip *chip, void *cmd, int len,
-			 const char *desc);
 int tpm_get_timeouts(struct tpm_chip *chip);
 int tpm1_auto_startup(struct tpm_chip *chip);
 int tpm_do_selftest(struct tpm_chip *chip);
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 499f405..a2a0314 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -576,7 +576,7 @@ static int tpm2_load(struct tpm_chip *chip,
 		goto out;
 	}
 
-	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "loading blob");
+	rc = __tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "loading blob", 0);
 	if (!rc)
 		*blob_handle = be32_to_cpup(
 			(__be32 *) &buf.data[TPM_HEADER_SIZE]);
@@ -604,7 +604,8 @@ static void tpm2_flush_context(struct tpm_chip *chip, u32 handle)
 
 	tpm_buf_append_u32(&buf, handle);
 
-	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "flushing context");
+	rc = __tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "flushing context",
+				0);
 	if (rc)
 		dev_warn(&chip->dev, "0x%08x was not flushed, rc=%d\n", handle,
 			 rc);
@@ -635,7 +636,7 @@ static int tpm2_unseal(struct tpm_chip *chip,
 			     options->blobauth /* hmac */,
 			     TPM_DIGEST_SIZE);
 
-	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "unsealing");
+	rc = __tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "unsealing", 0);
 	if (rc > 0)
 		rc = -EPERM;
 
@@ -668,14 +669,17 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
 	u32 blob_handle;
 	int rc;
 
+	mutex_lock(&chip->tpm_mutex);
 	rc = tpm2_load(chip, payload, options, &blob_handle);
 	if (rc)
-		return rc;
+		goto out;
 
 	rc = tpm2_unseal(chip, payload, options, blob_handle);
 
 	tpm2_flush_context(chip, blob_handle);
 
+out:
+	mutex_unlock(&chip->tpm_mutex);
 	return rc;
 }
 
-- 
2.7.4


------------------------------------------------------------------------------

             reply	other threads:[~2016-08-24  0:57 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-24  0:57 Jarkko Sakkinen [this message]
2016-08-24  0:57 ` [PATCH] tpm: fix a race condition tpm2_unseal_trusted() Jarkko Sakkinen
2016-08-24  1:32 ` Jarkko Sakkinen
2016-08-24  1:32   ` Jarkko Sakkinen
2016-08-24  1:32   ` Jarkko Sakkinen
2016-08-25 18:30 ` Jason Gunthorpe
2016-08-25 18:30   ` Jason Gunthorpe
2016-08-25 18:30   ` Jason Gunthorpe
2016-08-25 21:06   ` Jarkko Sakkinen
2016-08-25 21:06     ` Jarkko Sakkinen
2016-08-25 21:06     ` Jarkko Sakkinen
2016-08-25 21:09     ` Jason Gunthorpe
2016-08-25 21:09       ` Jason Gunthorpe
2016-08-25 21:09       ` Jason Gunthorpe
  -- strict thread matches above, loose matches on Subject: below --
2016-08-16 19:38 Jarkko Sakkinen
2016-08-16 19:38 ` Jarkko Sakkinen
2016-08-17  4:31 ` Jarkko Sakkinen
2016-08-17  4:31   ` Jarkko Sakkinen
2016-07-20  0:16 Jarkko Sakkinen
2016-07-20  0:16 ` Jarkko Sakkinen
2016-07-20 16:48 ` Jason Gunthorpe
2016-07-20 16:48   ` Jason Gunthorpe
2016-07-20 20:53   ` Jarkko Sakkinen
2016-07-20 20:53     ` Jarkko Sakkinen
2016-07-20 21:13     ` Jason Gunthorpe
2016-07-20 21:13       ` Jason Gunthorpe
2016-07-21  9:02       ` Jarkko Sakkinen
2016-07-21  9:02         ` Jarkko Sakkinen
2016-07-21 16:25         ` Jason Gunthorpe
2016-07-21 16:25           ` Jason Gunthorpe
2016-08-09 10:36           ` Jarkko Sakkinen
2016-08-09 10:36             ` Jarkko Sakkinen
2016-08-09 15:49             ` Jason Gunthorpe
2016-08-09 15:49               ` Jason Gunthorpe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1472000243-7088-1-git-send-email-jarkko.sakkinen@linux.intel.com \
    --to=jarkko.sakkinen@linux.intel.com \
    --cc=jgunthorpe@obsidianresearch.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=peterhuewe@gmx.de \
    --cc=stable@vger.kernel.org \
    --cc=tpmdd-devel@lists.sourceforge.net \
    --cc=tpmdd@selhorst.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.