Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager via a device link /dev/tpms<n>

[Jason Gunthorpe <jgunthorpe@obsidianresearch.com>]

On Fri, Jan 13, 2017 at 05:10:30PM -0800, James Bottomley wrote:

> > No, it is correct as is. The cdev fops rely only on the tpm module.
> > When tpm_chip_unregister returns to the driver the chips->ops is set
> > to NULL with proper locking - the driver code becomes uncallable at
> > that point.
>
> So that's the nastily subtle point I was missing.  The patch below will
> add the reference tracking to make sure the chip is held while the
> /dev/tpms<n> is open.  I really hate the additional layer of subtlety
> this adds, though ... I've tried to document the extra nasties, but I
> bet they confuse someone.

Thanks

> Doing it the standard way, where the owner is set to the pdev driver
> module and the references all track up to the pdev, meaning the actual
> device could never go away while a cdev file was open would be far
> easier to understand.

For a long time now devices can be unbound without a module unload. Eg the
'unbind' file in sysfs. Subsystems can no longer rely on module locking to
block unbind.

vtpm makes use of hot-unplug when the server side is closed. We
audited this area extensively when vtpm was introduced to make it work
properly.

Not all subsystems are correct in this regard, and you may still find
vestigial assumptions that module locking is enough to fix
locking/refcounting sins. It is not, those places are just broken. :(

Jason