Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1557686 > unrolled thread

[PATCH] tpm_tis: fix iTPM probe via probe_itpm() function

Back to article view | Back to linux.kernel

Contents

  [PATCH] tpm_tis: fix iTPM probe via probe_itpm() function "Maciej S. Szmigiero" <mail@maciej.szmigiero.name> - 2017-01-12 19:20 +0100
    Re: [PATCH] tpm_tis: fix iTPM probe via probe_itpm() function Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-12 20:00 +0100
      Re: [PATCH] tpm_tis: fix iTPM probe via probe_itpm() function "Maciej S. Szmigiero" <mail@maciej.szmigiero.name> - 2017-01-12 21:20 +0100
    Re: [PATCH] tpm_tis: fix iTPM probe via probe_itpm() function Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-12 21:30 +0100

#1557686 — [PATCH] tpm_tis: fix iTPM probe via probe_itpm() function

probe_itpm() function is supposed to send command without an itpm flag set
and if this fails to repeat it, this time with the itpm flag set.

However, commit 41a5e1cf1fe15 ("tpm/tpm_tis: Split tpm_tis driver into a
core and TCG TIS compliant phy") moved the itpm flag from an "itpm"
variable to a TPM_TIS_ITPM_POSSIBLE chip flag, so setting the
(now function-local) itpm variable no longer had any effect.

Finally, this function-local itpm variable was removed by
commit 56af322156dbe9 ("tpm/tpm_tis: remove unused itpm variable")

Tested only on non-iTPM TIS TPM.

Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>

Fixes: 41a5e1cf1fe15 ("Split tpm_tis driver into a core and TCG TIS compliant phy")
Cc: stable@vger.kernel.org
---
 drivers/char/tpm/tpm_tis_core.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index e42e5a6a3c2f..401f1228547c 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -467,6 +467,9 @@ static int probe_itpm(struct tpm_chip *chip)
 	size_t len = sizeof(cmd_getticks);
 	u16 vendor;
 
+	if (priv->flags & TPM_TIS_ITPM_POSSIBLE)
+		return 0;
+
 	rc = tpm_tis_read16(priv, TPM_DID_VID(0), &vendor);
 	if (rc < 0)
 		return rc;
@@ -482,12 +485,15 @@ static int probe_itpm(struct tpm_chip *chip)
 	tpm_tis_ready(chip);
 	release_locality(chip, priv->locality, 0);
 
+	priv->flags |= TPM_TIS_ITPM_POSSIBLE;
+
 	rc = tpm_tis_send_data(chip, cmd_getticks, len);
-	if (rc == 0) {
+	if (rc == 0)
 		dev_info(&chip->dev, "Detected an iTPM.\n");
-		rc = 1;
-	} else
+	else {
+		priv->flags &= ~TPM_TIS_ITPM_POSSIBLE;
 		rc = -EFAULT;
+	}
 
 out:
 	tpm_tis_ready(chip);
@@ -743,15 +749,10 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
 		 (chip->flags & TPM_CHIP_FLAG_TPM2) ? "2.0" : "1.2",
 		 vendor >> 16, rid);
 
-	if (!(priv->flags & TPM_TIS_ITPM_POSSIBLE)) {
-		probe = probe_itpm(chip);
-		if (probe < 0) {
-			rc = -ENODEV;
-			goto out_err;
-		}
-
-		if (!!probe)
-			priv->flags |= TPM_TIS_ITPM_POSSIBLE;
+	probe = probe_itpm(chip);
+	if (probe < 0) {
+		rc = -ENODEV;
+		goto out_err;
 	}
 
 	/* Figure out the capabilities */

[toc] | [next] | [standalone]

#1557730

On Thu, Jan 12, 2017 at 07:12:06PM +0100, Maciej S. Szmigiero wrote:
> probe_itpm() function is supposed to send command without an itpm flag set
> and if this fails to repeat it, this time with the itpm flag set.
> 
> However, commit 41a5e1cf1fe15 ("tpm/tpm_tis: Split tpm_tis driver into a
> core and TCG TIS compliant phy") moved the itpm flag from an "itpm"
> variable to a TPM_TIS_ITPM_POSSIBLE chip flag, so setting the
> (now function-local) itpm variable no longer had any effect.
> 
> Finally, this function-local itpm variable was removed by
> commit 56af322156dbe9 ("tpm/tpm_tis: remove unused itpm variable")
> 
> Tested only on non-iTPM TIS TPM.
> 
> Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>

could you also change the name of TPM_TIS_ITPM_POSSIBLE
to TPM_TIS_ITPM_WORKAROUND ?

Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>

Jason

[toc] | [prev] | [next] | [standalone]

#1557772

On 12.01.2017 19:50, Jason Gunthorpe wrote:
> On Thu, Jan 12, 2017 at 07:12:06PM +0100, Maciej S. Szmigiero wrote:
>> probe_itpm() function is supposed to send command without an itpm flag set
>> and if this fails to repeat it, this time with the itpm flag set.
>>
>> However, commit 41a5e1cf1fe15 ("tpm/tpm_tis: Split tpm_tis driver into a
>> core and TCG TIS compliant phy") moved the itpm flag from an "itpm"
>> variable to a TPM_TIS_ITPM_POSSIBLE chip flag, so setting the
>> (now function-local) itpm variable no longer had any effect.
>>
>> Finally, this function-local itpm variable was removed by
>> commit 56af322156dbe9 ("tpm/tpm_tis: remove unused itpm variable")
>>
>> Tested only on non-iTPM TIS TPM.
>>
>> Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
> 
> could you also change the name of TPM_TIS_ITPM_POSSIBLE
> to TPM_TIS_ITPM_WORKAROUND ?

I will prepare such patch.

> Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Jason

Maciej

[toc] | [prev] | [next] | [standalone]

#1557779

On Thu, Jan 12, 2017 at 07:12:06PM +0100, Maciej S. Szmigiero wrote:
> probe_itpm() function is supposed to send command without an itpm flag set
> and if this fails to repeat it, this time with the itpm flag set.
> 
> However, commit 41a5e1cf1fe15 ("tpm/tpm_tis: Split tpm_tis driver into a
> core and TCG TIS compliant phy") moved the itpm flag from an "itpm"
> variable to a TPM_TIS_ITPM_POSSIBLE chip flag, so setting the
> (now function-local) itpm variable no longer had any effect.
> 
> Finally, this function-local itpm variable was removed by
> commit 56af322156dbe9 ("tpm/tpm_tis: remove unused itpm variable")
> 
> Tested only on non-iTPM TIS TPM.
> 
> Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
> 
> Fixes: 41a5e1cf1fe15 ("Split tpm_tis driver into a core and TCG TIS compliant phy")
> Cc: stable@vger.kernel.org

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

/Jarkko

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web