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
| ||
|
Date: Wed, 05 Apr 2017 12:05:32 +0200 From: Peter Huewe <peterhuewe@....de> To: Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>, Enric Balletbo i Serra <enric.balletbo@...labora.com> CC: Andrew Lunn <andrew@...n.ch>, tpmdd-devel@...ts.sourceforge.net, linux-kernel@...r.kernel.org, wsa@...-dreams.de, Bryan Freed <bfreed@...omium.org> Subject: Re: [PATCH v3] tpm: Apply a sane minimum adapterlimit value for retransmission. Am 5. April 2017 11:03:27 MESZ schrieb Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>: >On Tue, Mar 28, 2017 at 05:29:38PM +0200, Enric Balletbo i Serra wrote: >> From: Bryan Freed <bfreed@...omium.org> >> >> When the I2C Infineon part is attached to an I2C adapter that imposes >> a size limitation, large requests will fail with -EOPNOTSUPP. Retry >> them with a sane minimum size without re-issuing the 0x05 command >> as this appears to occasionally put the TPM in a bad state. >> >> Signed-off-by: Bryan Freed <bfreed@...omium.org> >> [rework the patch to adapt to the feedback received] >> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@...labora.com> >> --- >> This is a reworked version of the original patch based on the >> suggestion made by Wolfram Sang to simply fall back to a sane minimum >> when the maximum fails. >> >> Changes since v2: >> - Do not remove faster transfers when chip is SLB9645 (Peter Huewe) >> - Remember the adapterlimit length once it fails to not generate >extra >> i2c core messages (suggested by Andrew Lunn) >> Changes since v1: >> - Check the correct return value (-EOPNOTSUPP instead of -EINVAL) >> - Fall back len to I2C_SMBUS_BLOCK_MAX if fails. >> >> drivers/char/tpm/tpm_i2c_infineon.c | 76 >+++++++++++++++++++++++++++---------- >> 1 file changed, 56 insertions(+), 20 deletions(-) >> >> diff --git a/drivers/char/tpm/tpm_i2c_infineon.c >b/drivers/char/tpm/tpm_i2c_infineon.c >> index 62ee44e..fdefcdb 100644 >> --- a/drivers/char/tpm/tpm_i2c_infineon.c >> +++ b/drivers/char/tpm/tpm_i2c_infineon.c >> @@ -70,6 +70,7 @@ struct tpm_inf_dev { >> u8 buf[TPM_BUFSIZE + sizeof(u8)]; /* max. buffer size + addr */ >> struct tpm_chip *chip; >> enum i2c_chip_type chip_type; >> + unsigned int adapterlimit; >> }; >> >> static struct tpm_inf_dev tpm_dev; >> @@ -111,6 +112,7 @@ static int iic_tpm_read(u8 addr, u8 *buffer, >size_t len) >> >> int rc = 0; >> int count; >> + unsigned int msglen = len; >> >> /* Lock the adapter for the duration of the whole sequence. */ >> if (!tpm_dev.client->adapter->algo->master_xfer) >> @@ -131,27 +133,61 @@ static int iic_tpm_read(u8 addr, u8 *buffer, >size_t len) >> usleep_range(SLEEP_DURATION_LOW, SLEEP_DURATION_HI); >> } >> } else { >> - /* slb9635 protocol should work in all cases */ >> - for (count = 0; count < MAX_COUNT; count++) { >> - rc = __i2c_transfer(tpm_dev.client->adapter, &msg1, 1); >> - if (rc > 0) >> - break; /* break here to skip sleep */ >> - >> - usleep_range(SLEEP_DURATION_LOW, SLEEP_DURATION_HI); >> - } >> - >> - if (rc <= 0) >> - goto out; >> - >> - /* After the TPM has successfully received the register address >> - * it needs some time, thus we're sleeping here again, before >> - * retrieving the data >> + /* Expect to send one command message and one data message, but >> + * support looping over each or both if necessary. >> */ >> - for (count = 0; count < MAX_COUNT; count++) { >> - usleep_range(SLEEP_DURATION_LOW, SLEEP_DURATION_HI); >> - rc = __i2c_transfer(tpm_dev.client->adapter, &msg2, 1); >> - if (rc > 0) >> - break; >> + while (len > 0) { >> + /* slb9635 protocol should work in all cases */ >> + for (count = 0; count < MAX_COUNT; count++) { >> + rc = __i2c_transfer(tpm_dev.client->adapter, >> + &msg1, 1); >> + if (rc > 0) >> + break; /* break here to skip sleep */ >> + >> + usleep_range(SLEEP_DURATION_LOW, >> + SLEEP_DURATION_HI); >> + } >> + >> + if (rc <= 0) >> + goto out; >> + >> + /* After the TPM has successfully received the register >> + * address it needs some time, thus we're sleeping here >> + * again, before retrieving the data >> + */ >> + for (count = 0; count < MAX_COUNT; count++) { >> + if (tpm_dev.adapterlimit) { >> + msglen = min_t(unsigned int, >> + tpm_dev.adapterlimit, >> + len); >> + msg2.len = msglen; >> + } >> + usleep_range(SLEEP_DURATION_LOW, >> + SLEEP_DURATION_HI); >> + rc = __i2c_transfer(tpm_dev.client->adapter, >> + &msg2, 1); >> + if (rc > 0) { >> + /* Since len is unsigned, make doubly >> + * sure we do not underflow it. >> + */ >> + if (msglen > len) >> + len = 0; >> + else >> + len -= msglen; >> + msg2.buf += msglen; >> + break; >> + } >> + /* If the I2C adapter rejected the request (e.g >> + * when the quirk read_max_len < len) fall back >> + * to a sane minimum value and try again. >> + */ >> + if (rc == -EOPNOTSUPP) >> + tpm_dev.adapterlimit = >> + I2C_SMBUS_BLOCK_MAX; >> + } >> + >> + if (rc <= 0) >> + goto out; >> } >> } >> >> -- >> 2.9.3 >> > >Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com> > >Peter, Andrew, anyone: Tested-by? > Not yet, I'll put it on my list to test. Hopefully by next tuesday. Peter >/Jarkko -- Sent from my mobile
Powered by blists - more mailing lists