Messages in this thread | | | Date | Sat, 13 Sep 2014 14:56:29 +0300 | From | Jarkko Sakkinen <> | Subject | Re: [tpmdd-devel] [PATCH] tpm: use tpm_pcr_read_dev() in tpm_do_selftest() |
| |
On Fri, Sep 12, 2014 at 09:59:05AM -0600, Jason Gunthorpe wrote: > On Fri, Sep 12, 2014 at 04:06:41PM +0300, Jarkko Sakkinen wrote: > > It does not make sense to construct the PCR read command in > > tpm_do_selftest() when there is already a function that does > > the job. > > This would seem to undo an older patch, I don't think things have > changed enough for that to make sense.. Can you comment?
Right. Sorry, I should have used git blame before jumping into this.
> It looks to me like the aim was to not print a warning on faliure > while looping, tpm_pcr_read_dev will print an error. > > Though, it would be better to use transmit_cmd with a null desc than > tpm_transmit. > > Also: > > - if (rc < TPM_HEADER_SIZE) > + if (rc < 0) > return -EFAULT; > > Should be return rc; > > commit 24ebe6670de3d1f0dca11c9eb372134c7ab05503 > Author: Rajiv Andrade <srajiv@linux.vnet.ibm.com> > Date: Tue Apr 24 17:38:17 2012 -0300 > > TPM: chip disabled state erronously being reported as error > > 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. > > Reported-and-tested-by: Paul Bolle <pebolle@tiscali.nl> > Cc: Stable <stable@vger.kernel.org> > Signed-off-by: Rajiv Andrade <srajiv@linux.vnet.ibm.com> > > diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c > index ad7c7320dd1b..08427abf5fa5 100644 > --- a/drivers/char/tpm/tpm.c > +++ b/drivers/char/tpm/tpm.c > @@ -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,15 @@ 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; > + > + 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); >
/Jarkko
| |