linux-kernel - Re: [PATCH] tpm: Actually fail on TPM errors during "get random"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAGXu5jLuPTvcQbhEXKfh0fgWkA4g__kET=tbY8njraXXQYug+Q@mail.gmail.com>
Date:   Mon, 1 Apr 2019 11:54:40 -0700
From:   Kees Cook <keescook@...omium.org>
To:     James Bottomley <James.Bottomley@...senpartnership.com>
Cc:     Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>,
        Tomas Winkler <tomas.winkler@...el.com>,
        Phil Baker <baker1tex@...il.com>,
        Craig Robson <craig@...tt.com>,
        Laura Abbott <labbott@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Peter Huewe <peterhuewe@....de>,
        Jason Gunthorpe <jgg@...pe.ca>, Arnd Bergmann <arnd@...db.de>,
        linux-integrity <linux-integrity@...r.kernel.org>
Subject: Re: [PATCH] tpm: Actually fail on TPM errors during "get random"

On Mon, Apr 1, 2019 at 11:52 AM James Bottomley
<James.Bottomley@...senpartnership.com> 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