Skip to contentSkip to conversation

tpm: Do not print an error message when doing TPM auto startup

PATCH In review Jason Gunthorpe 3 participants 4 messages Nov 21, 2016 – Jan 9, 2017 1 message not in the archive TPM device driver

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 tpm: Do not print an error message when doing TPM auto startup

This is a regression when this code was reworked and made the error print unconditional. The original code deliberately suppressed printing of the first error message so it could quietly sense TPM_ERR_INVALID_POSTINIT.

Fixes a502feb67b47 ("tpm: Clean up reading of timeout and duration capabilities")
---
 drivers/char/tpm/tpm-interface.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/char/tpm/tpm-interface.c +6 -3
index a2688ac2b48f3c..c90f4f6d7871be 100644
@@ -523,8 +523,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
 		return 0;
 	}
 
-	rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap,
-			"attempting to determine the timeouts");
+	rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, NULL);
 	if (rc == TPM_ERR_INVALID_POSTINIT) {
 		/* The TPM is not started, we are the first to talk to it.
 		   Execute a startup command. */
@@ -535,8 +534,12 @@ int tpm_get_timeouts(struct tpm_chip *chip)
 		rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap,
 				"attempting to determine the timeouts");
 	}
-	if (rc)
+	if (rc) {
+		dev_err(&chip->dev,
+			"A TPM error (%zd) occurred attempting to determine the timeouts\n",
+			rc);
 		return rc;
+	}
 
 	old_timeout[0] = be32_to_cpu(cap.timeout.a);
 	old_timeout[1] = be32_to_cpu(cap.timeout.b);
Signature · 1 line
2.1.4

A message that is not in the archive (sent off-list or not yet received): 20161121183109.GA18088-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org

Reviewed-by
#2 lore raw
Quoted 7 lines · Jason Gunthorpe

This is a regression when this code was reworked and made the error print unconditional. The original code deliberately suppressed printing of the first error message so it could quietly sense TPM_ERR_INVALID_POSTINIT.

Fixes a502feb67b47 ("tpm: Clean up reading of timeout and duration capabilities")

Thanks for spotting this out. I'll apply this tomorrow (i.e. not tonight) as it does not endager stability in anyway as I test the other fixes.

/Jarkko

Quoted 34 lines
---
 drivers/char/tpm/tpm-interface.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/char/tpm/tpm-interface.c +6 -3
index a2688ac2b48f3c..c90f4f6d7871be 100644
@@ -523,8 +523,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
 		return 0;
 	}
 
-	rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap,
-			"attempting to determine the timeouts");
+	rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, NULL);
 	if (rc == TPM_ERR_INVALID_POSTINIT) {
 		/* The TPM is not started, we are the first to talk to it.
 		   Execute a startup command. */
@@ -535,8 +534,12 @@ int tpm_get_timeouts(struct tpm_chip *chip)
 		rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap,
 				"attempting to determine the timeouts");
 	}
-	if (rc)
+	if (rc) {
+		dev_err(&chip->dev,
+			"A TPM error (%zd) occurred attempting to determine the timeouts\n",
+			rc);
 		return rc;
+	}
 
 	old_timeout[0] = be32_to_cpu(cap.timeout.a);
 	old_timeout[1] = be32_to_cpu(cap.timeout.b);
Signature · 1 line
2.1.4

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

#3 lore raw
Quoted 6 lines · Jarkko Sakkinen
Quoted 5 lines · Jason Gunthorpe

This is a regression when this code was reworked and made the error print unconditional. The original code deliberately suppressed printing of the first error message so it could quietly sense TPM_ERR_INVALID_POSTINIT.

Quoted 8 lines
Quoted 2 lines
Fixes a502feb67b47 ("tpm: Clean up reading of timeout and duration capabilities")

Thanks for spotting this out. I'll apply this tomorrow (i.e. not tonight) as it does not endager stability in anyway as I test the other fixes.

Hey Jarkko, I couldn't find this patch in your trees and it didn't make 4.10-rc1, can you please pick it up?

https://patchwork.kernel.org/patch/9439931/

Quoted 32 lines

/Jarkko

Quoted 30 lines
 drivers/char/tpm/tpm-interface.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/char/tpm/tpm-interface.c +7 -3
index a2688ac2b48f3c..c90f4f6d7871be 100644
+++ b/drivers/char/tpm/tpm-interface.c
@@ -523,8 +523,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
 		return 0;
 	}
 
-	rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap,
-			"attempting to determine the timeouts");
+	rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, NULL);
 	if (rc == TPM_ERR_INVALID_POSTINIT) {
 		/* The TPM is not started, we are the first to talk to it.
 		   Execute a startup command. */
@@ -535,8 +534,12 @@ int tpm_get_timeouts(struct tpm_chip *chip)
 		rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap,
 				"attempting to determine the timeouts");
 	}
-	if (rc)
+	if (rc) {
+		dev_err(&chip->dev,
+			"A TPM error (%zd) occurred attempting to determine the timeouts\n",
+			rc);
 		return rc;
+	}
 
 	old_timeout[0] = be32_to_cpu(cap.timeout.a);
 	old_timeout[1] = be32_to_cpu(cap.timeout.b);
Signature · 2 lines
Jason Gunthorpe <[email protected]>        (780)4406067x832
Chief Technology Officer, Obsidian Research Corp         Edmonton, Canada
#4 lore raw
Quoted 14 lines · Jason Gunthorpe
Quoted 13 lines · Jarkko Sakkinen
Quoted 7 lines · Jason Gunthorpe

This is a regression when this code was reworked and made the error print unconditional. The original code deliberately suppressed printing of the first error message so it could quietly sense TPM_ERR_INVALID_POSTINIT.

Fixes a502feb67b47 ("tpm: Clean up reading of timeout and duration capabilities")

Thanks for spotting this out. I'll apply this tomorrow (i.e. not tonight) as it does not endager stability in anyway as I test the other fixes.

Quoted 6 lines
Quoted 1 line

Hey Jarkko, I couldn't find this patch in your trees and it didn't make 4.10-rc1, can you please pick it up?

https://patchwork.kernel.org/patch/9439931/

My bad. It's applied now. Can you check that everything looks good and I'll ask James to take the fixes that I have to 4.10?

/Jarkko

Quoted 37 lines
Quoted 33 lines

/Jarkko

Quoted 30 lines
 drivers/char/tpm/tpm-interface.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/char/tpm/tpm-interface.c +7 -3
index a2688ac2b48f3c..c90f4f6d7871be 100644
+++ b/drivers/char/tpm/tpm-interface.c
@@ -523,8 +523,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
 		return 0;
 	}
 
-	rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap,
-			"attempting to determine the timeouts");
+	rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, NULL);
 	if (rc == TPM_ERR_INVALID_POSTINIT) {
 		/* The TPM is not started, we are the first to talk to it.
 		   Execute a startup command. */
@@ -535,8 +534,12 @@ int tpm_get_timeouts(struct tpm_chip *chip)
 		rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap,
 				"attempting to determine the timeouts");
 	}
-	if (rc)
+	if (rc) {
+		dev_err(&chip->dev,
+			"A TPM error (%zd) occurred attempting to determine the timeouts\n",
+			rc);
 		return rc;
+	}
 
 	old_timeout[0] = be32_to_cpu(cap.timeout.a);
 	old_timeout[1] = be32_to_cpu(cap.timeout.b);
Signature · 2 lines
Jason Gunthorpe <[email protected]>        (780)4406067x832
Chief Technology Officer, Obsidian Research Corp         Edmonton, Canada