All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
To: Stefan Berger <stefanb-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
Subject: Re: [PATCH v7 08/10] tpm: Proxy driver for supporting multiple emulated TPMs
Date: Sun, 13 Mar 2016 22:06:52 +0200	[thread overview]
Message-ID: <20160313200652.GA3087@intel.com> (raw)
In-Reply-To: <201603122327.u2CNRI56031122-YREtIfBy6dDImUpY6SP3GEEOCMrvLtNR@public.gmane.org>

On Sat, Mar 12, 2016 at 06:27:13PM -0500, Stefan Berger wrote:
>    Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote on 03/12/2016
>    01:51:54 PM:
> 
>    >
>    > On Fri, Mar 11, 2016 at 09:51:03PM -0500, Stefan Berger wrote:
>    > > This patch implements a proxy driver for supporting multiple emulated
>    TPMs
>    > > in a system.
>    > >
>    > > The driver implements a device /dev/vtpmx that is used to created
>    > > a client device pair /dev/tpmX (e.g., /dev/tpm10) and a server side
>    that
>    > > is accessed using a file descriptor returned by an ioctl.
>    > > The device /dev/tpmX is the usual TPM device created by the core TPM
>    > > driver. Applications or kernel subsystems can send TPM commands to it
>    > > and the corresponding server-side file descriptor receives these
>    > > commands and delivers them to an emulated TPM.
>    >
>    > With my test script [1] running on QEMU and TPM 2.0 simulator running on
>    the
>    > host side I get this:
>    >
>    > $ python tpm2-simulator-vtpm --host=10.0.2.2
>    > cmd
>    > 80 01 00 00 00 0c 00 00 01 44 00 00
>    > rsp
>    > 80 01 00 00 00 0a 00 00 00 00
>    > Traceback (most recent call last):
>    >   File "tpm2-simulator-vtpm", line 85, in <module>
>    >     main()
>    >   File "tpm2-simulator-vtpm", line 80, in main
>    >     resp = client.send_cmd(stream)
>    >   File "/home/tpmdd/tpm2-scripts/tpm2.py", line 454, in send_cmd
>    >     rsp = self.simulator.send_cmd(cmd)
>    >   File "/home/tpmdd/tpm2-scripts/tpm2.py", line 436, in send_cmd
>    >     raise SimulatorError("Empty response")
>    > tpm2.SimulatorError: Empty response
>    >
>    > However, the process does not exit before the read call expires:
>    >
>    > $ python tpm2-list-handles
>    > Traceback (most recent call last):
>    >   File "tpm2-list-handles", line 61, in <module>
>    >     main()
>    >   File "tpm2-list-handles", line 51, in main
>    >     handles += client.get_cap(tpm2.TPM2_CAP_HANDLES,
>    tpm2.HR_LOADED_SESSION)
>    >   File "/home/tpmdd/tpm2-scripts/tpm2.py", line 782, in get_cap
>    >     next_handles, more_data = self.__get_cap_cnt(cap, pt, 1)
>    >   File "/home/tpmdd/tpm2-scripts/tpm2.py", line 766, in __get_cap_cnt
>    >     rsp = self.send_cmd(cmd)[10:]
>    >   File "/home/tpmdd/tpm2-scripts/tpm2.py", line 458, in send_cmd
>    >     rsp = f.read()
>    > IOError: [Errno 62] Timer expired
>    >
>    > The server side stays stuck unti this happens.
> 
>    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
>    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).

>       Stefan

/Jarkko

------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785111&iu=/4140

WARNING: multiple messages have this Message-ID (diff)
From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
To: Stefan Berger <stefanb@us.ibm.com>
Cc: linux-api@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	Stefan Berger <stefanb@linux.vnet.ibm.com>,
	tpmdd-devel@lists.sourceforge.net
Subject: Re: [tpmdd-devel] [PATCH v7 08/10] tpm: Proxy driver for supporting multiple emulated TPMs
Date: Sun, 13 Mar 2016 22:06:52 +0200	[thread overview]
Message-ID: <20160313200652.GA3087@intel.com> (raw)
In-Reply-To: <201603122327.u2CNRI56031122@d01av04.pok.ibm.com>

On Sat, Mar 12, 2016 at 06:27:13PM -0500, Stefan Berger wrote:
>    Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote on 03/12/2016
>    01:51:54 PM:
> 
>    >
>    > On Fri, Mar 11, 2016 at 09:51:03PM -0500, Stefan Berger wrote:
>    > > This patch implements a proxy driver for supporting multiple emulated
>    TPMs
>    > > in a system.
>    > >
>    > > The driver implements a device /dev/vtpmx that is used to created
>    > > a client device pair /dev/tpmX (e.g., /dev/tpm10) and a server side
>    that
>    > > is accessed using a file descriptor returned by an ioctl.
>    > > The device /dev/tpmX is the usual TPM device created by the core TPM
>    > > driver. Applications or kernel subsystems can send TPM commands to it
>    > > and the corresponding server-side file descriptor receives these
>    > > commands and delivers them to an emulated TPM.
>    >
>    > With my test script [1] running on QEMU and TPM 2.0 simulator running on
>    the
>    > host side I get this:
>    >
>    > $ python tpm2-simulator-vtpm --host=10.0.2.2
>    > cmd
>    > 80 01 00 00 00 0c 00 00 01 44 00 00
>    > rsp
>    > 80 01 00 00 00 0a 00 00 00 00
>    > Traceback (most recent call last):
>    >   File "tpm2-simulator-vtpm", line 85, in <module>
>    >     main()
>    >   File "tpm2-simulator-vtpm", line 80, in main
>    >     resp = client.send_cmd(stream)
>    >   File "/home/tpmdd/tpm2-scripts/tpm2.py", line 454, in send_cmd
>    >     rsp = self.simulator.send_cmd(cmd)
>    >   File "/home/tpmdd/tpm2-scripts/tpm2.py", line 436, in send_cmd
>    >     raise SimulatorError("Empty response")
>    > tpm2.SimulatorError: Empty response
>    >
>    > However, the process does not exit before the read call expires:
>    >
>    > $ python tpm2-list-handles
>    > Traceback (most recent call last):
>    >   File "tpm2-list-handles", line 61, in <module>
>    >     main()
>    >   File "tpm2-list-handles", line 51, in main
>    >     handles += client.get_cap(tpm2.TPM2_CAP_HANDLES,
>    tpm2.HR_LOADED_SESSION)
>    >   File "/home/tpmdd/tpm2-scripts/tpm2.py", line 782, in get_cap
>    >     next_handles, more_data = self.__get_cap_cnt(cap, pt, 1)
>    >   File "/home/tpmdd/tpm2-scripts/tpm2.py", line 766, in __get_cap_cnt
>    >     rsp = self.send_cmd(cmd)[10:]
>    >   File "/home/tpmdd/tpm2-scripts/tpm2.py", line 458, in send_cmd
>    >     rsp = f.read()
>    > IOError: [Errno 62] Timer expired
>    >
>    > The server side stays stuck unti this happens.
> 
>    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
>    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).

>       Stefan

/Jarkko

  parent reply	other threads:[~2016-03-13 20:06 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-12  2:50 [PATCH v7 00/10] Multi-instance vTPM proxy driver Stefan Berger
2016-03-12  2:50 ` Stefan Berger
     [not found] ` <1457751065-11507-1-git-send-email-stefanb-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
2016-03-12  2:50   ` [PATCH v7 01/10] tpm: Get rid of chip->pdev Stefan Berger
2016-03-12  2:50     ` Stefan Berger
2016-03-12  2:50   ` [PATCH v7 03/10] tpm: Provide strong locking for device removal Stefan Berger
2016-03-12  2:50     ` Stefan Berger
2016-03-12  2:50   ` [PATCH v7 04/10] tpm: Get rid of module locking Stefan Berger
2016-03-12  2:50     ` Stefan Berger
2016-03-12  2:51   ` [PATCH v7 05/10] tpm: Split out the devm stuff from tpmm_chip_alloc Stefan Berger
2016-03-12  2:51     ` Stefan Berger
2016-03-12  2:51   ` [PATCH v7 06/10] tpm: Replace device number bitmap with IDR Stefan Berger
2016-03-12  2:51     ` Stefan Berger
2016-03-12  2:51   ` [PATCH v7 07/10] tpm: Introduce TPM_CHIP_FLAG_VIRTUAL Stefan Berger
2016-03-12  2:51     ` Stefan Berger
2016-03-12  2:51   ` [PATCH v7 08/10] tpm: Proxy driver for supporting multiple emulated TPMs Stefan Berger
2016-03-12  2:51     ` Stefan Berger
     [not found]     ` <1457751065-11507-9-git-send-email-stefanb-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
2016-03-12 18:51       ` Jarkko Sakkinen
2016-03-12 18:51         ` Jarkko Sakkinen
     [not found]         ` <20160312185154.GA12412-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2016-03-12 23:27           ` Stefan Berger
2016-03-13  1:52         ` Stefan Berger
     [not found]         ` <201603122327.u2CNRI56031122@d01av04.pok.ibm.com>
     [not found]           ` <201603122327.u2CNRI56031122-YREtIfBy6dDImUpY6SP3GEEOCMrvLtNR@public.gmane.org>
2016-03-13 20:06             ` Jarkko Sakkinen [this message]
2016-03-13 20:06               ` [tpmdd-devel] " Jarkko Sakkinen
     [not found]               ` <20160313200652.GA3087-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2016-03-13 22:20                 ` Stefan Berger
2016-03-12  2:51   ` [PATCH v7 09/10] tpm: Initialize TPM and get durations and timeouts Stefan Berger
2016-03-12  2:51     ` Stefan Berger
2016-03-12  2:51   ` [PATCH v7 10/10] tpm: Add documentation for the tpm_vtpm device driver Stefan Berger
2016-03-12  2:51     ` Stefan Berger
2016-03-12  2:50 ` [PATCH v7 02/10] tpm: Get rid of devname Stefan Berger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160313200652.GA3087@intel.com \
    --to=jarkko.sakkinen-vuqaysv1563yd54fqh9/ca@public.gmane.org \
    --cc=linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-security-module-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=stefanb-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org \
    --cc=tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.