| Message ID | 20161122124007.16487-1-flus@cesar.org.br (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Nov 22, 2016 at 09:40:07AM -0300, Fabio Urquiza wrote: > If the variable expected_len is greater than 35 bytes, i2c_atmel_recv() > ignores the amount of data already read in i2c_atmel_read_status() and > request more data than what the device is ready to supply. As result the > TPM data sent to the upper layers will miss the first 35 bytes of the > response and will be filled with garbage in the end. I'm concerned your chip is not behaving the same as mine, I have to dig out my hardware and check. I'm fairly certain I would have hit this.. Do you know the revision code for your chip? IIRC my TPM re-read the message starting from byte 0, which is why the code is like this. Jason ------------------------------------------------------------------------------
On Tue, Nov 22, 2016 at 2:08 PM, Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote: > > On Tue, Nov 22, 2016 at 09:40:07AM -0300, Fabio Urquiza wrote: > > If the variable expected_len is greater than 35 bytes, i2c_atmel_recv() > > ignores the amount of data already read in i2c_atmel_read_status() and > > request more data than what the device is ready to supply. As result the > > TPM data sent to the upper layers will miss the first 35 bytes of the > > response and will be filled with garbage in the end. > > I'm concerned your chip is not behaving the same as mine, I have to > dig out my hardware and check. I'm fairly certain I would have hit > this.. > > Do you know the revision code for your chip? > > IIRC my TPM re-read the message starting from byte 0, which is why the > code is like this. > > Jason I'm working on an emulator for that chip and I don't have the real hardware to check. That behavior makes sense. I will change the emulator to follow that. You can ignore that patch, then. Thanks, -Fabio Urquiza ------------------------------------------------------------------------------
On Tue, Nov 22, 2016 at 02:47:37PM -0300, Fábio Urquiza wrote: > On Tue, Nov 22, 2016 at 2:08 PM, Jason Gunthorpe > <jgunthorpe@obsidianresearch.com> wrote: > > > > On Tue, Nov 22, 2016 at 09:40:07AM -0300, Fabio Urquiza wrote: > > > If the variable expected_len is greater than 35 bytes, i2c_atmel_recv() > > > ignores the amount of data already read in i2c_atmel_read_status() and > > > request more data than what the device is ready to supply. As result the > > > TPM data sent to the upper layers will miss the first 35 bytes of the > > > response and will be filled with garbage in the end. > > > > I'm concerned your chip is not behaving the same as mine, I have to > > dig out my hardware and check. I'm fairly certain I would have hit > > this.. > > > > Do you know the revision code for your chip? > > > > IIRC my TPM re-read the message starting from byte 0, which is why the > > code is like this. > > > > Jason > > I'm working on an emulator for that chip and I don't have the real > hardware to check. > > That behavior makes sense. I will change the emulator to follow that. > > You can ignore that patch, then. > > Thanks, > > -Fabio Urquiza Thanks guys. I was going to ask if someone is able to test this. Good that you sorted this out. /Jarkko ------------------------------------------------------------------------------
diff --git a/drivers/char/tpm/tpm_i2c_atmel.c b/drivers/char/tpm/tpm_i2c_atmel.c index 95ce2e9..a02f5a7 100644 --- a/drivers/char/tpm/tpm_i2c_atmel.c +++ b/drivers/char/tpm/tpm_i2c_atmel.c @@ -75,9 +75,9 @@ static int i2c_atmel_recv(struct tpm_chip *chip, u8 *buf, size_t count) struct tpm_output_header *hdr = (struct tpm_output_header *)priv->buffer; u32 expected_len; - int rc; + int rc = priv->len; - if (priv->len == 0) + if (rc == 0) return -EIO; /* Get the message size from the message header, if we didn't get the @@ -87,7 +87,7 @@ static int i2c_atmel_recv(struct tpm_chip *chip, u8 *buf, size_t count) if (expected_len > count) return -ENOMEM; - if (priv->len >= expected_len) { + if (rc >= expected_len) { dev_dbg(&chip->dev, "%s early(buf=%*ph count=%0zx) -> ret=%d\n", __func__, (int)min_t(size_t, 64, expected_len), buf, count, @@ -96,7 +96,8 @@ static int i2c_atmel_recv(struct tpm_chip *chip, u8 *buf, size_t count) return expected_len; } - rc = i2c_master_recv(client, buf, expected_len); + memcpy(buf, priv->buffer, rc); + rc += i2c_master_recv(client, buf + rc, expected_len - rc); dev_dbg(&chip->dev, "%s reread(buf=%*ph count=%0zx) -> ret=%d\n", __func__, (int)min_t(size_t, 64, expected_len), buf, count,
If the variable expected_len is greater than 35 bytes, i2c_atmel_recv() ignores the amount of data already read in i2c_atmel_read_status() and request more data than what the device is ready to supply. As result the TPM data sent to the upper layers will miss the first 35 bytes of the response and will be filled with garbage in the end. TCSP_GetRandom_Internal before fix: tpm_i2c_atmel 0-0020: i2c_atmel_send(buf=00 c1 00 00 00 0e 00 00 00 46 00 00 00 20 len=e) -> sts=14 tpm_i2c_atmel 0-0020: i2c_atmel_read_status: sts=-6 tpm_i2c_atmel 0-0020: i2c_atmel_read_status: sts=35 tpm_i2c_atmel 0-0020: i2c_atmel_recv reread(buf=3b ec a5 17 37 27 2a fb a0 cc ce ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff count=1000) -> ret=46 TCSP_GetRandom_Internal after fix: tpm_i2c_atmel 0-0020: i2c_atmel_send(buf=00 c1 00 00 00 0e 00 00 00 46 00 00 00 20 len=e) -> sts=14 tpm_i2c_atmel 0-0020: i2c_atmel_read_status: sts=-6 tpm_i2c_atmel 0-0020: i2c_atmel_read_status: sts=35 tpm_i2c_atmel 0-0020: i2c_atmel_recv reread(buf=00 c4 00 00 00 2e 00 00 00 00 00 00 00 20 63 dc 83 a1 55 e6 b4 5d 5a 10 70 63 28 5c 5b a8 87 ca 57 fd 45 c3 a0 62 1b c2 1d b3 d2 0d 8f 19 count=1000) -> ret=46 Signed-off-by: Fabio Urquiza <flus@cesar.org.br> --- drivers/char/tpm/tpm_i2c_atmel.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)