Re: [PATCH v8 3/3] tpm: Lazily flush the auth session

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 10/28/24 1:50 AM, Jarkko Sakkinen wrote:
Move the allocation of chip->auth to tpm2_start_auth_session() so that this
field can be used as flag to tell whether auth session is active or not.

Instead of flushing and reloading the auth session for every transaction
separately, keep the session open unless /dev/tpm0 is used.

Reported-by: Pengyu Ma <mapengyu@xxxxxxxxx>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219229
Cc: stable@xxxxxxxxxxxxxxx # v6.10+
Fixes: 7ca110f2679b ("tpm: Address !chip->auth in tpm_buf_append_hmac_session*()")
Tested-by: Pengyu Ma <mapengyu@xxxxxxxxx>
Signed-off-by: Jarkko Sakkinen <jarkko@xxxxxxxxxx>

Reviewed-by: Stefan Berger <stefanb@xxxxxxxxxxxxx>
Tested-by: Stefan Berger <stefanb@xxxxxxxxxxxxx>

---
v8:
- Since auth session and null key are flushed at a same time, only
   either needs to be checked. Addresses and a remark from James
   Bottomley few revisions ago.
- kfree_sensitive()
- Effectively squash top three patches given the simplifications.
v7:
- No changes.
v6:
- No changes.
v5:
- No changes.
v4:
- Changed as bug.
v3:
- Refined the commit message.
- Removed the conditional for applying TPM2_SA_CONTINUE_SESSION only when
   /dev/tpm0 is open. It is not required as the auth session is flushed,
   not saved.
v2:
- A new patch.
---
  drivers/char/tpm/tpm-chip.c       | 10 +++++++
  drivers/char/tpm/tpm-dev-common.c |  3 +++
  drivers/char/tpm/tpm-interface.c  |  6 +++--
  drivers/char/tpm/tpm2-sessions.c  | 45 ++++++++++++++++++-------------
  4 files changed, 44 insertions(+), 20 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 854546000c92..1ff99a7091bb 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -674,6 +674,16 @@ EXPORT_SYMBOL_GPL(tpm_chip_register);
   */
  void tpm_chip_unregister(struct tpm_chip *chip)
  {
+#ifdef CONFIG_TCG_TPM2_HMAC
+	int rc;
+
+	rc = tpm_try_get_ops(chip);
+	if (!rc) {
+		tpm2_end_auth_session(chip);
+		tpm_put_ops(chip);
+	}
+#endif
+
  	tpm_del_legacy_sysfs(chip);
  	if (tpm_is_hwrng_enabled(chip))
  		hwrng_unregister(&chip->hwrng);
diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
index 30b4c288c1bb..c7a88fa7b0fc 100644
--- a/drivers/char/tpm/tpm-dev-common.c
+++ b/drivers/char/tpm/tpm-dev-common.c
@@ -27,6 +27,9 @@ static ssize_t tpm_dev_transmit(struct tpm_chip *chip, struct tpm_space *space,
  	struct tpm_header *header = (void *)buf;
  	ssize_t ret, len;
+ if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		tpm2_end_auth_session(chip);
+
  	ret = tpm2_prepare_space(chip, space, buf, bufsiz);
  	/* If the command is not implemented by the TPM, synthesize a
  	 * response with a TPM2_RC_COMMAND_CODE return for user-space.
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 5da134f12c9a..8134f002b121 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -379,10 +379,12 @@ int tpm_pm_suspend(struct device *dev)
rc = tpm_try_get_ops(chip);
  	if (!rc) {
-		if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+			tpm2_end_auth_session(chip);
  			tpm2_shutdown(chip, TPM2_SU_STATE);
-		else
+		} else {
  			rc = tpm1_pm_suspend(chip, tpm_suspend_pcr);
+		}
tpm_put_ops(chip);
  	}
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index 950a3e48293b..03145a465b5d 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -333,6 +333,9 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
  	}
#ifdef CONFIG_TCG_TPM2_HMAC
+	/* The first write to /dev/tpm{rm0} will flush the session. */
+	attributes |= TPM2_SA_CONTINUE_SESSION;
+
  	/*
  	 * The Architecture Guide requires us to strip trailing zeros
  	 * before computing the HMAC
@@ -484,7 +487,8 @@ static void tpm2_KDFe(u8 z[EC_PT_SZ], const char *str, u8 *pt_u, u8 *pt_v,
  	sha256_final(&sctx, out);
  }
-static void tpm_buf_append_salt(struct tpm_buf *buf, struct tpm_chip *chip)
+static void tpm_buf_append_salt(struct tpm_buf *buf, struct tpm_chip *chip,
+				struct tpm2_auth *auth)
  {
  	struct crypto_kpp *kpp;
  	struct kpp_request *req;
@@ -543,7 +547,7 @@ static void tpm_buf_append_salt(struct tpm_buf *buf, struct tpm_chip *chip)
  	sg_set_buf(&s[0], chip->null_ec_key_x, EC_PT_SZ);
  	sg_set_buf(&s[1], chip->null_ec_key_y, EC_PT_SZ);
  	kpp_request_set_input(req, s, EC_PT_SZ*2);
-	sg_init_one(d, chip->auth->salt, EC_PT_SZ);
+	sg_init_one(d, auth->salt, EC_PT_SZ);
  	kpp_request_set_output(req, d, EC_PT_SZ);
  	crypto_kpp_compute_shared_secret(req);
  	kpp_request_free(req);
@@ -554,8 +558,7 @@ static void tpm_buf_append_salt(struct tpm_buf *buf, struct tpm_chip *chip)
  	 * This works because KDFe fully consumes the secret before it
  	 * writes the salt
  	 */
-	tpm2_KDFe(chip->auth->salt, "SECRET", x, chip->null_ec_key_x,
-		  chip->auth->salt);
+	tpm2_KDFe(auth->salt, "SECRET", x, chip->null_ec_key_x, auth->salt);
out:
  	crypto_free_kpp(kpp);
@@ -853,7 +856,9 @@ int tpm_buf_check_hmac_response(struct tpm_chip *chip, struct tpm_buf *buf,
  		if (rc)
  			/* manually close the session if it wasn't consumed */
  			tpm2_flush_context(chip, auth->handle);
-		memzero_explicit(auth, sizeof(*auth));
+
+		kfree_sensitive(auth);
+		chip->auth = NULL;
  	} else {
  		/* reset for next use  */
  		auth->session = TPM_HEADER_SIZE;
@@ -881,7 +886,8 @@ void tpm2_end_auth_session(struct tpm_chip *chip)
  		return;
tpm2_flush_context(chip, auth->handle);
-	memzero_explicit(auth, sizeof(*auth));
+	kfree_sensitive(auth);
+	chip->auth = NULL;
  }
  EXPORT_SYMBOL(tpm2_end_auth_session);
@@ -962,16 +968,20 @@ static int tpm2_load_null(struct tpm_chip *chip, u32 *null_key)
   */
  int tpm2_start_auth_session(struct tpm_chip *chip)
  {
+	struct tpm2_auth *auth;
  	struct tpm_buf buf;
-	struct tpm2_auth *auth = chip->auth;
-	int rc;
  	u32 null_key;
+	int rc;
- if (!auth) {
-		dev_warn_once(&chip->dev, "auth session is not active\n");
+	if (chip->auth) {
+		dev_warn_once(&chip->dev, "auth session is active\n");
  		return 0;
  	}
+ auth = kzalloc(sizeof(*auth), GFP_KERNEL);
+	if (!auth)
+		return -ENOMEM;
+
  	rc = tpm2_load_null(chip, &null_key);
  	if (rc)
  		goto out;
@@ -992,7 +1002,7 @@ int tpm2_start_auth_session(struct tpm_chip *chip)
  	tpm_buf_append(&buf, auth->our_nonce, sizeof(auth->our_nonce));
/* append encrypted salt and squirrel away unencrypted in auth */
-	tpm_buf_append_salt(&buf, chip);
+	tpm_buf_append_salt(&buf, chip, auth);
  	/* session type (HMAC, audit or policy) */
  	tpm_buf_append_u8(&buf, TPM2_SE_HMAC);
@@ -1014,10 +1024,13 @@ int tpm2_start_auth_session(struct tpm_chip *chip) tpm_buf_destroy(&buf); - if (rc)
-		goto out;
+	if (rc == TPM2_RC_SUCCESS) {
+		chip->auth = auth;
+		return 0;
+	}
- out:
+out:
+	kfree_sensitive(auth);
  	return rc;
  }
  EXPORT_SYMBOL(tpm2_start_auth_session);
@@ -1367,10 +1380,6 @@ int tpm2_sessions_init(struct tpm_chip *chip)
  		return rc;
  	}
- chip->auth = kmalloc(sizeof(*chip->auth), GFP_KERNEL);
-	if (!chip->auth)
-		return -ENOMEM;
-
  	return rc;
  }
  #endif /* CONFIG_TCG_TPM2_HMAC */





[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux Kernel]     [Linux Kernel Hardening]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux