From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754330AbeE1JBz (ORCPT ); Mon, 28 May 2018 05:01:55 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60178 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932102AbeE1JBs (ORCPT ); Mon, 28 May 2018 05:01:48 -0400 Date: Mon, 28 May 2018 02:01:47 -0700 From: Jerry Snitselaar To: Laurent Bigonville Cc: linux-integrity@vger.kernel.org, linux-kernel@vger.kernel.org, Jarkko Sakkinen , Peter Huewe , Jason Gunthorpe Subject: Re: [PATCH] tpm_tis: verify locality released before returning from release_locality Message-ID: <20180528090147.yjuup2jved7geumv@cantor> Reply-To: Jerry Snitselaar Mail-Followup-To: Laurent Bigonville , linux-integrity@vger.kernel.org, linux-kernel@vger.kernel.org, Jarkko Sakkinen , Peter Huewe , Jason Gunthorpe References: <20180505195453.10431-1-jsnitsel@redhat.com> <20180505200315.x7jt33j7psizmfyi@cantor> <06d7794e-125b-85da-72af-c386d999341c@debian.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1; format=flowed Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <06d7794e-125b-85da-72af-c386d999341c@debian.org> User-Agent: NeoMutt/20180512 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon May 28 18, Laurent Bigonville wrote: >Hello, > >Top posting, sorry. > >I don't know if I did it well to include the "Tested-by" tag because I >don't see that the patch has landed in linus branch already. > >And as far as I understand, this will not be in the upcoming 4.17 >release as we are already late in the cycle? > >Kind regards, > >Laurent Bigonville > It should go into his branch during the merge window for 4.18. > >Le 11/05/18 à 21:02, Laurent Bigonville a écrit : >>Le 05/05/18 à 22:03, Jerry Snitselaar a écrit : >>>On Sat May 05 18, Jerry Snitselaar wrote: >>>>For certain tpm chips releasing locality can take long enough that a >>>>subsequent call to request_locality will see the locality as being >>>>active when the access register is read in check_locality. So check >>>>that the locality has been released before returning from >>>>release_locality. >>>> >>>>Cc: Jarkko Sakkinen >>>>Cc: Peter Huewe >>>>Cc: Jason Gunthorpe >>>>Reported-by: Laurent Bigonville >>>>Signed-off-by: Jerry Snitselaar >>Tested-by: Laurent Bigonville >>>>--- >>>>drivers/char/tpm/tpm_tis_core.c | 47 >>>>++++++++++++++++++++++++++++++++++++++++- >>>>1 file changed, 46 insertions(+), 1 deletion(-) >>>> >>>>diff --git a/drivers/char/tpm/tpm_tis_core.c >>>>b/drivers/char/tpm/tpm_tis_core.c >>>>index 5a1f47b43947..d547cd309dbd 100644 >>>>--- a/drivers/char/tpm/tpm_tis_core.c >>>>+++ b/drivers/char/tpm/tpm_tis_core.c >>>>@@ -143,13 +143,58 @@ static bool check_locality(struct tpm_chip >>>>*chip, int l) >>>>    return false; >>>>} >>>> >>>>+static bool locality_inactive(struct tpm_chip *chip, int l) >>>>+{ >>>>+    struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev); >>>>+    int rc; >>>>+    u8 access; >>>>+ >>>>+    rc = tpm_tis_read8(priv, TPM_ACCESS(l), &access); >>>>+    if (rc < 0) >>>>+        return false; >>>>+ >>>>+    if ((access & (TPM_ACCESS_VALID | TPM_ACCESS_ACTIVE_LOCALITY)) >>>>+        == TPM_ACCESS_VALID) >>>>+        return true; >>>>+ >>>>+    return false; >>>>+} >>>>+ >>>>static int release_locality(struct tpm_chip *chip, int l) >>>>{ >>>>    struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev); >>>>+    unsigned long stop, timeout; >>>>+    long rc; >>>> >>>>    tpm_tis_write8(priv, TPM_ACCESS(l), TPM_ACCESS_ACTIVE_LOCALITY); >>>> >>>>-    return 0; >>>>+    stop = jiffies + chip->timeout_a; >>>>+ >>>>+    if (chip->flags & TPM_CHIP_FLAG_IRQ) { >>>>+again: >>>>+        timeout = stop - jiffies; >>>>+        if ((long)timeout <= 0) >>>>+            return -1; >>>>+ >>>>+        rc = wait_event_interruptible_timeout(priv->int_queue, >>>>+                              (locality_inactive(chip, l)), >>>>+                              timeout); >>>>+ >>>>+        if (rc > 0) >>>>+            return 0; >>>>+ >>>>+        if (rc == -ERESTARTSYS && freezing(current)) { >>>>+            clear_thread_flag(TIF_SIGPENDING); >>>>+            goto again; >>>>+        } >>>>+    } else { >>>>+        do { >>>>+            if (locality_inactive(chip, l)) >>>>+                return 0; >>>>+            tpm_msleep(TPM_TIMEOUT); >>>>+        } while (time_before(jiffies, stop)); >>>>+    } >>>>+    return -1; >>>>} >>>> >>>>static int request_locality(struct tpm_chip *chip, int l) >>>>-- >>>>2.15.0 >>>> >>> >>>Laurent, >>> >>>Can you try this patch with your system since it is the one >>>that has exhibited the problem so far. I've tested on a >>>tpm2.0 and tpm1.2 system here. >>> >>>Regards, >>>Jerry >> >