On Wed, Sep 30, 2020 at 04:37:58AM +0300, Jarkko Sakkinen wrote: > On Mon, Sep 28, 2020 at 02:15:33PM -0700, James Bottomley wrote: > > On Mon, 2020-09-28 at 15:33 -0300, Jason Gunthorpe wrote: > > > On Mon, Sep 28, 2020 at 11:00:12AM -0700, James Bottomley wrote: > > > > Some TIS based TPMs can return 0xff to status reads if the locality > > > > hasn't been properly requested. Detect this condition by checking > > > > the bits that the TIS spec specifies must return zero are clear and > > > > return zero in that case. Also drop a warning so the problem can > > > > be identified in the calling path and fixed (usually a missing > > > > try_get_ops()). > > > > > > > > Signed-off-by: James Bottomley < > > > > James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> > > > > > > > > This is the patch I've been using to catch and kill all the points > > > > in the stack where we're not properly using get/put ops on the tpm > > > > chip. > > > > > > If this is a problem add a lockdep on ops_sem in various places too? > > > > It's not really possible because of the init issue with checking the > > interrupt. That originally had no locking at all (it doesn't need any > > because the TPM device isn't publicly exposed at the point the check is > > done). If the patch to add get/put around the tpm2_get_tpm_pt is > > acceptable, then perhaps we could because I think that's the last > > unguarded use of tpm_tis_status. > > > > James > > I think this sanity check would not hurt at all. > > We should also improve the testing coverage. E.g. there is zero coverage > for trusted keys. It can easily lead weird conclusions if there is no > common test to run. > > I touched that over here: > > https://lore.kernel.org/linux-integrity/20200929225841.GA805025@xxxxxxxxxxxxxxx/ > > Anyway, I will apply this for sure. > > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxxxxxxxx> Has been applied. /Jarkko
