On Mon, Apr 1, 2019 at 11:52 AM James Bottomley
<James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Mon, 2019-04-01 at 11:32 -0700, Kees Cook wrote:
> [...]
> > --- a/drivers/char/tpm/tpm1-cmd.c
> > +++ b/drivers/char/tpm/tpm1-cmd.c
> > @@ -510,7 +510,7 @@ struct tpm1_get_random_out {
> > *
> > * Return:
> > * * number of bytes read
> > - * * -errno or a TPM return code otherwise
> > + * * -errno (positive TPM return codes are masked to -EIO)
> > */
> > int tpm1_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
> > {
> > @@ -524,7 +524,7 @@ int tpm1_get_random(struct tpm_chip *chip, u8
> > *dest, size_t max)
> >
> > rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND,
> > TPM_ORD_GET_RANDOM);
> > if (rc)
> > - return rc;
> > + goto fail;
> >
> > do {
> > tpm_buf_append_u32(&buf, num_bytes);
> > @@ -559,7 +559,10 @@ int tpm1_get_random(struct tpm_chip *chip, u8
> > *dest, size_t max)
> > rc = total ? (int)total : -EIO;
> > out:
> > tpm_buf_destroy(&buf);
> > - return rc;
>
> You can't remove this otherwise the only return will ever be a failure.
>
> I think what you're trying to catch is tpm_transmit_cmd returning a
> positive failure, So you need to check the output of tpm_transmit_cmd
> as well and goto failure leaving the above return in place.
eek, yes. I double-checked this in tpm2, but tpm1 is different...
--
Kees Cook
