On Sun, Nov 18, 2018 at 12:51:04PM -0700, Jason Gunthorpe wrote: > On Sun, Nov 18, 2018 at 02:47:47PM +0200, Jarkko Sakkinen wrote: > > Added locking as part of tpm_try_get_ops() and tpm_put_ops() as they are > > anyway used in most of the call sites except in tpmrm_release() where we > > take the locks manually. > > > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxxxxxxxx> > > Reviewed-by: Stefan Berger <stefanb@xxxxxxxxxxxxx> > > --- > > drivers/char/tpm/tpm-chip.c | 2 ++ > > drivers/char/tpm/tpm-dev-common.c | 4 +--- > > drivers/char/tpm/tpm-interface.c | 8 -------- > > drivers/char/tpm/tpm.h | 8 ++------ > > drivers/char/tpm/tpm2-cmd.c | 13 ++++--------- > > drivers/char/tpm/tpm2-space.c | 15 ++++++--------- > > 6 files changed, 15 insertions(+), 35 deletions(-) > > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c > > index 32db84683c40..157505b0f755 100644 > > --- a/drivers/char/tpm/tpm-chip.c > > +++ b/drivers/char/tpm/tpm-chip.c > > @@ -58,6 +58,7 @@ int tpm_try_get_ops(struct tpm_chip *chip) > > if (!chip->ops) > > goto out_lock; > > > > + mutex_lock(&chip->tpm_mutex); > > return 0; > > It really isn't appropriate for something called 'get' to be exclusive > like this.. Call it tpm_try_lock_ops() ? Would definitely be appropriate to rename it, yes. > > diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c > > index c7dc54930576..582caefcf19b 100644 > > --- a/drivers/char/tpm/tpm-dev-common.c > > +++ b/drivers/char/tpm/tpm-dev-common.c > > @@ -33,7 +33,6 @@ static ssize_t tpm_dev_transmit(struct tpm_chip *chip, struct tpm_space *space, > > struct tpm_header *header = (void *)buf; > > ssize_t ret, len; > > > > - mutex_lock(&chip->tpm_mutex); > > The now implicit locking should be tested for using lockdep calls in > all places that assume the lock is held by the caller. Yes. Most importantly trusted keys and IMA should be tested with lockdep turned on. /Jarkko
