Skip to contentSkip to conversation

TPM: chip disabled state erronously being reported as error

PATCH In review Rajiv Andrade 2 participants 2 messages Apr 24, 2012 – Apr 25, 2012 TPM device driver

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 TPM: chip disabled state erronously being reported as error

Hi Paul,

Can you test it?

Thanks,
Rajiv

---
tpm_do_selftest() attempts to read a PCR in order to decide if one can rely on the TPM being used or not. The function that's used by __tpm_pcr_read() does not expect the TPM to be disabled or deactivated, and if so, reports an error.

It's fine if the TPM returns this error when trying to use it for the first time after a power cycle, but it's definitely not if it already returned success for a previous attempt to read one of its PCRs.

The tpm_do_selftest() was modified so that the driver only reports this return code as an error when it really is.

---
 drivers/char/tpm/tpm.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)
drivers/char/tpm/tpm.c +11 -2
index ad7c732..9511abd 100644
@@ -827,10 +827,10 @@ EXPORT_SYMBOL_GPL(tpm_pcr_extend);
 int tpm_do_selftest(struct tpm_chip *chip)
 {
 	int rc;
-	u8 digest[TPM_DIGEST_SIZE];
 	unsigned int loops;
 	unsigned int delay_msec = 1000;
 	unsigned long duration;
+	struct tpm_cmd_t cmd;
 
 	duration = tpm_calc_ordinal_duration(chip,
 	                                     TPM_ORD_CONTINUE_SELFTEST);
@@ -845,7 +845,16 @@ int tpm_do_selftest(struct tpm_chip *chip)
 		return rc;
 
 	do {
-		rc = __tpm_pcr_read(chip, 0, digest);
+		/* Attempt to read a PCR value */
+		cmd.header.in = pcrread_header;
+		cmd.params.pcrread_in.pcr_idx = cpu_to_be32(0);
+		rc = tpm_transmit(chip, (u8 *) &cmd, READ_PCR_RESULT_SIZE);
+
+		if (rc < TPM_HEADER_SIZE)
+			return -EFAULT;
+		else
+			rc = be32_to_cpu(cmd.header.out.return_code);
+
 		if (rc == TPM_ERR_DISABLED || rc == TPM_ERR_DEACTIVATED) {
 			dev_info(chip->dev,
 				 "TPM is disabled/deactivated (0x%X)\n", rc);
Signature · 1 line
1.7.3.4
#1 lore raw

Rajiv,

Quoted 1 line · Rajiv Andrade

Can you test it?

Sure. Your patch applied cleanly to the current Fedora 16 kernel's sources (ie, kernel-3.3.2-6.fc16). One boot and one suspend-and-resume cycle both were without the error message that bothered me so much.

That should be all testing that this patch needs, shouldn't it?

Quoted 11 lines

tpm_do_selftest() attempts to read a PCR in order to decide if one can rely on the TPM being used or not. The function that's used by __tpm_pcr_read() does not expect the TPM to be disabled or deactivated, and if so, reports an error.

It's fine if the TPM returns this error when trying to use it for the first time after a power cycle, but it's definitely not if it already returned success for a previous attempt to read one of its PCRs.

Quoted 4 lines

The tpm_do_selftest() was modified so that the driver only reports this return code as an error when it really is.

I guess that is now "Reported-and-tested-by", even though the test was run against a v3.3.2 based kernel.

Quoted 1 line

Should this be CC-ed to stable for v3.3?

Paul Bolle