Re: [tpmdd-devel] [PATCH] tpm: fix multiple race conditions in tpm_ppi.c

[Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>]

On Tue, Oct 21, 2014 at 10:55:51AM -0600, Jason Gunthorpe wrote:
> On Tue, Oct 21, 2014 at 11:22:41AM +0300, Jarkko Sakkinen wrote:
> > * Traversal of the ACPI device tree was not done right. It should lookup
> >   PPI only under the ACPI device that it is associated. Otherwise, it could
> >   match to a wrong PPI interface if there are two TPM devices in the device
> >   tree.
> > * Removed global ACPI handle and version string from tpm_ppi.c as this
> >   is racy. Instead they should be associated with the chip.
> > * Moved code just a tiny bit towards two-phase allocation to implement
> >   fix for the PPI race conditions.
> > * Added missing copyright platter in tpm_ppi.c.
> 
> The PPI parts of this look fine to me, and are a really nice cleanup,
> thanks!
> 
> Personally, I'd sequence this commit right after your 'tpm: two-phase
> chip management functions' commit because it makes it much saner (no
> half step toward the new functions). I assume this is a theoretical
> problem? Or do you have a two TPM system?

This has realized in Intel NUCs where there is PTT and dTPM module. Even
when PTT is selected there is still ACPI device for dTPM so three is a
race condition and PPI is unusable. I think that it's not good that code is
not robust enough to deal with this.

Even if you forget the race condition it feels waste to lookup a handle
that is already known.

I'll send non-TPM2 patches in their own patch set because they don't
require to deal with sysfs attributes.

> Jason

/Jarkko