diff mbox series

[v4,2/7] tpm: tpm_tis: Add check_data handle to tpm_tis_phy_ops in order to check data integrity

Message ID 20200331113207.107080-3-amirmizi6@gmail.com (mailing list archive)
State New, archived
Headers show
Series Add tpm i2c ptp driver | expand

Commit Message

Amir Mizinski March 31, 2020, 11:32 a.m. UTC 
From: Amir Mizinski <amirmizi6@gmail.com>

In order to compute the crc over the data sent in lower layer
 (I2C for instance), tpm_tis_check_data() calls an operation (if available)
 to check data integrity. If data integrity cannot be verified, a retry
 attempt to save the sent/received data is implemented.

The current steps are done when sending a command:
    1. Host writes to TPM_STS.commandReady.
    2. Host writes command.
    3. Host checks that TPM received data is valid.
    4. If data is currupted go to step 1.

When receiving data:
    1. Host checks that TPM_STS.dataAvail is set.
    2. Host saves received data.
    3. Host checks that received data is correct.
    4. If data is currupted Host writes to TPM_STS.responseRetry and go to
        step 1.

Co-developed-by: Christophe Richard <hristophe-h.ricard@st.com>
Signed-off-by: Christophe Richard <hristophe-h.ricard@st.com>
Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>
---
 drivers/char/tpm/tpm_tis_core.c | 102 +++++++++++++++++++++++++---------------
 drivers/char/tpm/tpm_tis_core.h |   3 ++
 2 files changed, 67 insertions(+), 38 deletions(-)

--
2.7.4



===========================================================================================
The privileged confidential information contained in this email is intended for use only by the addressees as indicated by the original sender of this email. If you are not the addressee indicated in this email or are not responsible for delivery of the email to such a person, please kindly reply to the sender indicating this fact and delete all copies of it from your computer and network server immediately. Your cooperation is highly appreciated. It is advised that any unauthorized use of confidential information of Nuvoton is strictly prohibited; and any information in this email irrelevant to the official business of Nuvoton shall be deemed as neither given nor endorsed by Nuvoton.

Comments

Jarkko Sakkinen April 1, 2020, 8:20 a.m. UTC | #1 
On Tue, Mar 31, 2020 at 02:32:02PM +0300, amirmizi6@gmail.com wrote:
> From: Amir Mizinski <amirmizi6@gmail.com>
> 
> In order to compute the crc over the data sent in lower layer
>  (I2C for instance), tpm_tis_check_data() calls an operation (if available)
>  to check data integrity. If data integrity cannot be verified, a retry
>  attempt to save the sent/received data is implemented.
> 
> The current steps are done when sending a command:
>     1. Host writes to TPM_STS.commandReady.
>     2. Host writes command.
>     3. Host checks that TPM received data is valid.
>     4. If data is currupted go to step 1.
> 
> When receiving data:
>     1. Host checks that TPM_STS.dataAvail is set.
>     2. Host saves received data.
>     3. Host checks that received data is correct.
>     4. If data is currupted Host writes to TPM_STS.responseRetry and go to
>         step 1.
> 
> Co-developed-by: Christophe Richard <hristophe-h.ricard@st.com>
> Signed-off-by: Christophe Richard <hristophe-h.ricard@st.com>
> Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>

The email is malformed.

So.. How did Christophe participate on writing this patch? I haven't
seen him shouting anything about the subject and still his SOB is
there.

/Jarkko
Benoit HOUYERE April 1, 2020, 8:18 p.m. UTC | #2 
>>On Tue, Mar 31, 2020 at 02:32:02PM +0300, amirmizi6@gmail.com wrote:
>> From: Amir Mizinski <amirmizi6@gmail.com>
>> 
>> In order to compute the crc over the data sent in lower layer  (I2C 
>> for instance), tpm_tis_check_data() calls an operation (if available)  
>> to check data integrity. If data integrity cannot be verified, a retry  
>> attempt to save the sent/received data is implemented.
>> 
>> The current steps are done when sending a command:
>>     1. Host writes to TPM_STS.commandReady.
>>     2. Host writes command.
>>     3. Host checks that TPM received data is valid.
>>     4. If data is currupted go to step 1.
>> 
>> When receiving data:
>>     1. Host checks that TPM_STS.dataAvail is set.
>>     2. Host saves received data.
>>     3. Host checks that received data is correct.
>>     4. If data is currupted Host writes to TPM_STS.responseRetry and go to
>>         step 1.
>> 
>> Co-developed-by: Christophe Richard <hristophe-h.ricard@st.com>
>> Signed-off-by: Christophe Richard <hristophe-h.ricard@st.com>
>> Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>

>The email is malformed.

>So.. How did Christophe participate on writing this patch? I haven't seen him shouting anything about the subject and still his SOB is there.

>/Jarkko

Christophe sent patch to support I2C TCG TPM driver tpm_tis_i2c (https://patchwork.kernel.org/patch/8628681/) in the same time that tpm_tis_spi. This function was named tpm_tis_i2c_check_data.

Best Regards,
Jarkko Sakkinen April 2, 2020, 8:17 p.m. UTC | #3 
On Wed, Apr 01, 2020 at 12:19:39PM +0300, Amir Mizinski wrote:
>    this commit is based on Christophe work and was submitted in 2016:
>    https://patchwork.kernel.org/patch/8628661/
>    I followed you comments from the previous version on commit 1/7:
>    https://lore.kernel.org/patchwork/patch/1192101/
>    and fixed it in this one too. should i write it differently?
>    thank you,
>    Amir Mizinski

No then it's correct thanks. Just wanted to verify.

Please configure your email client:

https://www.kernel.org/doc/html/v4.10/process/email-clients.html

/Jarkko
diff mbox series

Patch

diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 27c6ca0..6c4f232 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -242,6 +242,15 @@  static u8 tpm_tis_status(struct tpm_chip *chip)
        return status;
 }

+static bool tpm_tis_check_data(struct tpm_chip *chip, const u8 *buf, size_t len)
+{
+       struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
+
+       if (priv->phy_ops->check_data)
+               return priv->phy_ops->check_data(priv, buf, len);
+       return true;
+}
+
 static void tpm_tis_ready(struct tpm_chip *chip)
 {
        struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
@@ -308,47 +317,59 @@  static int tpm_tis_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 {
        struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
        int size = 0;
-       int status;
+       int status, i;
        u32 expected;
+       bool check_data = false;

-       if (count < TPM_HEADER_SIZE) {
-               size = -EIO;
-               goto out;
-       }
+       for (i = 0; i < TPM_RETRY; i++) {
+               if (count < TPM_HEADER_SIZE) {
+                       size = -EIO;
+                       goto out;
+               }

-       size = recv_data(chip, buf, TPM_HEADER_SIZE);
-       /* read first 10 bytes, including tag, paramsize, and result */
-       if (size < TPM_HEADER_SIZE) {
-               dev_err(&chip->dev, "Unable to read header\n");
-               goto out;
-       }
+               size = recv_data(chip, buf, TPM_HEADER_SIZE);
+               /* read first 10 bytes, including tag, paramsize, and result */
+               if (size < TPM_HEADER_SIZE) {
+                       dev_err(&chip->dev, "Unable to read header\n");
+                       goto out;
+               }

-       expected = be32_to_cpu(*(__be32 *) (buf + 2));
-       if (expected > count || expected < TPM_HEADER_SIZE) {
-               size = -EIO;
-               goto out;
-       }
+               expected = be32_to_cpu(*(__be32 *) (buf + 2));
+               if (expected > count || expected < TPM_HEADER_SIZE) {
+                       size = -EIO;
+                       goto out;
+               }

-       size += recv_data(chip, &buf[TPM_HEADER_SIZE],
-                         expected - TPM_HEADER_SIZE);
-       if (size < expected) {
-               dev_err(&chip->dev, "Unable to read remainder of result\n");
-               size = -ETIME;
-               goto out;
-       }
+               size += recv_data(chip, &buf[TPM_HEADER_SIZE],
+                                 expected - TPM_HEADER_SIZE);
+               if (size < expected) {
+                       dev_err(&chip->dev, "Unable to read remainder of result\n");
+                       size = -ETIME;
+                       goto out;
+               }

-       if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
-                               &priv->int_queue, false) < 0) {
-               size = -ETIME;
-               goto out;
+               if (wait_for_tpm_stat(chip, TPM_STS_VALID, chip->timeout_c,
+                                     &priv->int_queue, false) < 0) {
+                       size = -ETIME;
+                       goto out;
+               }
+
+               status = tpm_tis_status(chip);
+               if (status & TPM_STS_DATA_AVAIL) {      /* retry? */
+                       dev_err(&chip->dev, "Error left over data\n");
+                       size = -EIO;
+                       goto out;
+               }
+
+               check_data = tpm_tis_check_data(chip, buf, size);
+               if (!check_data)
+                       tpm_tis_write8(priv, TPM_STS(priv->locality),
+                                      TPM_STS_RESPONSE_RETRY);
+               else
+                       break;
        }
-       status = tpm_tis_status(chip);
-       if (status & TPM_STS_DATA_AVAIL) {      /* retry? */
-               dev_err(&chip->dev, "Error left over data\n");
+       if (!check_data)
                size = -EIO;
-               goto out;
-       }
-
 out:
        tpm_tis_ready(chip);
        return size;
@@ -453,14 +474,19 @@  static void disable_interrupts(struct tpm_chip *chip)
 static int tpm_tis_send_main(struct tpm_chip *chip, const u8 *buf, size_t len)
 {
        struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
-       int rc;
+       int rc, i;
        u32 ordinal;
        unsigned long dur;
+       bool data_valid = false;

-       rc = tpm_tis_send_data(chip, buf, len);
-       if (rc < 0)
-               return rc;
-
+       for (i = 0; i < TPM_RETRY && !data_valid; i++) {
+               rc = tpm_tis_send_data(chip, buf, len);
+               if (rc < 0)
+                       return rc;
+               data_valid = tpm_tis_check_data(chip, buf, len);
+       }
+       if (!data_valid)
+               return -EIO;
        /* go and do it */
        rc = tpm_tis_write8(priv, TPM_STS(priv->locality), TPM_STS_GO);
        if (rc < 0)
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index d06c65b..486c2e9 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -34,6 +34,7 @@  enum tis_status {
        TPM_STS_GO = 0x20,
        TPM_STS_DATA_AVAIL = 0x10,
        TPM_STS_DATA_EXPECT = 0x08,
+       TPM_STS_RESPONSE_RETRY = 0x02,
 };

 enum tis_int_flags {
@@ -106,6 +107,8 @@  struct tpm_tis_phy_ops {
        int (*read16)(struct tpm_tis_data *data, u32 addr, u16 *result);
        int (*read32)(struct tpm_tis_data *data, u32 addr, u32 *result);
        int (*write32)(struct tpm_tis_data *data, u32 addr, u32 src);
+       bool (*check_data)(struct tpm_tis_data *data, const u8 *buf,
+                          size_t len);
 };

 static inline int tpm_tis_read_bytes(struct tpm_tis_data *data, u32 addr,