Re: [PATCH] tpm/tpm_i2c_infineon: ensure no ongoing commands on shutdown

From: Jarkko Sakkinen
Date: Mon Jan 16 2017 - 04:33:38 EST


On Fri, Jan 13, 2017 at 04:42:30PM -0800, Andrey Pronin wrote:
> On Fri, Jan 13, 2017 at 05:28:57PM -0700, Jason Gunthorpe wrote:
> > On Fri, Jan 13, 2017 at 04:09:54PM -0800, Andrey Pronin wrote:
> > > Resetting TPM while processing a command may lead to issues
> > > on the next boot. Ensure that we don't have any ongoing
> > > commands, and that no further commands can be sent to the chip
> > > by unregistering the device in the shutdown handler.
> > > tpm_chip_unregister() waits for the completion of an ongoing
> > > command, if any, and then clears out chip->ops and unregisters
> > > sysfs entities.
> >
> > Unregistering in a shutdown handler seems very strange, it also waits
> > for userspace things, so I wonder if it could be problematic?
> >
> > Maybe just use
> >
> > down_write(&chip->ops_sem);
> > chip->ops = NULL;
> > up_write(&chip->ops_sem);
> >
> > In the shutdown handler?
>
> down_write(&chip->ops_sem) would still wait for completing the initiated
> writes, since tpm_write() in tpm-dev.c calls tpm_try_get_ops().
> Also, tpm-sysfs.c calls chip->ops directly, so sysfs should be
> unregistered first.

Why don't you fix the tpm-sysfs issue but rather misusing
tpm_chip_unregister?

/Jarkko