tpm: read burstcount from TPM_STS in one 32-bit transaction

tpm: read burstcount from TPM_STS in one 32-bit transaction

[Michal Suchánek]

Hello,

in commit 9754d45e9970 ("tpm: read burstcount from TPM_STS in one
32-bit transaction") you change reading of two 8-bit values to one
32bit read. This is obviously wrong wrt endianess unless the
underlying tpm_tis_read32 does endian conversion. 

Looking at the implementation 
static inline int tpm_tis_read32(struct tpm_tis_data *data, u32 addr,
                                 u32 *result)
{
        return data->phy_ops->read32(data, addr, result);
}

it calls read32 which has two implementations:

static const struct tpm_tis_phy_ops tpm_tcg = {
	.read32 = tpm_tcg_read32,

static int tpm_tcg_read32(struct tpm_tis_data *data, u32 addr, u32
*result) {
        struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);

        *result = ioread32(phy->iobase + addr);
       return 0;
}

static const struct tpm_tis_phy_ops tpm_spi_phy_ops = {
	.read32 = tpm_tis_spi_read32,

static int tpm_tis_spi_read32(struct tpm_tis_data *data, u32 addr, u32
*result) {
        int rc;

        rc = data->phy_ops->read_bytes(data, addr, sizeof(u32), (u8
        *)result); if (!rc)
                *result = le32_to_cpu(*result);
        return rc;
}

meaning that unless you are on LE where le32_to_cpu is a noop these
functions do completely different thing. So presumably this is
completely broken on BE. 

Presumably only the SPI variant can be actually used with TPM devices
bolted on after the fact so it is more likely correct for obscure
hardware. Conseqently tpm_tcg_read32 should use
le32_to_cpu(ioread32(phy->iobase + addr)) in case somebody manages to
map a TPM into io-space on a BE machine.

Thanks

Michal

Re: [tpmdd-devel] tpm: read burstcount from TPM_STS in one 32-bit transaction

[James Bottomley]

On Tue, 2017-07-25 at 15:04 +0200, Michal Suchánek wrote:
> Hello,
> 
> in commit 9754d45e9970 ("tpm: read burstcount from TPM_STS in one
> 32-bit transaction") you change reading of two 8-bit values to one
> 32bit read. This is obviously wrong wrt endianess unless the
> underlying tpm_tis_read32 does endian conversion. 

Some of the bus read primitives do do endianness conversions.  The
problem is with the SPI attachment, which has unclear endianness.  A
standard PCI bus attachment uses ioread32() which automatically
transforms from a little endian bus to the cpu endianness, however SPI
is forced to transfer the bytes one at a time over the serial bus and
then transform.  The assumption seems to be that the TIS TPM is
replying in little endian format when SPI connected.

We can probably get the PPC people to confirm this, I believe they have
a SPI attached TPM.

James

Re: [tpmdd-devel] tpm: read burstcount from TPM_STS in one 32-bit transaction

[Michal Suchánek]

On Tue, 25 Jul 2017 10:36:11 -0700
James Bottomley <jejb@linux.vnet.ibm.com> wrote:

> On Tue, 2017-07-25 at 15:04 +0200, Michal Suchánek wrote:
> > Hello,
> > 
> > in commit 9754d45e9970 ("tpm: read burstcount from TPM_STS in one
> > 32-bit transaction") you change reading of two 8-bit values to one
> > 32bit read. This is obviously wrong wrt endianess unless the
> > underlying tpm_tis_read32 does endian conversion.   
> 
> Some of the bus read primitives do do endianness conversions.  The
> problem is with the SPI attachment, which has unclear endianness.  A
> standard PCI bus attachment uses ioread32() which automatically
> transforms from a little endian bus to the cpu endianness, however SPI
> is forced to transfer the bytes one at a time over the serial bus and
> then transform.  The assumption seems to be that the TIS TPM is
> replying in little endian format when SPI connected.
> 

Yes, that makes sense.

Thanks for clarification.

Michal

Re: tpm: read burstcount from TPM_STS in one 32-bit transaction

[Jarkko Sakkinen]

On Tue, Jul 25, 2017 at 08:17:58PM +0200, Michal Suchánek wrote:
> On Tue, 25 Jul 2017 10:36:11 -0700
> James Bottomley <jejb-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> wrote:
> 
> > On Tue, 2017-07-25 at 15:04 +0200, Michal Suchánek wrote:
> > > Hello,
> > > 
> > > in commit 9754d45e9970 ("tpm: read burstcount from TPM_STS in one
> > > 32-bit transaction") you change reading of two 8-bit values to one
> > > 32bit read. This is obviously wrong wrt endianess unless the
> > > underlying tpm_tis_read32 does endian conversion.   
> > 
> > Some of the bus read primitives do do endianness conversions.  The
> > problem is with the SPI attachment, which has unclear endianness.  A
> > standard PCI bus attachment uses ioread32() which automatically
> > transforms from a little endian bus to the cpu endianness, however SPI
> > is forced to transfer the bytes one at a time over the serial bus and
> > then transform.  The assumption seems to be that the TIS TPM is
> > replying in little endian format when SPI connected.
> > 
> 
> Yes, that makes sense.
> 
> Thanks for clarification.
> 
> Michal

Thank you for reporting this and thanks James for explaining this.

I do not have access to PPC hardware with SPI-TPM.

/Jarkko

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

Re: tpm: read burstcount from TPM_STS in one 32-bit transaction

[George Wilson]

On Tue, Jul 25, 2017 at 10:36:11AM -0700, James Bottomley wrote:
> On Tue, 2017-07-25 at 15:04 +0200, Michal Suchánek wrote:
> > Hello,
> > 
> > in commit 9754d45e9970 ("tpm: read burstcount from TPM_STS in one
> > 32-bit transaction") you change reading of two 8-bit values to one
> > 32bit read. This is obviously wrong wrt endianess unless the
> > underlying tpm_tis_read32 does endian conversion. 
> 
> Some of the bus read primitives do do endianness conversions.  The
> problem is with the SPI attachment, which has unclear endianness.  A
> standard PCI bus attachment uses ioread32() which automatically
> transforms from a little endian bus to the cpu endianness, however SPI
> is forced to transfer the bytes one at a time over the serial bus and
> then transform.  The assumption seems to be that the TIS TPM is
> replying in little endian format when SPI connected.
> 
> We can probably get the PPC people to confirm this, I believe they have
> a SPI attached TPM.

All the current OpenPOWER hardware designs I'm aware of have the TPM on
I2C.  Trusted Computing support in OpenPOWER firmware depends on it
being on I2C.

> 
> James
> 
> 
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> tpmdd-devel mailing list
> tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel


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