Skip to contentSkip to conversation

tpm: fix RC value check in tpm2_seal_trusted

PATCH In review Jarkko Sakkinen 2 participants 7 messages Jan 25, 2017 – Jan 29, 2017 TPM device driver

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 tpm: fix RC value check in tpm2_seal_trusted

Fixes 5ca4c20cfd37 ("keys, trusted: select hash algorithm for TPM2 chips")
---
 drivers/char/tpm/tpm.h      | 5 +++++
 drivers/char/tpm/tpm2-cmd.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)
drivers/char/tpm/tpm.h +5 -0
index 4e5fb22..abc621e 100644
@@ -523,6 +523,11 @@ static inline void tpm_add_ppi(struct tpm_chip *chip)
 }
 #endif
 
+static inline inline u32 tpm2_rc_value(u32 rc)
+{
+	return (rc & BIT(7)) ? rc & 0xff : rc;
+}
+
 int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
 int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
 		    struct tpm2_digest *digests);
drivers/char/tpm/tpm2-cmd.c +1 -1
index 6c8174a..40b09ca 100644
@@ -551,7 +551,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 	tpm_buf_destroy(&buf);
 
 	if (rc > 0) {
-		if ((rc & TPM2_RC_HASH) == TPM2_RC_HASH)
+		if (tpm2_rc_value(rc) == TPM2_RC_HASH)
 			rc = -EINVAL;
 		else
 			rc = -EPERM;
Signature · 6 lines
2.9.3


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
#2 lore raw
Quoted 6 lines · Jason Gunthorpe
Quoted 2 lines · Jarkko Sakkinen
Fixes 5ca4c20cfd37 ("keys, trusted: select hash algorithm for TPM2 chips")

I think you need a commit message for this.. Is this following the spec?

Yes.

Format one commands the upper bits contain either handle, session or parameter index. Bit 7 tells whether it the error code is format zero or one. Format zero errors do not require masking. They do not have any data in addition to value.

The reason why this bug was repeated in TPM space code was that I originally melded that code form my trusted keys code (copy pasted and edited message construction).

"The error code handling is bogus as any error code that has the bits set that TPM_RC_HASH could pass. Implemented tpm2_rc_value() helper to parse the error value from FMT0 and FMT1 error codes to use to check the error so that these types of mistakes is prevented in the future."

Is that suitable or do you want me to add something?

Jarkko

#3 lore raw
Quoted 4 lines · Jarkko Sakkinen

"The error code handling is bogus as any error code that has the bits set that TPM_RC_HASH could pass. Implemented tpm2_rc_value() helper to parse the error value from FMT0 and FMT1 error codes to use to check the error so that these types of mistakes is prevented in the future."

Great thanks

Jason

#4 lore raw
Quoted 10 lines · Jason Gunthorpe
Quoted 4 lines · Jarkko Sakkinen

"The error code handling is bogus as any error code that has the bits set that TPM_RC_HASH could pass. Implemented tpm2_rc_value() helper to parse the error value from FMT0 and FMT1 error codes to use to check the error so that these types of mistakes is prevented in the future."

Great thanks

Jason

Can I put your Reviewed-by? I would like to get this into 4.11.

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot

Reviewed-by
#5 lore raw
Quoted 8 lines · Jarkko Sakkinen
Quoted 7 lines · Jason Gunthorpe
Quoted 4 lines · Jarkko Sakkinen

"The error code handling is bogus as any error code that has the bits set that TPM_RC_HASH could pass. Implemented tpm2_rc_value() helper to parse the error value from FMT0 and FMT1 error codes to use to check the error so that these types of mistakes is prevented in the future."

Quoted 5 lines
Quoted 3 lines

Great thanks

Jason

Can I put your Reviewed-by? I would like to get this into 4.11.

I'm not up to speed on the TPM2 parsing, but it looks OK based on your description.

Jason

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot

#6 lore raw
Quoted 13 lines · Jason Gunthorpe
Quoted 12 lines · Jarkko Sakkinen
Quoted 10 lines · Jason Gunthorpe
Quoted 4 lines · Jarkko Sakkinen

"The error code handling is bogus as any error code that has the bits set that TPM_RC_HASH could pass. Implemented tpm2_rc_value() helper to parse the error value from FMT0 and FMT1 error codes to use to check the error so that these types of mistakes is prevented in the future."

Great thanks

Jason

Quoted 8 lines
Quoted 1 line

Can I put your Reviewed-by? I would like to get this into 4.11.

I'm not up to speed on the TPM2 parsing, but it looks OK based on your description.

Jason

Thanks I applied this patch to master.

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot