Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Message-ID: <1554144741.3029.18.camel@HansenPartnership.com>
Subject: Re: [PATCH] tpm: Actually fail on TPM errors during "get random"
From:   James Bottomley <James.Bottomley@HansenPartnership.com>
To:     Kees Cook <keescook@chromium.org>,
        Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
        Tomas Winkler <tomas.winkler@intel.com>
Cc:     Phil Baker <baker1tex@gmail.com>, Craig Robson <craig@zhatt.com>,
        Laura Abbott <labbott@redhat.com>,
        linux-kernel@vger.kernel.org, Peter Huewe <peterhuewe@gmx.de>,
        Jason Gunthorpe <jgg@ziepe.ca>, Arnd Bergmann <arnd@arndb.de>,
        linux-integrity@vger.kernel.org
Date:   Mon, 01 Apr 2019 11:52:21 -0700
In-Reply-To: <20190401183211.GA21228@beast>
References: <20190401183211.GA21228@beast>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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.

James


> +fail:
> +	if (rc < 0)
> +		return rc;
> +	return -EIO;
>  }
>  
>  #define TPM_ORD_PCRREAD 21
> -- 
> 2.17.1
> 
>