On Mon, 2017-06-19 at 01:21 +0200, Jarkko Sakkinen wrote:
> Hold on.
>
> On Fri, 2017-06-16 at 10:25 -0700, Josh Zimmerman wrote:
> > If a TPM2 loses power without a TPM2_Shutdown command being issued
> > (a
> > "disorderly reboot"), it may lose some state that has yet to be
> > persisted to NVRam, and will increment the DA counter. After the DA
> > counter gets sufficiently large, the TPM will lock the user out.
> >
> > NOTE: This only changes behavior on TPM2 devices. Since TPM1 uses
>
> sysfs,
> > and sysfs relies on implicit locking on chip->ops, it is not safe
> > to
> > allow this code to run in TPM1, or to add sysfs support to TPM2,
>
> until
> > that locking is made explicit.
> >
> > Signed-off-by: Josh Zimmerman <jo...@google.com>
> > Cc: sta...@vger.kernel.org
> >
> > ----
> > v2:
> > - Properly split changes between this and another commit
> > - Use proper locking primitive.
> > - Fix commenting style
> > v3:
> > - Re-fix commenting style
> > v4:
> > - Update description and tags (Reviewed-by, Cc).
> > v5:
> > - Update documentation.
> > v6:
> > - Call device and/or bus shutdown from tpm_shutdown.
> > ---
> > drivers/char/tpm/tpm-chip.c | 31 +++++++++++++++++++++++++++++++
> > drivers/char/tpm/tpm-sysfs.c | 3 +++
> > 2 files changed, 34 insertions(+)
> >
> > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-
>
> chip.c
> > index 9dec9f551b83..62691d266f22 100644
> > --- a/drivers/char/tpm/tpm-chip.c
> > +++ b/drivers/char/tpm/tpm-chip.c
> > @@ -142,6 +142,36 @@ static void tpm_devs_release(struct device
> > *dev)
> > put_device(&chip->dev);
> > }
> >
> > +/**
> > + * tpm_shutdown() - prepare the TPM device for loss of power.
> > + * @dev: device to which the chip is associated.
> > + *
> > + * Issues a TPM2_Shutdown command prior to loss of power, as
>
> required by the
> > + * TPM 2.0 spec.
> > + *
> > + * XXX: This codepath relies on the fact that sysfs is not enabled
>
> for
> > + * TPM2: sysfs uses an implicit lock on chip->ops, so this could
>
> race if TPM2
> > + * has sysfs support enabled before TPM sysfs's implicit locking
> > is
>
> fixed.
> > + */
> > +static void tpm_shutdown(struct device *dev)
> > +{
> > + struct tpm_chip *chip = container_of(dev, struct tpm_chip,
>
> dev);
> > +
> > + if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> > + down_write(&chip->ops_sem);
> > + tpm2_shutdown(chip, TPM_SU_CLEAR);
> > + chip->ops = NULL;
> > + up_write(&chip->ops_sem);
> > + }
> > + // Allow bus- and device-specific code to run. Note: since
>
> chip->ops
> > + // is NULL, more-specific shutdown code will not be able
> > to
>
> issue TPM
> > + // commands.
> > + if (dev->bus->shutdown)
> > + dev->bus->shutdown(dev);
> > + else if (dev->driver && dev->driver->shutdown)
> > + dev->driver->shutdown(dev);
>
> WTF is this part.
>
> And why we have now duplicate code??
>
> I'm dropping these patches for now from my master branch. I don't
> understand what is going on...
>
> /Jarkko
This series is too broken to be merged :-( I expect cover letter for
the next version...
Feels weird that you have to call framework functions like that in the
driver. You must have brilliant reason to do so and that should be very
well documented in the code. This is terrible...
/Jarkko
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
