Jarkko Sakkinen (5):
tpm: Return on tpm2_create_null_primary() failure
tpm: Implement tpm2_load_null() rollback
tpm: flush the null key only when /dev/tpm0 is accessed
tpm: Allocate chip->auth in tpm2_start_auth_session()
tpm: flush the auth session only when /dev/tpm0 is open

 drivers/char/tpm/tpm-chip.c       |  14 ++++
 drivers/char/tpm/tpm-dev-common.c |   8 +++
 drivers/char/tpm/tpm-interface.c  |  10 ++-
 drivers/char/tpm/tpm2-cmd.c       |   3 +
 drivers/char/tpm/tpm2-sessions.c  | 115 +++++++++++++++++++-----------
 include/linux/tpm.h               |   2 +
 6 files changed, 108 insertions(+), 44 deletions(-)

Signature · 1 line

2.47.0

Jarkko Sakkinen

author maintainer

2024-10-21 05:39 UTC #1 lore raw

PATCH v7 1/5 tpm: Return on tpm2_create_null_primary() failure

tpm2_sessions_init() does not ignore the result of tpm2_create_null_primary(). Address this by returning -ENODEV to the caller. Given that upper layers cannot help healing the situation further, deal with the TPM error here by

Cc [email protected] # v6.10+

Fixes d2add27cf2b8 ("tpm: Add NULL primary creation")

Signed-off-by Jarkko Sakkinen <[email protected]>

---
v7:
- Add the error message back but fix it up a bit:
1. Remove 'TPM:' given dev_err().
2. s/NULL/null/ as this has nothing to do with the macro in libc.
3. Fix the reasoning: null key creation failed
v6:
- Address:
https://lore.kernel.org/linux-integrity/[email protected]/
as TPM RC is taken care of at the call site. Add also the missing documentation for the return values.
v5:
- Do not print klog messages on error, as tpm2_save_context() already takes care of this.
v4:
- Fixed up stable version.
v3:
- Handle TPM and POSIX error separately and return -ENODEV always back to the caller.
v2:
- Refined the commit message.

---
 drivers/char/tpm/tpm2-sessions.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

drivers/char/tpm/tpm2-sessions.c +9 -2

index d3521aadd43e..1e12e0b2492e 100644
@@ -1347,14 +1347,21 @@ static int tpm2_create_null_primary(struct tpm_chip *chip)
  *
  * Derive and context save the null primary and allocate memory in the
  * struct tpm_chip for the authorizations.
+ *
+ * Return:
+ * * 0		- OK
+ * * -errno	- A system error
+ * * TPM_RC	- A TPM error
  */
 int tpm2_sessions_init(struct tpm_chip *chip)
 {
 	int rc;
 
 	rc = tpm2_create_null_primary(chip);
-	if (rc)
-		dev_err(&chip->dev, "TPM: security failed (NULL seed derivation): %d\n", rc);
+	if (rc) {
+		dev_err(&chip->dev, "null primary key creation failed with %d\n", rc);
+		return rc;
+	}
 
 	chip->auth = kmalloc(sizeof(*chip->auth), GFP_KERNEL);
 	if (!chip->auth)

Signature · 1 line

2.47.0

Stefan Berger

2024-10-23 17:47 UTC #2 lore raw

Re: [PATCH v7 1/5] tpm: Return on tpm2_create_null_primary() failure

Quoted 3 lines · Jarkko Sakkinen

tpm2_sessions_init() does not ignore the result of tpm2_create_null_primary(). Address this by returning -ENODEV to the caller. Given that upper layers cannot help healing the situation

It looks like returning -ENODEV applied to a previous version of the patch.

Quoted 1 line

further, deal with the TPM error here by

This sounds like an incomplete sentence...

Quoted 57 lines

Cc [email protected] # v6.10+
Fixes d2add27cf2b8 ("tpm: Add NULL primary creation")
Signed-off-by Jarkko Sakkinen <[email protected]>
---
v7:
- Add the error message back but fix it up a bit:
1. Remove 'TPM:' given dev_err().
2. s/NULL/null/ as this has nothing to do with the macro in libc.
3. Fix the reasoning: null key creation failed
v6:
- Address:
https://lore.kernel.org/linux-integrity/[email protected]/
as TPM RC is taken care of at the call site. Add also the missing documentation for the return values.
v5:
- Do not print klog messages on error, as tpm2_save_context() already takes care of this.
v4:
- Fixed up stable version.
v3:
- Handle TPM and POSIX error separately and return -ENODEV always back to the caller.
v2:
- Refined the commit message.
---
  drivers/char/tpm/tpm2-sessions.c | 11 +++++++++--
  1 file changed, 9 insertions(+), 2 deletions(-)
drivers/char/tpm/tpm2-sessions.c +9 -2
index d3521aadd43e..1e12e0b2492e 100644
@@ -1347,14 +1347,21 @@ static int tpm2_create_null_primary(struct tpm_chip *chip)
   *
   * Derive and context save the null primary and allocate memory in the
   * struct tpm_chip for the authorizations.
+ *
+ * Return:
+ * * 0		- OK
+ * * -errno	- A system error
+ * * TPM_RC	- A TPM error
   */
  int tpm2_sessions_init(struct tpm_chip *chip)
  {
  	int rc;
 
  	rc = tpm2_create_null_primary(chip);
-	if (rc)
-		dev_err(&chip->dev, "TPM: security failed (NULL seed derivation): %d\n", rc);
+	if (rc) {
+		dev_err(&chip->dev, "null primary key creation failed with %d\n", rc);
+		return rc;
+	}
 
  	chip->auth = kmalloc(sizeof(*chip->auth), GFP_KERNEL);
  	if (!chip->auth)

Jarkko Sakkinen

author maintainer

2024-10-24 11:22 UTC #3 lore raw

Re: [PATCH v7 1/5] tpm: Return on tpm2_create_null_primary() failure

Quoted 7 lines · Stefan Berger

Quoted 3 lines · Jarkko Sakkinen
tpm2_sessions_init() does not ignore the result of tpm2_create_null_primary(). Address this by returning -ENODEV to the caller. Given that upper layers cannot help healing the situation

Quoted 5 lines

It looks like returning -ENODEV applied to a previous version of the patch.
Quoted 1 line
further, deal with the TPM error here by
This sounds like an incomplete sentence...

It looks like totally corrupted, thanks for the remark.

"tpm2_sessions_init() ignores the return value of tpm2_create_null_primary(). Fail early and return back to the caller if it fails. Fine-tune the error message while at it."

Is this sufficient?

BR, Jarkko

Jarkko Sakkinen

author maintainer

2024-10-21 05:39 UTC #4 lore raw

PATCH v7 2/5 tpm: Implement tpm2_load_null() rollback

In reply to Jarkko Sakkinen

tpm2_load_null() has weak and broken error handling:

- The return value of tpm2_create_primary() is ignored.
- Leaks TPM return codes from tpm2_load_context() to the caller.
- If the key name comparison succeeds returns previous error instead of zero to the caller.

Implement a proper error rollback.

Cc [email protected] # v6.10+

Fixes eb24c9788cd9 ("tpm: disable the TPM if NULL name changes")

Signed-off-by Jarkko Sakkinen <[email protected]>

--
v6:
- Address Stefan's remark:
https://lore.kernel.org/linux-integrity/[email protected]/
v5:
- Fix the TPM error code leak from tpm2_load_context(). v4:
- No changes.
v3:
- Update log messages. Previously the log message incorrectly stated on load failure that integrity check had been failed, even tho the check is done *after* the load operation.
v2:
- Refined the commit message.
- Reverted tpm2_create_primary() changes. They are not required if tmp_null_key is used as the parameter.

---
 drivers/char/tpm/tpm2-sessions.c | 43 +++++++++++++++++---------------
 1 file changed, 23 insertions(+), 20 deletions(-)

drivers/char/tpm/tpm2-sessions.c +23 ~~-20~~

index 1e12e0b2492e..bdac11964b55 100644
@@ -915,33 +915,36 @@ static int tpm2_parse_start_auth_session(struct tpm2_auth *auth,
 
 static int tpm2_load_null(struct tpm_chip *chip, u32 *null_key)
 {
-	int rc;
 	unsigned int offset = 0; /* dummy offset for null seed context */
 	u8 name[SHA256_DIGEST_SIZE + 2];
+	u32 tmp_null_key;
+	int rc;
 
 	rc = tpm2_load_context(chip, chip->null_key_context, &offset,
-			       null_key);
-	if (rc != -EINVAL)
-		return rc;
+			       &tmp_null_key);
+	if (rc != -EINVAL) {
+		if (!rc)
+			*null_key = tmp_null_key;
+		goto err;
+	}
 
-	/* an integrity failure may mean the TPM has been reset */
-	dev_err(&chip->dev, "NULL key integrity failure!\n");
-	/* check the null name against what we know */
-	tpm2_create_primary(chip, TPM2_RH_NULL, NULL, name);
-	if (memcmp(name, chip->null_key_name, sizeof(name)) == 0)
-		/* name unchanged, assume transient integrity failure */
-		return rc;
-	/*
-	 * Fatal TPM failure: the NULL seed has actually changed, so
-	 * the TPM must have been illegally reset.  All in-kernel TPM
-	 * operations will fail because the NULL primary can't be
-	 * loaded to salt the sessions, but disable the TPM anyway so
-	 * userspace programmes can't be compromised by it.
-	 */
-	dev_err(&chip->dev, "NULL name has changed, disabling TPM due to interference\n");
+	rc = tpm2_create_primary(chip, TPM2_RH_NULL, &tmp_null_key, name);
+	if (rc)
+		goto err;
+
+	/* Return the null key if the name has not been changed: */
+	if (memcmp(name, chip->null_key_name, sizeof(name)) == 0) {
+		*null_key = tmp_null_key;
+		return 0;
+	}
+
+	/* Deduce from the name change TPM interference: */
+	dev_err(&chip->dev, "the null key integrity check failedh\n");
+	tpm2_flush_context(chip, tmp_null_key);
 	chip->flags |= TPM_CHIP_FLAG_DISABLE;
 
-	return rc;
+err:
+	return rc ? -ENODEV : 0;
 }
 
 /**

Signature · 1 line

2.47.0

Stefan Berger

Reviewed-by on v7 2/5

2024-10-23 17:38 UTC #5 lore raw

Re: [PATCH v7 2/5] tpm: Implement tpm2_load_null() rollback

Quoted 74 lines · Jarkko Sakkinen

tpm2_load_null() has weak and broken error handling:
- The return value of tpm2_create_primary() is ignored.
- Leaks TPM return codes from tpm2_load_context() to the caller.
- If the key name comparison succeeds returns previous error instead of zero to the caller.
Implement a proper error rollback.
Cc [email protected] # v6.10+
Fixes eb24c9788cd9 ("tpm: disable the TPM if NULL name changes")
Signed-off-by Jarkko Sakkinen <[email protected]>
--
v6:
- Address Stefan's remark:
   https://lore.kernel.org/linux-integrity/[email protected]/
v5:
- Fix the TPM error code leak from tpm2_load_context(). v4:
- No changes.
v3:
- Update log messages. Previously the log message incorrectly stated on load failure that integrity check had been failed, even tho the check is done *after* the load operation.
v2:
- Refined the commit message.
- Reverted tpm2_create_primary() changes. They are not required if tmp_null_key is used as the parameter.
---
  drivers/char/tpm/tpm2-sessions.c | 43 +++++++++++++++++---------------
  1 file changed, 23 insertions(+), 20 deletions(-)
drivers/char/tpm/tpm2-sessions.c +10 ~~-19~~
index 1e12e0b2492e..bdac11964b55 100644
@@ -915,33 +915,36 @@ static int tpm2_parse_start_auth_session(struct tpm2_auth *auth,
 
  static int tpm2_load_null(struct tpm_chip *chip, u32 *null_key)
  {
-	int rc;
  	unsigned int offset = 0; /* dummy offset for null seed context */
  	u8 name[SHA256_DIGEST_SIZE + 2];
+	u32 tmp_null_key;
+	int rc;
 
  	rc = tpm2_load_context(chip, chip->null_key_context, &offset,
-			       null_key);
-	if (rc != -EINVAL)
-		return rc;
+			       &tmp_null_key);
+	if (rc != -EINVAL) {
+		if (!rc)
+			*null_key = tmp_null_key;
+		goto err;
+	}
 
-	/* an integrity failure may mean the TPM has been reset */
-	dev_err(&chip->dev, "NULL key integrity failure!\n");
-	/* check the null name against what we know */
-	tpm2_create_primary(chip, TPM2_RH_NULL, NULL, name);
-	if (memcmp(name, chip->null_key_name, sizeof(name)) == 0)
-		/* name unchanged, assume transient integrity failure */
-		return rc;
-	/*
-	 * Fatal TPM failure: the NULL seed has actually changed, so
-	 * the TPM must have been illegally reset.  All in-kernel TPM
-	 * operations will fail because the NULL primary can't be
-	 * loaded to salt the sessions, but disable the TPM anyway so
-	 * userspace programmes can't be compromised by it.
-	 */
-	dev_err(&chip->dev, "NULL name has changed, disabling TPM due to interference\n");
+	rc = tpm2_create_primary(chip, TPM2_RH_NULL, &tmp_null_key, name);
+	if (rc)

drivers/char/tpm/tpm2-sessions.c static int tpm2_parse_start_auth_session(struct tpm2_auth *auth, · line 942

Quoted 10 lines

drivers/char/tpm/tpm2-sessions.c +10 -0

+		goto err;
+
+	/* Return the null key if the name has not been changed: */
+	if (memcmp(name, chip->null_key_name, sizeof(name)) == 0) {
+		*null_key = tmp_null_key;
+		return 0;
+	}
+
+	/* Deduce from the name change TPM interference: */
+	dev_err(&chip->dev, "the null key integrity check failedh\n");

stray 'h': s/failedh/failed

drivers/char/tpm/tpm2-sessions.c static int tpm2_parse_start_auth_session(struct tpm2_auth *auth, · line 950

Quoted 9 lines

drivers/char/tpm/tpm2-sessions.c +3 -1

+	tpm2_flush_context(chip, tmp_null_key);
  	chip->flags |= TPM_CHIP_FLAG_DISABLE;
 
-	return rc;
+err:
+	return rc ? -ENODEV : 0;
  }
 
  /**

Reviewed-by Stefan Berger <[email protected]>

Jarkko Sakkinen

author maintainer

2024-10-21 05:39 UTC #6 lore raw

PATCH v7 3/5 tpm: flush the null key only when /dev/tpm0 is accessed

In reply to Jarkko Sakkinen

Instead of flushing and reloading the null key for every single auth session, flush it only when:

1. User space needs to access /dev/tpm{rm}0.
2. When going to sleep.
3. When unregistering the chip.

This removes the need to load and swap the null key between TPM and regular memory per transaction, when the user space is not using the chip.

Cc [email protected] # v6.10+

Fixes d2add27cf2b8 ("tpm: Add NULL primary creation")

Tested-by Pengyu Ma <[email protected]>

Signed-off-by Jarkko Sakkinen <[email protected]>

---
v5:
- No changes.
v4:
- Changed to bug fix as not having the patch there is a major hit to bootup times.
v3:
- Unchanged.
v2:
- Refined the commit message.
- Added tested-by from Pengyu Ma <[email protected]>.
- Removed spurious pr_info() statement.

---
 drivers/char/tpm/tpm-chip.c       | 13 +++++++++++++
 drivers/char/tpm/tpm-dev-common.c |  7 +++++++
 drivers/char/tpm/tpm-interface.c  |  9 +++++++--
 drivers/char/tpm/tpm2-cmd.c       |  3 +++
 drivers/char/tpm/tpm2-sessions.c  | 17 ++++++++++++++---
 include/linux/tpm.h               |  2 ++
 6 files changed, 46 insertions(+), 5 deletions(-)

drivers/char/tpm/tpm-chip.c +13 -0

index 854546000c92..0ea00e32f575 100644
@@ -674,6 +674,19 @@ 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) {
+		if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+			tpm2_flush_context(chip, chip->null_key);
+			chip->null_key = 0;
+		}
+		tpm_put_ops(chip);
+	}
+#endif
+
 	tpm_del_legacy_sysfs(chip);
 	if (tpm_is_hwrng_enabled(chip))
 		hwrng_unregister(&chip->hwrng);

drivers/char/tpm/tpm-dev-common.c +7 -0

index 30b4c288c1bb..4eaa8e05c291 100644
@@ -27,6 +27,13 @@ static ssize_t tpm_dev_transmit(struct tpm_chip *chip, struct tpm_space *space,
 	struct tpm_header *header = (void *)buf;
 	ssize_t ret, len;
 
+#ifdef CONFIG_TCG_TPM2_HMAC
+	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+		tpm2_flush_context(chip, chip->null_key);
+		chip->null_key = 0;
+	}
+#endif
+
 	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.

drivers/char/tpm/tpm-interface.c +7 -2

index 5da134f12c9a..bfa47d48b0f2 100644
@@ -379,10 +379,15 @@ 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) {
+#ifdef CONFIG_TCG_TPM2_HMAC
+			tpm2_flush_context(chip, chip->null_key);
+			chip->null_key = 0;
+#endif
 			tpm2_shutdown(chip, TPM2_SU_STATE);
-		else
+		} else {
 			rc = tpm1_pm_suspend(chip, tpm_suspend_pcr);
+		}
 
 		tpm_put_ops(chip);
 	}

drivers/char/tpm/tpm2-cmd.c +3 -0

index 1e856259219e..aba024cbe7c5 100644
@@ -364,6 +364,9 @@ void tpm2_flush_context(struct tpm_chip *chip, u32 handle)
 	struct tpm_buf buf;
 	int rc;
 
+	if (!handle)
+		return;
+
 	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
 	if (rc) {
 		dev_warn(&chip->dev, "0x%08x was not flushed, out of memory\n",

drivers/char/tpm/tpm2-sessions.c +14 -3

index bdac11964b55..78c650ce4c9f 100644
@@ -920,11 +920,19 @@ static int tpm2_load_null(struct tpm_chip *chip, u32 *null_key)
 	u32 tmp_null_key;
 	int rc;
 
+	/* fast path */
+	if (chip->null_key) {
+		*null_key = chip->null_key;
+		return 0;
+	}
+
 	rc = tpm2_load_context(chip, chip->null_key_context, &offset,
 			       &tmp_null_key);
 	if (rc != -EINVAL) {
-		if (!rc)
+		if (!rc) {
+			chip->null_key = tmp_null_key;
 			*null_key = tmp_null_key;
+		}
 		goto err;
 	}

@@ -934,6 +942,7 @@ static int tpm2_load_null(struct tpm_chip *chip, u32 *null_key)
 
 	/* Return the null key if the name has not been changed: */
 	if (memcmp(name, chip->null_key_name, sizeof(name)) == 0) {
+		chip->null_key = tmp_null_key;
 		*null_key = tmp_null_key;
 		return 0;
 	}

@@ -1006,7 +1015,6 @@ int tpm2_start_auth_session(struct tpm_chip *chip)
 	tpm_buf_append_u16(&buf, TPM_ALG_SHA256);
 
 	rc = tpm_transmit_cmd(chip, &buf, 0, "start auth session");
-	tpm2_flush_context(chip, null_key);
 
 	if (rc == TPM2_RC_SUCCESS)
 		rc = tpm2_parse_start_auth_session(auth, &buf);

@@ -1338,7 +1346,10 @@ static int tpm2_create_null_primary(struct tpm_chip *chip)
 
 		rc = tpm2_save_context(chip, null_key, chip->null_key_context,
 				       sizeof(chip->null_key_context), &offset);
-		tpm2_flush_context(chip, null_key);
+		if (rc)
+			tpm2_flush_context(chip, null_key);
+		else
+			chip->null_key = null_key;
 	}
 
 	return rc;

include/linux/tpm.h +2 -0

index e93ee8d936a9..4eb39db80e05 100644
@@ -205,6 +205,8 @@ struct tpm_chip {
 #ifdef CONFIG_TCG_TPM2_HMAC
 	/* details for communication security via sessions */
 
+	/* loaded null key */
+	u32 null_key;
 	/* saved context for NULL seed */
 	u8 null_key_context[TPM2_MAX_CONTEXT_SIZE];
 	 /* name of NULL seed */

Signature · 1 line

2.47.0

Stefan Berger

2024-10-23 18:18 UTC #7 lore raw

Re: [PATCH v7 3/5] tpm: flush the null key only when /dev/tpm0 is accessed

Quoted 41 lines · Jarkko Sakkinen

Instead of flushing and reloading the null key for every single auth session, flush it only when:
1. User space needs to access /dev/tpm{rm}0.
2. When going to sleep.
3. When unregistering the chip.
This removes the need to load and swap the null key between TPM and regular memory per transaction, when the user space is not using the chip.
Cc [email protected] # v6.10+
Fixes d2add27cf2b8 ("tpm: Add NULL primary creation")
Tested-by Pengyu Ma <[email protected]>
Signed-off-by Jarkko Sakkinen <[email protected]>
---
v5:
- No changes.
v4:
- Changed to bug fix as not having the patch there is a major hit to bootup times.
v3:
- Unchanged.
v2:
- Refined the commit message.
- Added tested-by from Pengyu Ma <[email protected]>.
- Removed spurious pr_info() statement.
---
  drivers/char/tpm/tpm-chip.c       | 13 +++++++++++++
  drivers/char/tpm/tpm-dev-common.c |  7 +++++++
  drivers/char/tpm/tpm-interface.c  |  9 +++++++--
  drivers/char/tpm/tpm2-cmd.c       |  3 +++
  drivers/char/tpm/tpm2-sessions.c  | 17 ++++++++++++++---
  include/linux/tpm.h               |  2 ++
  6 files changed, 46 insertions(+), 5 deletions(-)
drivers/char/tpm/tpm-chip.c +0 -0
index 854546000c92..0ea00e32f575 100644

drivers/char/tpm/tpm-chip.c EXPORT_SYMBOL_GPL(tpm_chip_register); · line 683

Quoted 10 lines

drivers/char/tpm/tpm-chip.c +7 -0

@@ -674,6 +674,19 @@ 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) {
+		if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+			tpm2_flush_context(chip, chip->null_key);

If chip->null_key is already 0, the above function will not do anything good, but you could avoid this whole block then by checking for 0 before tpm_try_get_ops().

Quoted 32 lines

drivers/char/tpm/tpm-chip.c +6 -0

+			chip->null_key = 0;
+		}
+		tpm_put_ops(chip);
+	}
+#endif
+
  	tpm_del_legacy_sysfs(chip);
  	if (tpm_is_hwrng_enabled(chip))
  		hwrng_unregister(&chip->hwrng);

drivers/char/tpm/tpm-dev-common.c +7 -0

index 30b4c288c1bb..4eaa8e05c291 100644
@@ -27,6 +27,13 @@ static ssize_t tpm_dev_transmit(struct tpm_chip *chip, struct tpm_space *space,
  	struct tpm_header *header = (void *)buf;
  	ssize_t ret, len;
 
+#ifdef CONFIG_TCG_TPM2_HMAC
+	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+		tpm2_flush_context(chip, chip->null_key);
+		chip->null_key = 0;
+	}
+#endif
+
  	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.

drivers/char/tpm/tpm-interface.c +0 -0

index 5da134f12c9a..bfa47d48b0f2 100644

drivers/char/tpm/tpm-interface.c int tpm_pm_suspend(struct device *dev) · line 386

Quoted 9 lines

drivers/char/tpm/tpm-interface.c +5 -1

@@ -379,10 +379,15 @@ 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) {
+#ifdef CONFIG_TCG_TPM2_HMAC
+			tpm2_flush_context(chip, chip->null_key);
+			chip->null_key = 0;
+#endif

Worth using an inline on this repeating pattern? Up to you.

static inline void tpm2_flush_null_key(struct tpm_chip *chip) {

#ifdef CONFIG_TCG_TPM2_HMAC
    if (chip->flags & TPM_CHIP_FLAG_TPM2 && chip->null_key) {
        tpm2_flush_context(chip, chip->null_key);
        chip->null_key = 0;
    }

#endif
}

Quoted 13 lines

drivers/char/tpm/tpm-interface.c +2 -1

  			tpm2_shutdown(chip, TPM2_SU_STATE);
-		else
+		} else {
  			rc = tpm1_pm_suspend(chip, tpm_suspend_pcr);
+		}
 
  		tpm_put_ops(chip);
  	}

drivers/char/tpm/tpm2-cmd.c +0 -0

index 1e856259219e..aba024cbe7c5 100644

drivers/char/tpm/tpm2-cmd.c void tpm2_flush_context(struct tpm_chip *chip, u32 handle) · line 369

Quoted 6 lines

drivers/char/tpm/tpm2-cmd.c +3 -0

@@ -364,6 +364,9 @@ void tpm2_flush_context(struct tpm_chip *chip, u32 handle)
  	struct tpm_buf buf;
  	int rc;
 
+	if (!handle)
+		return;
+

wouldn't be necessary with inline.

Quoted 61 lines

drivers/char/tpm/tpm2-cmd.c +0 -0

  	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
  	if (rc) {
  		dev_warn(&chip->dev, "0x%08x was not flushed, out of memory\n",

drivers/char/tpm/tpm2-sessions.c +14 -3

index bdac11964b55..78c650ce4c9f 100644
@@ -920,11 +920,19 @@ static int tpm2_load_null(struct tpm_chip *chip, u32 *null_key)
  	u32 tmp_null_key;
  	int rc;
 
+	/* fast path */
+	if (chip->null_key) {
+		*null_key = chip->null_key;
+		return 0;
+	}
+
  	rc = tpm2_load_context(chip, chip->null_key_context, &offset,
  			       &tmp_null_key);
  	if (rc != -EINVAL) {
-		if (!rc)
+		if (!rc) {
+			chip->null_key = tmp_null_key;
  			*null_key = tmp_null_key;
+		}
  		goto err;
  	}

@@ -934,6 +942,7 @@ static int tpm2_load_null(struct tpm_chip *chip, u32 *null_key)
 
  	/* Return the null key if the name has not been changed: */
  	if (memcmp(name, chip->null_key_name, sizeof(name)) == 0) {
+		chip->null_key = tmp_null_key;
  		*null_key = tmp_null_key;
  		return 0;
  	}

@@ -1006,7 +1015,6 @@ int tpm2_start_auth_session(struct tpm_chip *chip)
  	tpm_buf_append_u16(&buf, TPM_ALG_SHA256);
 
  	rc = tpm_transmit_cmd(chip, &buf, 0, "start auth session");
-	tpm2_flush_context(chip, null_key);
 
  	if (rc == TPM2_RC_SUCCESS)
  		rc = tpm2_parse_start_auth_session(auth, &buf);

@@ -1338,7 +1346,10 @@ static int tpm2_create_null_primary(struct tpm_chip *chip)
 
  		rc = tpm2_save_context(chip, null_key, chip->null_key_context,
  				       sizeof(chip->null_key_context), &offset);
-		tpm2_flush_context(chip, null_key);
+		if (rc)
+			tpm2_flush_context(chip, null_key);
+		else
+			chip->null_key = null_key;
  	}
 
  	return rc;

include/linux/tpm.h +0 -0

index e93ee8d936a9..4eb39db80e05 100644

include/linux/tpm.h struct tpm_chip { · line 208

Quoted 4 lines

include/linux/tpm.h +1 -0

@@ -205,6 +205,8 @@ struct tpm_chip {
  #ifdef CONFIG_TCG_TPM2_HMAC
  	/* details for communication security via sessions */
 
+	/* loaded null key */

nit: handle of loaded null key

Quoted 4 lines

include/linux/tpm.h +1 -0

+	u32 null_key;
  	/* saved context for NULL seed */
  	u8 null_key_context[TPM2_MAX_CONTEXT_SIZE];
  	 /* name of NULL seed */

Jarkko Sakkinen

author maintainer

2024-10-24 11:25 UTC #8 lore raw

Re: [PATCH v7 3/5] tpm: flush the null key only when /dev/tpm0 is accessed

Quoted 129 lines · Stefan Berger

Quoted 41 lines · Jarkko Sakkinen
Instead of flushing and reloading the null key for every single auth session, flush it only when:
1. User space needs to access /dev/tpm{rm}0.
2. When going to sleep.
3. When unregistering the chip.
This removes the need to load and swap the null key between TPM and regular memory per transaction, when the user space is not using the chip.
Cc [email protected] # v6.10+
Fixes d2add27cf2b8 ("tpm: Add NULL primary creation")
Tested-by Pengyu Ma <[email protected]>
Signed-off-by Jarkko Sakkinen <[email protected]>
---
v5:
- No changes.
v4:
- Changed to bug fix as not having the patch there is a major hit to bootup times.
v3:
- Unchanged.
v2:
- Refined the commit message.
- Added tested-by from Pengyu Ma <[email protected]>.
- Removed spurious pr_info() statement.
---
  drivers/char/tpm/tpm-chip.c       | 13 +++++++++++++
  drivers/char/tpm/tpm-dev-common.c |  7 +++++++
  drivers/char/tpm/tpm-interface.c  |  9 +++++++--
  drivers/char/tpm/tpm2-cmd.c       |  3 +++
  drivers/char/tpm/tpm2-sessions.c  | 17 ++++++++++++++---
  include/linux/tpm.h               |  2 ++
  6 files changed, 46 insertions(+), 5 deletions(-)
drivers/char/tpm/tpm-chip.c +0 -0
index 854546000c92..0ea00e32f575 100644
drivers/char/tpm/tpm-chip.c EXPORT_SYMBOL_GPL(tpm_chip_register); · line 683
Quoted 10 lines
drivers/char/tpm/tpm-chip.c +7 -0
@@ -674,6 +674,19 @@ 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) {
+		if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+			tpm2_flush_context(chip, chip->null_key);
If chip->null_key is already 0, the above function will not do anything good, but you could avoid this whole block then by checking for 0 before tpm_try_get_ops().
Quoted 32 lines
drivers/char/tpm/tpm-chip.c +6 -0
+			chip->null_key = 0;
+		}
+		tpm_put_ops(chip);
+	}
+#endif
+
  	tpm_del_legacy_sysfs(chip);
  	if (tpm_is_hwrng_enabled(chip))
  		hwrng_unregister(&chip->hwrng);
drivers/char/tpm/tpm-dev-common.c +7 -0
index 30b4c288c1bb..4eaa8e05c291 100644
@@ -27,6 +27,13 @@ static ssize_t tpm_dev_transmit(struct tpm_chip *chip, struct tpm_space *space,
  	struct tpm_header *header = (void *)buf;
  	ssize_t ret, len;
 
+#ifdef CONFIG_TCG_TPM2_HMAC
+	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+		tpm2_flush_context(chip, chip->null_key);
+		chip->null_key = 0;
+	}
+#endif
+
  	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.
drivers/char/tpm/tpm-interface.c +0 -0
index 5da134f12c9a..bfa47d48b0f2 100644
drivers/char/tpm/tpm-interface.c int tpm_pm_suspend(struct device *dev) · line 386
Quoted 9 lines
drivers/char/tpm/tpm-interface.c +5 -1
@@ -379,10 +379,15 @@ 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) {
+#ifdef CONFIG_TCG_TPM2_HMAC
+			tpm2_flush_context(chip, chip->null_key);
+			chip->null_key = 0;
+#endif
Worth using an inline on this repeating pattern? Up to you.
static inline void tpm2_flush_null_key(struct tpm_chip *chip) {
#ifdef CONFIG_TCG_TPM2_HMAC
     if (chip->flags & TPM_CHIP_FLAG_TPM2 && chip->null_key) {
         tpm2_flush_context(chip, chip->null_key);
         chip->null_key = 0;
     }
#endif
}
Quoted 16 lines
drivers/char/tpm/tpm-interface.c +2 -1
  			tpm2_shutdown(chip, TPM2_SU_STATE);
-		else
+		} else {
  			rc = tpm1_pm_suspend(chip, tpm_suspend_pcr);
+		}
 
  		tpm_put_ops(chip);
  	}
drivers/char/tpm/tpm2-cmd.c +0 -0
index 1e856259219e..aba024cbe7c5 100644
@@ -364,6 +364,9 @@ void tpm2_flush_context(struct tpm_chip *chip, u32 handle)
  	struct tpm_buf buf;
  	int rc;
 

Quoted 5 lines

drivers/char/tpm/tpm2-cmd.c void tpm2_flush_context(struct tpm_chip *chip, u32 handle) · line 369
Quoted 3 lines
drivers/char/tpm/tpm2-cmd.c +3 -0
+	if (!handle)
+		return;
+
wouldn't be necessary with inline.

True!

Quoted 65 lines

drivers/char/tpm/tpm2-cmd.c +0 -0

  	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
  	if (rc) {
  		dev_warn(&chip->dev, "0x%08x was not flushed, out of memory\n",

drivers/char/tpm/tpm2-sessions.c +14 -3

index bdac11964b55..78c650ce4c9f 100644
@@ -920,11 +920,19 @@ static int tpm2_load_null(struct tpm_chip *chip, u32 *null_key)
  	u32 tmp_null_key;
  	int rc;
 
+	/* fast path */
+	if (chip->null_key) {
+		*null_key = chip->null_key;
+		return 0;
+	}
+
  	rc = tpm2_load_context(chip, chip->null_key_context, &offset,
  			       &tmp_null_key);
  	if (rc != -EINVAL) {
-		if (!rc)
+		if (!rc) {
+			chip->null_key = tmp_null_key;
  			*null_key = tmp_null_key;
+		}
  		goto err;
  	}

@@ -934,6 +942,7 @@ static int tpm2_load_null(struct tpm_chip *chip, u32 *null_key)
 
  	/* Return the null key if the name has not been changed: */
  	if (memcmp(name, chip->null_key_name, sizeof(name)) == 0) {
+		chip->null_key = tmp_null_key;
  		*null_key = tmp_null_key;
  		return 0;
  	}

@@ -1006,7 +1015,6 @@ int tpm2_start_auth_session(struct tpm_chip *chip)
  	tpm_buf_append_u16(&buf, TPM_ALG_SHA256);
 
  	rc = tpm_transmit_cmd(chip, &buf, 0, "start auth session");
-	tpm2_flush_context(chip, null_key);
 
  	if (rc == TPM2_RC_SUCCESS)
  		rc = tpm2_parse_start_auth_session(auth, &buf);

@@ -1338,7 +1346,10 @@ static int tpm2_create_null_primary(struct tpm_chip *chip)
 
  		rc = tpm2_save_context(chip, null_key, chip->null_key_context,
  				       sizeof(chip->null_key_context), &offset);
-		tpm2_flush_context(chip, null_key);
+		if (rc)
+			tpm2_flush_context(chip, null_key);
+		else
+			chip->null_key = null_key;
  	}
 
  	return rc;

include/linux/tpm.h +0 -0

index e93ee8d936a9..4eb39db80e05 100644
@@ -205,6 +205,8 @@ struct tpm_chip {
  #ifdef CONFIG_TCG_TPM2_HMAC
  	/* details for communication security via sessions */

Quoted 8 lines

include/linux/tpm.h struct tpm_chip { · line 208
Quoted 1 line
include/linux/tpm.h +1 -0
+	/* loaded null key */
nit: handle of loaded null key
include/linux/tpm.h struct tpm_chip { · line 212
Quoted 4 lines
include/linux/tpm.h +1 -0
+	u32 null_key;
  	/* saved context for NULL seed */
  	u8 null_key_context[TPM2_MAX_CONTEXT_SIZE];
  	 /* name of NULL seed */

I agree with all of your remarks. I'll send one more round because there is enough changes. Thank you.

BR, Jarkko

Jarkko Sakkinen

author maintainer

2024-10-21 05:39 UTC #9 lore raw

PATCH v7 4/5 tpm: Allocate chip->auth in tpm2_start_auth_session()

In reply to Jarkko Sakkinen

Move allocation of chip->auth to tpm2_start_auth_session() so that the field can be used as flag to tell whether auth session is active or not.

Cc [email protected] # v6.10+

Fixes 699e3efd6c64 ("tpm: Add HMAC session start and end functions")

Signed-off-by Jarkko Sakkinen <[email protected]>

---
v5:
- No changes.
v4:
- Change to bug.
v3:
- No changes.
v2:
- A new patch.

---
 drivers/char/tpm/tpm2-sessions.c | 43 +++++++++++++++++++-------------
 1 file changed, 25 insertions(+), 18 deletions(-)

drivers/char/tpm/tpm2-sessions.c +25 ~~-18~~

index 78c650ce4c9f..6e52785de9fd 100644
@@ -484,7 +484,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 +544,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 +555,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);

@@ -854,6 +854,8 @@ int tpm_buf_check_hmac_response(struct tpm_chip *chip, struct tpm_buf *buf,
 			/* manually close the session if it wasn't consumed */
 			tpm2_flush_context(chip, auth->handle);
 		memzero_explicit(auth, sizeof(*auth));
+		kfree(auth);
+		chip->auth = NULL;
 	} else {
 		/* reset for next use  */
 		auth->session = TPM_HEADER_SIZE;

@@ -882,6 +884,8 @@ void tpm2_end_auth_session(struct tpm_chip *chip)
 
 	tpm2_flush_context(chip, auth->handle);
 	memzero_explicit(auth, sizeof(*auth));
+	kfree(auth);
+	chip->auth = NULL;
 }
 EXPORT_SYMBOL(tpm2_end_auth_session);

@@ -970,25 +974,29 @@ 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;
+		goto err;
 
 	auth->session = TPM_HEADER_SIZE;
 
 	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_START_AUTH_SESS);
 	if (rc)
-		goto out;
+		goto err;
 
 	/* salt key handle */
 	tpm_buf_append_u32(&buf, null_key);

@@ -1000,7 +1008,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);

@@ -1021,10 +1029,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:
+err:
+	kfree(auth);
 	return rc;
 }
 EXPORT_SYMBOL(tpm2_start_auth_session);

@@ -1377,10 +1388,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 */

Signature · 1 line

2.47.0

Stefan Berger

Reviewed-by on v7 4/5

2024-10-23 19:15 UTC #10 lore raw

Re: [PATCH v7 4/5] tpm: Allocate chip->auth in tpm2_start_auth_session()

Quoted 118 lines · Jarkko Sakkinen

Move allocation of chip->auth to tpm2_start_auth_session() so that the field can be used as flag to tell whether auth session is active or not.

Cc [email protected] # v6.10+

Fixes 699e3efd6c64 ("tpm: Add HMAC session start and end functions")

Signed-off-by Jarkko Sakkinen <[email protected]>

---
v5:
- No changes.
v4:
- Change to bug.
v3:
- No changes.
v2:
- A new patch.

---
  drivers/char/tpm/tpm2-sessions.c | 43 +++++++++++++++++++-------------
  1 file changed, 25 insertions(+), 18 deletions(-)

drivers/char/tpm/tpm2-sessions.c +19 ~~-11~~

index 78c650ce4c9f..6e52785de9fd 100644
@@ -484,7 +484,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 +544,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 +555,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);

@@ -854,6 +854,8 @@ int tpm_buf_check_hmac_response(struct tpm_chip *chip, struct tpm_buf *buf,
  			/* manually close the session if it wasn't consumed */
  			tpm2_flush_context(chip, auth->handle);
  		memzero_explicit(auth, sizeof(*auth));
+		kfree(auth);
+		chip->auth = NULL;
  	} else {
  		/* reset for next use  */
  		auth->session = TPM_HEADER_SIZE;

@@ -882,6 +884,8 @@ void tpm2_end_auth_session(struct tpm_chip *chip)
 
  	tpm2_flush_context(chip, auth->handle);
  	memzero_explicit(auth, sizeof(*auth));
+	kfree(auth);
+	chip->auth = NULL;
  }
  EXPORT_SYMBOL(tpm2_end_auth_session);

@@ -970,25 +974,29 @@ 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;
+		goto err;
 
  	auth->session = TPM_HEADER_SIZE;
 
  	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_START_AUTH_SESS);
  	if (rc)
-		goto out;
+		goto err;
 
  	/* salt key handle */
  	tpm_buf_append_u32(&buf, null_key);

@@ -1000,7 +1008,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);

@@ -1021,10 +1029,13 @@ int tpm2_start_auth_session(struct tpm_chip *chip)
 
  	tpm_buf_destroy(&buf);

drivers/char/tpm/tpm2-sessions.c int tpm2_start_auth_session(struct tpm_chip *chip) · line 1037

Quoted 10 lines

drivers/char/tpm/tpm2-sessions.c +5 -3

 
-	if (rc)
-		goto out;
+	if (rc == TPM2_RC_SUCCESS) {
+		chip->auth = auth;
+		return 0;
+	}
 
- out:
+err:

like in many other cases before kfree(auth):
memzero_explicit(auth, sizeof(*auth));

With this:

Reviewed-by Stefan Berger <[email protected]>

Quoted 15 lines

drivers/char/tpm/tpm2-sessions.c +1 -4

+	kfree(auth);
  	return rc;
  }
  EXPORT_SYMBOL(tpm2_start_auth_session);

@@ -1377,10 +1388,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 */

Jarkko Sakkinen

author maintainer

2024-10-24 11:28 UTC #11 lore raw

Re: [PATCH v7 4/5] tpm: Allocate chip->auth in tpm2_start_auth_session()

Quoted 135 lines · Stefan Berger

Quoted 118 lines · Jarkko Sakkinen

Move allocation of chip->auth to tpm2_start_auth_session() so that the field can be used as flag to tell whether auth session is active or not.

Cc [email protected] # v6.10+

Fixes 699e3efd6c64 ("tpm: Add HMAC session start and end functions")

Signed-off-by Jarkko Sakkinen <[email protected]>

---
v5:
- No changes.
v4:
- Change to bug.
v3:
- No changes.
v2:
- A new patch.

---
  drivers/char/tpm/tpm2-sessions.c | 43 +++++++++++++++++++-------------
  1 file changed, 25 insertions(+), 18 deletions(-)

drivers/char/tpm/tpm2-sessions.c +19 ~~-11~~

index 78c650ce4c9f..6e52785de9fd 100644
@@ -484,7 +484,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 +544,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 +555,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);

@@ -854,6 +854,8 @@ int tpm_buf_check_hmac_response(struct tpm_chip *chip, struct tpm_buf *buf,
  			/* manually close the session if it wasn't consumed */
  			tpm2_flush_context(chip, auth->handle);
  		memzero_explicit(auth, sizeof(*auth));
+		kfree(auth);
+		chip->auth = NULL;
  	} else {
  		/* reset for next use  */
  		auth->session = TPM_HEADER_SIZE;

@@ -882,6 +884,8 @@ void tpm2_end_auth_session(struct tpm_chip *chip)
 
  	tpm2_flush_context(chip, auth->handle);
  	memzero_explicit(auth, sizeof(*auth));
+	kfree(auth);
+	chip->auth = NULL;
  }
  EXPORT_SYMBOL(tpm2_end_auth_session);

@@ -970,25 +974,29 @@ 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;
+		goto err;
 
  	auth->session = TPM_HEADER_SIZE;
 
  	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_START_AUTH_SESS);
  	if (rc)
-		goto out;
+		goto err;
 
  	/* salt key handle */
  	tpm_buf_append_u32(&buf, null_key);

@@ -1000,7 +1008,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);

@@ -1021,10 +1029,13 @@ int tpm2_start_auth_session(struct tpm_chip *chip)
 
  	tpm_buf_destroy(&buf);

drivers/char/tpm/tpm2-sessions.c int tpm2_start_auth_session(struct tpm_chip *chip) · line 1037

Quoted 10 lines

drivers/char/tpm/tpm2-sessions.c +5 -3

 
-	if (rc)
-		goto out;
+	if (rc == TPM2_RC_SUCCESS) {
+		chip->auth = auth;
+		return 0;
+	}
 
- out:
+err:

like in many other cases before kfree(auth):
memzero_explicit(auth, sizeof(*auth));

Quoted 3 lines

With this:
Reviewed-by Stefan Berger <[email protected]>

Thanks, or should we use kfree_sensitive()?

It has some additional functionality, which is missed now:

https://elixir.bootlin.com/linux/v6.11.5/source/mm/slab_common.c#L1339

I.e. kasan_unpoison().

BR, Jarkko

Stefan Berger

2024-10-24 13:00 UTC #12 lore raw

Re: [PATCH v7 4/5] tpm: Allocate chip->auth in tpm2_start_auth_session()

Quoted 140 lines · Jarkko Sakkinen

Quoted 135 lines · Stefan Berger

Quoted 118 lines · Jarkko Sakkinen

Move allocation of chip->auth to tpm2_start_auth_session() so that the field can be used as flag to tell whether auth session is active or not.

Cc [email protected] # v6.10+

Fixes 699e3efd6c64 ("tpm: Add HMAC session start and end functions")

Signed-off-by Jarkko Sakkinen <[email protected]>

---
v5:
- No changes.
v4:
- Change to bug.
v3:
- No changes.
v2:
- A new patch.

---
   drivers/char/tpm/tpm2-sessions.c | 43 +++++++++++++++++++-------------
   1 file changed, 25 insertions(+), 18 deletions(-)

drivers/char/tpm/tpm2-sessions.c +19 ~~-11~~

index 78c650ce4c9f..6e52785de9fd 100644
@@ -484,7 +484,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 +544,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 +555,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);

@@ -854,6 +854,8 @@ int tpm_buf_check_hmac_response(struct tpm_chip *chip, struct tpm_buf *buf,
   			/* manually close the session if it wasn't consumed */
   			tpm2_flush_context(chip, auth->handle);
   		memzero_explicit(auth, sizeof(*auth));
+		kfree(auth);
+		chip->auth = NULL;
   	} else {
   		/* reset for next use  */
   		auth->session = TPM_HEADER_SIZE;

@@ -882,6 +884,8 @@ void tpm2_end_auth_session(struct tpm_chip *chip)
 
   	tpm2_flush_context(chip, auth->handle);
   	memzero_explicit(auth, sizeof(*auth));
+	kfree(auth);
+	chip->auth = NULL;
   }
   EXPORT_SYMBOL(tpm2_end_auth_session);

@@ -970,25 +974,29 @@ 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;
+		goto err;
 
   	auth->session = TPM_HEADER_SIZE;
 
   	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_START_AUTH_SESS);
   	if (rc)
-		goto out;
+		goto err;
 
   	/* salt key handle */
   	tpm_buf_append_u32(&buf, null_key);

@@ -1000,7 +1008,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);

@@ -1021,10 +1029,13 @@ int tpm2_start_auth_session(struct tpm_chip *chip)
 
   	tpm_buf_destroy(&buf);

drivers/char/tpm/tpm2-sessions.c int tpm2_start_auth_session(struct tpm_chip *chip) · line 1037

Quoted 10 lines

drivers/char/tpm/tpm2-sessions.c +5 -3

 
-	if (rc)
-		goto out;
+	if (rc == TPM2_RC_SUCCESS) {
+		chip->auth = auth;
+		return 0;
+	}
 
- out:
+err:

like in many other cases before kfree(auth):
memzero_explicit(auth, sizeof(*auth));

Quoted 3 lines

With this:
Reviewed-by Stefan Berger <[email protected]>

Quoted 7 lines

Thanks, or should we use kfree_sensitive()?
It has some additional functionality, which is missed now:
https://elixir.bootlin.com/linux/v6.11.5/source/mm/slab_common.c#L1339
I.e. kasan_unpoison().

And change the other ones that use memzero_explicit()?

Quoted 3 lines

BR, Jarkko

Jarkko Sakkinen

author maintainer

2024-10-25 14:45 UTC #13 lore raw

Re: [PATCH v7 4/5] tpm: Allocate chip->auth in tpm2_start_auth_session()

Quoted 145 lines · Stefan Berger

Quoted 142 lines · Jarkko Sakkinen

Quoted 135 lines · Stefan Berger

Quoted 118 lines · Jarkko Sakkinen

Move allocation of chip->auth to tpm2_start_auth_session() so that the field can be used as flag to tell whether auth session is active or not.

Cc [email protected] # v6.10+

Fixes 699e3efd6c64 ("tpm: Add HMAC session start and end functions")

Signed-off-by Jarkko Sakkinen <[email protected]>

---
v5:
- No changes.
v4:
- Change to bug.
v3:
- No changes.
v2:
- A new patch.

---
   drivers/char/tpm/tpm2-sessions.c | 43 +++++++++++++++++++-------------
   1 file changed, 25 insertions(+), 18 deletions(-)

drivers/char/tpm/tpm2-sessions.c +19 ~~-11~~

index 78c650ce4c9f..6e52785de9fd 100644
@@ -484,7 +484,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 +544,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 +555,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);

@@ -854,6 +854,8 @@ int tpm_buf_check_hmac_response(struct tpm_chip *chip, struct tpm_buf *buf,
   			/* manually close the session if it wasn't consumed */
   			tpm2_flush_context(chip, auth->handle);
   		memzero_explicit(auth, sizeof(*auth));
+		kfree(auth);
+		chip->auth = NULL;
   	} else {
   		/* reset for next use  */
   		auth->session = TPM_HEADER_SIZE;

@@ -882,6 +884,8 @@ void tpm2_end_auth_session(struct tpm_chip *chip)
 
   	tpm2_flush_context(chip, auth->handle);
   	memzero_explicit(auth, sizeof(*auth));
+	kfree(auth);
+	chip->auth = NULL;
   }
   EXPORT_SYMBOL(tpm2_end_auth_session);

@@ -970,25 +974,29 @@ 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;
+		goto err;
 
   	auth->session = TPM_HEADER_SIZE;
 
   	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_START_AUTH_SESS);
   	if (rc)
-		goto out;
+		goto err;
 
   	/* salt key handle */
   	tpm_buf_append_u32(&buf, null_key);

@@ -1000,7 +1008,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);

@@ -1021,10 +1029,13 @@ int tpm2_start_auth_session(struct tpm_chip *chip)
 
   	tpm_buf_destroy(&buf);

drivers/char/tpm/tpm2-sessions.c int tpm2_start_auth_session(struct tpm_chip *chip) · line 1037

Quoted 10 lines

drivers/char/tpm/tpm2-sessions.c +5 -3

 
-	if (rc)
-		goto out;
+	if (rc == TPM2_RC_SUCCESS) {
+		chip->auth = auth;
+		return 0;
+	}
 
- out:
+err:

like in many other cases before kfree(auth):
memzero_explicit(auth, sizeof(*auth));

Quoted 3 lines

With this:
Reviewed-by Stefan Berger <[email protected]>

Thanks, or should we use kfree_sensitive()?

Quoted 7 lines

Quoted 5 lines
It has some additional functionality, which is missed now:
https://elixir.bootlin.com/linux/v6.11.5/source/mm/slab_common.c#L1339
I.e. kasan_unpoison().
And change the other ones that use memzero_explicit()?

Yeah, might be a good idea too. Don't invent your own "safe primitives" sounds like a good idea to me at least...

Quoted 4 lines

Quoted 4 lines
BR, Jarkko

BR, Jarkko

Jarkko Sakkinen

author maintainer

2024-10-21 05:39 UTC #14 lore raw

PATCH v7 5/5 tpm: flush the auth session only when /dev/tpm0 is open

In reply to Jarkko Sakkinen

Instead of flushing and reloading the auth session for every single transaction, keep the session open unless /dev/tpm0 is used. In practice this means applying TPM2_SA_CONTINUE_SESSION to the session attributes. Flush the session always when /dev/tpm0 is written.

Reported-by Pengyu Ma <[email protected]>

Closes https://bugzilla.kernel.org/show_bug.cgi?id=219229

Cc [email protected] # v6.10+

Fixes 7ca110f2679b ("tpm: Address !chip->auth in tpm_buf_append_hmac_session*()")

Tested-by Pengyu Ma <[email protected]>

Signed-off-by Jarkko Sakkinen <[email protected]>

---
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       | 1 +
 drivers/char/tpm/tpm-dev-common.c | 1 +
 drivers/char/tpm/tpm-interface.c  | 1 +
 drivers/char/tpm/tpm2-sessions.c  | 3 +++
 4 files changed, 6 insertions(+)

drivers/char/tpm/tpm-chip.c +1 -0

index 0ea00e32f575..7a6bb30d1f32 100644
@@ -680,6 +680,7 @@ void tpm_chip_unregister(struct tpm_chip *chip)
 	rc = tpm_try_get_ops(chip);
 	if (!rc) {
 		if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+			tpm2_end_auth_session(chip);
 			tpm2_flush_context(chip, chip->null_key);
 			chip->null_key = 0;
 		}

drivers/char/tpm/tpm-dev-common.c +1 -0

index 4eaa8e05c291..a3ed7a99a394 100644
@@ -29,6 +29,7 @@ static ssize_t tpm_dev_transmit(struct tpm_chip *chip, struct tpm_space *space,
 
 #ifdef CONFIG_TCG_TPM2_HMAC
 	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+		tpm2_end_auth_session(chip);
 		tpm2_flush_context(chip, chip->null_key);
 		chip->null_key = 0;
 	}

drivers/char/tpm/tpm-interface.c +1 -0

index bfa47d48b0f2..2363018fa8fb 100644
@@ -381,6 +381,7 @@ int tpm_pm_suspend(struct device *dev)
 	if (!rc) {
 		if (chip->flags & TPM_CHIP_FLAG_TPM2) {
 #ifdef CONFIG_TCG_TPM2_HMAC
+			tpm2_end_auth_session(chip);
 			tpm2_flush_context(chip, chip->null_key);
 			chip->null_key = 0;
 #endif

drivers/char/tpm/tpm2-sessions.c +3 -0

index 6e52785de9fd..a7079c7ec6d1 100644
@@ -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

Signature · 1 line

2.47.0

Stefan Berger

2024-10-23 19:29 UTC #15 lore raw

Re: [PATCH v7 5/5] tpm: flush the auth session only when /dev/tpm0 is open

Quoted 35 lines · Jarkko Sakkinen

Instead of flushing and reloading the auth session for every single transaction, keep the session open unless /dev/tpm0 is used. In practice this means applying TPM2_SA_CONTINUE_SESSION to the session attributes. Flush the session always when /dev/tpm0 is written.
Reported-by Pengyu Ma <[email protected]>
Closes https://bugzilla.kernel.org/show_bug.cgi?id=219229
Cc [email protected] # v6.10+
Fixes 7ca110f2679b ("tpm: Address !chip->auth in tpm_buf_append_hmac_session*()")
Tested-by Pengyu Ma <[email protected]>
Signed-off-by Jarkko Sakkinen <[email protected]>
---
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       | 1 +
  drivers/char/tpm/tpm-dev-common.c | 1 +
  drivers/char/tpm/tpm-interface.c  | 1 +
  drivers/char/tpm/tpm2-sessions.c  | 3 +++
  4 files changed, 6 insertions(+)
drivers/char/tpm/tpm-chip.c +0 -0
index 0ea00e32f575..7a6bb30d1f32 100644

drivers/char/tpm/tpm-chip.c void tpm_chip_unregister(struct tpm_chip *chip) · line 683

Quoted 4 lines

drivers/char/tpm/tpm-chip.c +1 -0

@@ -680,6 +680,7 @@ void tpm_chip_unregister(struct tpm_chip *chip)
  	rc = tpm_try_get_ops(chip);
  	if (!rc) {
  		if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+			tpm2_end_auth_session(chip);

That could also go into the inline. Looks good otherwise.

Quoted 41 lines

drivers/char/tpm/tpm-chip.c +0 -0

  			tpm2_flush_context(chip, chip->null_key);
  			chip->null_key = 0;
  		}

drivers/char/tpm/tpm-dev-common.c +1 -0

index 4eaa8e05c291..a3ed7a99a394 100644
@@ -29,6 +29,7 @@ static ssize_t tpm_dev_transmit(struct tpm_chip *chip, struct tpm_space *space,
 
  #ifdef CONFIG_TCG_TPM2_HMAC
  	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+		tpm2_end_auth_session(chip);
  		tpm2_flush_context(chip, chip->null_key);
  		chip->null_key = 0;
  	}

drivers/char/tpm/tpm-interface.c +1 -0

index bfa47d48b0f2..2363018fa8fb 100644
@@ -381,6 +381,7 @@ int tpm_pm_suspend(struct device *dev)
  	if (!rc) {
  		if (chip->flags & TPM_CHIP_FLAG_TPM2) {
  #ifdef CONFIG_TCG_TPM2_HMAC
+			tpm2_end_auth_session(chip);
  			tpm2_flush_context(chip, chip->null_key);
  			chip->null_key = 0;
  #endif

drivers/char/tpm/tpm2-sessions.c +3 -0

index 6e52785de9fd..a7079c7ec6d1 100644
@@ -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