Re: [PATCH v7 08/10] tpm: Proxy driver for supporting multiple emulated TPMs

Jarkko Sakkinen [off-list ref] wrote on 03/13/2016 
04:06:52 PM:

On Sat, Mar 12, 2016 at 06:27:13PM -0500, Stefan Berger wrote:

quoted

   This fix should solve the problem:

   diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c
   b/drivers/char/tpm/tpm_vtpm_proxy.c
   index d73944e..01e5070 100644
   --- a/drivers/char/tpm/tpm_vtpm_proxy.c
   +++ b/drivers/char/tpm/tpm_vtpm_proxy.c
   @@ -456,10 +456,10 @@ err_delete_proxy_dev:
     */
    static void vtpm_proxy_delete_device(struct proxy_dev *proxy_dev)
    {
   -       tpm_chip_unregister(proxy_dev->chip);
   -
           vtpm_proxy_fops_undo_open(proxy_dev);

   +      tpm_chip_unregister(proxy_dev->chip);
   +
           vtpm_proxy_delete_proxy_dev(proxy_dev);
    }

   Can you let me know whether this gets it working for you? I'd 

prepare a

quoted

   v9.

So is the deadlock such that:

* tpm_chip_unregister() tries to write lock ops_sem.
* tpm_transmit() holds read lock to ops_sem.

This takes two minutes if no timeouts are calculated.

And is the effect of moving vtpm_proxy_fops_undo_open() upwards such
that vtpm_proxy_tpm_req_canceled() starts returning true, which in
effect breaks the loop in tpm_transmit()?

Yeah, the fix as a code change is very simple but I had to use perf
probe to verify this so maybe a comment there would be in place to
tell why tpm_chip_unregister() must be called last (write lock).

I'll put a comment. The reason is that the client (user of /dev/tpmX) is 
possibly holding the 'ops' lock while we the proxy driver needs it when 
calling tom_chip_unregister.
Thanks for trying it.

   Stefan