On Fri, Apr 28, 2017 at 03:07:02PM -0700, Josh Zimmerman wrote: > On Fri, Apr 28, 2017 at 2:39 PM, Jason Gunthorpe > <[email protected]> wrote: > > On Fri, Apr 28, 2017 at 02:32:31PM -0700, Josh Zimmerman wrote: > >> On Fri, Apr 28, 2017 at 11:43 AM, Jason Gunthorpe > >> <[email protected]> wrote: > >> > On Fri, Apr 28, 2017 at 11:27:16AM -0700, Josh Zimmerman wrote: > >> >> It sounds like there are several intertwined issues here. I'd like to > >> >> keep this patch relatively minimal, so here are my current thoughts on > >> >> a path forward: > >> >> > >> >> * Refactor references to chip->ops to go through tpm_try_get_ops, as > >> >> mentioned in the previous patch > >> > > >> > As I said, if you rely on the fact that sysfs does not exist for TPM2 > >> > then this should already be done for the TPM2 case and an incremental > >> > later patch could solve this problem in sysfs to turn shutdown on for > >> > tpm1. As long as this logic is clearly documented it is probably an OK > >> > step for now. > >> > >> This I'm not actually sure about: it seems that there are other places > >> than sysfs where chip->ops is referenced without being guarded by > >> tpm_try_get_ops. If we rely on just NULLing out chip->ops this could > >> be dangerous. I'll look at sending a patch for this first. > > > > If that is the case then we have an existing bug.. > > > > The only other user entry point is via the cdev, and those call > > try_get_ops around all calls down into the tpm code. > > > > Everything coming in via the kapi must call tpm_chip_find_get, which > > does try_get_ops. > > OK, then I'm confused about the problem with sysfs. If everything > is already grabbing a ref first, why can't we just grab a ref and > NULL out chip->ops?
sysfs relies on an implicit ref that is held across the duration of the sysfs file's lifetime. That ref is not given up until device_del returns, which serializes everything. See the comment in tpm_sysfs_add_device This is fine so long as ops is only ever null'd after device_del, but this shutdown work proposes to change that invariant.. To compensate, the ops lock would have to be pushed down into each sysfs method (for tpm1), or just not use sysfs (like tpm2 does) Jason ------------------------------------------------------------------------------ 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 [email protected] https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
