| Message ID | 20180505195453.10431-1-jsnitsel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <jarkko.sakkinen@linux.intel.com> >Cc: Peter Huewe <peterhuewe@gmx.de> >Cc: Jason Gunthorpe <jgg@ziepe.ca> >Reported-by: Laurent Bigonville <bigon@debian.org> >Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com> >--- > 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
Le 05/05/18 à 22:03, Jerry Snitselaar a écrit : > On Sat May 05 18, Jerry Snitselaar wrote: >> [...] >> > > Laurent, Hello Jerry, > 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. I just tested the patch and the driver is loading fine again and the device in /dev is present again, so it seems to work. But for some reason the tpm is again locked (for no visible reason) due to "dictionary attack" so I cannot test further ATM :/ Regards, Laurent > > Regards, > Jerry
On Thu, May 10, 2018 at 01:21:39PM +0200, Laurent Bigonville wrote: > Le 05/05/18 à 22:03, Jerry Snitselaar a écrit : > > On Sat May 05 18, Jerry Snitselaar wrote: > > > [...] > > > > > > > Laurent, > > Hello Jerry, > > > 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. > > I just tested the patch and the driver is loading fine again and the device > in /dev is present again, so it seems to work. > > But for some reason the tpm is again locked (for no visible reason) due to > "dictionary attack" so I cannot test further ATM :/ > > Regards, > > Laurent > > > > > Regards, > > Jerry > Can you response with the tested-by tag as soon as you can? Before we have tested-by, I cannot land the fix. /Jarkko
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 <jarkko.sakkinen@linux.intel.com> >> Cc: Peter Huewe <peterhuewe@gmx.de> >> Cc: Jason Gunthorpe <jgg@ziepe.ca> >> Reported-by: Laurent Bigonville <bigon@debian.org> >> Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com> Tested-by: Laurent Bigonville <bigon@debian.org> >> --- >> 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
On Sat, May 05, 2018 at 12:54:53PM -0700, 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 <jarkko.sakkinen@linux.intel.com> > Cc: Peter Huewe <peterhuewe@gmx.de> > Cc: Jason Gunthorpe <jgg@ziepe.ca> > Reported-by: Laurent Bigonville <bigon@debian.org> > Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> /Jarkko
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 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 <jarkko.sakkinen@linux.intel.com> >>> Cc: Peter Huewe <peterhuewe@gmx.de> >>> Cc: Jason Gunthorpe <jgg@ziepe.ca> >>> Reported-by: Laurent Bigonville <bigon@debian.org> >>> Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com> > Tested-by: Laurent Bigonville <bigon@debian.org> >>> --- >>> 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 >
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 <jarkko.sakkinen@linux.intel.com> >>>>Cc: Peter Huewe <peterhuewe@gmx.de> >>>>Cc: Jason Gunthorpe <jgg@ziepe.ca> >>>>Reported-by: Laurent Bigonville <bigon@debian.org> >>>>Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com> >>Tested-by: Laurent Bigonville <bigon@debian.org> >>>>--- >>>>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 >> >
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)
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 <jarkko.sakkinen@linux.intel.com> Cc: Peter Huewe <peterhuewe@gmx.de> Cc: Jason Gunthorpe <jgg@ziepe.ca> Reported-by: Laurent Bigonville <bigon@debian.org> Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com> --- drivers/char/tpm/tpm_tis_core.c | 47 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-)