tpmdd-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed
* [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown
@ 2017-05-25 13:12 Stefan Berger
       [not found] ` <1495717956-14252-1-git-send-email-stefanb-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Berger @ 2017-05-25 13:12 UTC (permalink / raw)
  To: jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
  Cc: linux-security-module-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

The tpm2_shutdown does not work with the VTPM proxy driver since the
function only gets called when the backend file descriptor is already
closed and at this point no data can be sent anymore. A proper shutdown
would have to be initated by a user space application, such as a container
management stack, that sends the command via the character device before
terminating the TPM emulator.

To avoid the tpm2_shutdown we introduce a TPM_CHIP_FLAG_NO_SHUTDOWN flag
that only the VTPM proxy driver sets. This also avoids misleading kernel
log messages.

Signed-off-by: Stefan Berger <stefanb-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 drivers/char/tpm/tpm.h            | 1 +
 drivers/char/tpm/tpm2-cmd.c       | 3 +++
 drivers/char/tpm/tpm_vtpm_proxy.c | 3 ++-
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 25d9858..23b656f 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -170,6 +170,7 @@ enum tpm_chip_flags {
 	TPM_CHIP_FLAG_IRQ		= BIT(2),
 	TPM_CHIP_FLAG_VIRTUAL		= BIT(3),
 	TPM_CHIP_FLAG_HAVE_TIMEOUTS	= BIT(4),
+	TPM_CHIP_FLAG_NO_SHUTDOWN	= BIT(5),
 };
 
 struct tpm_bios_log {
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 3ee6883..495d316 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -831,6 +831,9 @@ void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
 	struct tpm2_cmd cmd;
 	int rc;
 
+	if (chip->flags & TPM_CHIP_FLAG_NO_SHUTDOWN)
+		return;
+
 	cmd.header.in = tpm2_shutdown_header;
 	cmd.params.startup_in.startup_type = cpu_to_be16(shutdown_type);
 
diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index 1d877cc..d439ce7 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -573,7 +573,8 @@ static struct file *vtpm_proxy_create_device(
 	vtpm_proxy_fops_open(file);
 
 	if (proxy_dev->flags & VTPM_PROXY_FLAG_TPM2)
-		proxy_dev->chip->flags |= TPM_CHIP_FLAG_TPM2;
+		proxy_dev->chip->flags |= TPM_CHIP_FLAG_TPM2 |
+					  TPM_CHIP_FLAG_NO_SHUTDOWN;
 
 	vtpm_proxy_work_start(proxy_dev);
 
-- 
2.4.3


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown
       [not found] ` <1495717956-14252-1-git-send-email-stefanb-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
@ 2017-05-25 15:50   ` Jason Gunthorpe
  2017-05-25 20:04     ` Stefan Berger
  0 siblings, 1 reply; 10+ messages in thread
From: Jason Gunthorpe @ 2017-05-25 15:50 UTC (permalink / raw)
  To: Stefan Berger
  Cc: linux-security-module-u79uwXL29TY76Z2rM5mHXA,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On Thu, May 25, 2017 at 09:12:36AM -0400, Stefan Berger wrote:
> The tpm2_shutdown does not work with the VTPM proxy driver since the
> function only gets called when the backend file descriptor is already
> closed and at this point no data can be sent anymore. A proper shutdown
> would have to be initated by a user space application, such as a container
> management stack, that sends the command via the character device before
> terminating the TPM emulator.
> 
> To avoid the tpm2_shutdown we introduce a TPM_CHIP_FLAG_NO_SHUTDOWN flag
> that only the VTPM proxy driver sets. This also avoids misleading kernel
> log messages.

This seems strange to me..

Why isn't ops null if the fd has gone away?

What is the call flow that hits this?

Jason

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown
  2017-05-25 15:50   ` Jason Gunthorpe
@ 2017-05-25 20:04     ` Stefan Berger
  2017-05-25 20:09       ` Jason Gunthorpe
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Berger @ 2017-05-25 20:04 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: jarkko.sakkinen, tpmdd-devel, linux-security-module, linux-kernel

On 05/25/2017 11:50 AM, Jason Gunthorpe wrote:
> On Thu, May 25, 2017 at 09:12:36AM -0400, Stefan Berger wrote:
>> The tpm2_shutdown does not work with the VTPM proxy driver since the
>> function only gets called when the backend file descriptor is already
>> closed and at this point no data can be sent anymore. A proper shutdown
>> would have to be initated by a user space application, such as a container
>> management stack, that sends the command via the character device before
>> terminating the TPM emulator.
>>
>> To avoid the tpm2_shutdown we introduce a TPM_CHIP_FLAG_NO_SHUTDOWN flag
>> that only the VTPM proxy driver sets. This also avoids misleading kernel
>> log messages.
> This seems strange to me..
>
> Why isn't ops null if the fd has gone away?
>
> What is the call flow that hits this?

In this function here.

static void tpm_del_char_device(struct tpm_chip *chip)
{
     cdev_device_del(&chip->cdev, &chip->dev);

     /* Make the chip unavailable. */
     mutex_lock(&idr_lock);
     idr_replace(&dev_nums_idr, NULL, chip->dev_num);
     mutex_unlock(&idr_lock);

     /* Make the driver uncallable. */
     down_write(&chip->ops_sem);
     if (chip->flags & TPM_CHIP_FLAG_TPM2)
         tpm2_shutdown(chip, TPM2_SU_CLEAR);
     chip->ops = NULL;
     up_write(&chip->ops_sem);
}

The request cannot be deliver because the anonymous fd has been closed 
already.

     Stefan


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown
  2017-05-25 20:04     ` Stefan Berger
@ 2017-05-25 20:09       ` Jason Gunthorpe
  2017-05-25 20:32         ` Stefan Berger
  0 siblings, 1 reply; 10+ messages in thread
From: Jason Gunthorpe @ 2017-05-25 20:09 UTC (permalink / raw)
  To: Stefan Berger
  Cc: jarkko.sakkinen, tpmdd-devel, linux-security-module, linux-kernel

On Thu, May 25, 2017 at 04:04:24PM -0400, Stefan Berger wrote:
> On 05/25/2017 11:50 AM, Jason Gunthorpe wrote:
> >On Thu, May 25, 2017 at 09:12:36AM -0400, Stefan Berger wrote:
> >>The tpm2_shutdown does not work with the VTPM proxy driver since the
> >>function only gets called when the backend file descriptor is already
> >>closed and at this point no data can be sent anymore. A proper shutdown
> >>would have to be initated by a user space application, such as a container
> >>management stack, that sends the command via the character device before
> >>terminating the TPM emulator.
> >>
> >>To avoid the tpm2_shutdown we introduce a TPM_CHIP_FLAG_NO_SHUTDOWN flag
> >>that only the VTPM proxy driver sets. This also avoids misleading kernel
> >>log messages.
> >This seems strange to me..
> >
> >Why isn't ops null if the fd has gone away?
> >
> >What is the call flow that hits this?
> 
> In this function here.
> 
> static void tpm_del_char_device(struct tpm_chip *chip)
> {
>     cdev_device_del(&chip->cdev, &chip->dev);
> 
>     /* Make the chip unavailable. */
>     mutex_lock(&idr_lock);
>     idr_replace(&dev_nums_idr, NULL, chip->dev_num);
>     mutex_unlock(&idr_lock);
> 
>     /* Make the driver uncallable. */
>     down_write(&chip->ops_sem);
>     if (chip->flags & TPM_CHIP_FLAG_TPM2)
>         tpm2_shutdown(chip, TPM2_SU_CLEAR);
>     chip->ops = NULL;
>     up_write(&chip->ops_sem);
> }
> 
> The request cannot be deliver because the anonymous fd has been closed
> already.

The driver must always be able to process requests until
tpm_del_char_device completes, so this is triggering an existing bug
in vtpm. This change in core behvior is not going to fix the bug.

eg a request from sysfs/etc could come in between vtpm fd closure and
tpm_del_char_device, and it still must be handled properly.

I guess you need to have transmit command fail fast once the fd is
closed.

Jason

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown
  2017-05-25 20:09       ` Jason Gunthorpe
@ 2017-05-25 20:32         ` Stefan Berger
       [not found]           ` <9ff88c24-ca7a-1867-7284-17689fdac655-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Berger @ 2017-05-25 20:32 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: jarkko.sakkinen, tpmdd-devel, linux-security-module, linux-kernel

On 05/25/2017 04:09 PM, Jason Gunthorpe wrote:
> On Thu, May 25, 2017 at 04:04:24PM -0400, Stefan Berger wrote:
>> On 05/25/2017 11:50 AM, Jason Gunthorpe wrote:
>>> On Thu, May 25, 2017 at 09:12:36AM -0400, Stefan Berger wrote:
>>>> The tpm2_shutdown does not work with the VTPM proxy driver since the
>>>> function only gets called when the backend file descriptor is already
>>>> closed and at this point no data can be sent anymore. A proper shutdown
>>>> would have to be initated by a user space application, such as a container
>>>> management stack, that sends the command via the character device before
>>>> terminating the TPM emulator.
>>>>
>>>> To avoid the tpm2_shutdown we introduce a TPM_CHIP_FLAG_NO_SHUTDOWN flag
>>>> that only the VTPM proxy driver sets. This also avoids misleading kernel
>>>> log messages.
>>> This seems strange to me..
>>>
>>> Why isn't ops null if the fd has gone away?
>>>
>>> What is the call flow that hits this?
>> In this function here.
>>
>> static void tpm_del_char_device(struct tpm_chip *chip)
>> {
>>      cdev_device_del(&chip->cdev, &chip->dev);
>>
>>      /* Make the chip unavailable. */
>>      mutex_lock(&idr_lock);
>>      idr_replace(&dev_nums_idr, NULL, chip->dev_num);
>>      mutex_unlock(&idr_lock);
>>
>>      /* Make the driver uncallable. */
>>      down_write(&chip->ops_sem);
>>      if (chip->flags & TPM_CHIP_FLAG_TPM2)
>>          tpm2_shutdown(chip, TPM2_SU_CLEAR);
>>      chip->ops = NULL;
>>      up_write(&chip->ops_sem);
>> }
>>
>> The request cannot be deliver because the anonymous fd has been closed
>> already.
> The driver must always be able to process requests until
> tpm_del_char_device completes, so this is triggering an existing bug
> in vtpm. This change in core behvior is not going to fix the bug.
>
> eg a request from sysfs/etc could come in between vtpm fd closure and
> tpm_del_char_device, and it still must be handled properly.
>
> I guess you need to have transmit command fail fast once the fd is
> closed.

It doesn't hang. Everything is torn down immediately. What is primarily 
annoying are these two log messages:
tpm tpm0: tpm_transmit: tpm_send: error -32
tpm tpm0: transmit returned -32 while stopping the TPM


     Stefan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown
       [not found]           ` <9ff88c24-ca7a-1867-7284-17689fdac655-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
@ 2017-05-25 20:44             ` Jason Gunthorpe
       [not found]               ` <20170525204414.GA13742-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
  2017-05-25 22:33             ` Jarkko Sakkinen
  1 sibling, 1 reply; 10+ messages in thread
From: Jason Gunthorpe @ 2017-05-25 20:44 UTC (permalink / raw)
  To: Stefan Berger
  Cc: linux-security-module-u79uwXL29TY76Z2rM5mHXA,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On Thu, May 25, 2017 at 04:32:50PM -0400, Stefan Berger wrote:

> It doesn't hang. Everything is torn down immediately. What is primarily
> annoying are these two log messages:
> tpm tpm0: tpm_transmit: tpm_send: error -32
> tpm tpm0: transmit returned -32 while stopping the TPM

I think it would be better to change the core to suppress that logging
if the FD is closed.

Jason

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown
       [not found]               ` <20170525204414.GA13742-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
@ 2017-05-25 20:54                 ` Stefan Berger
  2017-05-25 21:00                   ` Jason Gunthorpe
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Berger @ 2017-05-25 20:54 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: linux-security-module-u79uwXL29TY76Z2rM5mHXA,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On 05/25/2017 04:44 PM, Jason Gunthorpe wrote:
> On Thu, May 25, 2017 at 04:32:50PM -0400, Stefan Berger wrote:
>
>> It doesn't hang. Everything is torn down immediately. What is primarily
>> annoying are these two log messages:
>> tpm tpm0: tpm_transmit: tpm_send: error -32
>> tpm tpm0: transmit returned -32 while stopping the TPM
> I think it would be better to change the core to suppress that logging
> if the FD is closed.

This particular command will never reach anyone listening on the proxy's 
file descriptor since the tear-down only begins when the front- and 
backend are closed.
The logging happens somewhere else than where the error occurs. What is 
the best way to suppress the logging? Remove it entirely  -- probably 
not. Return a special error code that doesn't get logged? Return a 2nd 
parameter that indicates this condition? It's not clear to me. Why not 
just prevent the command from being sent if it will never reach its 
intended destination ?

   Stefan

>
> Jason
>


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown
  2017-05-25 20:54                 ` Stefan Berger
@ 2017-05-25 21:00                   ` Jason Gunthorpe
  0 siblings, 0 replies; 10+ messages in thread
From: Jason Gunthorpe @ 2017-05-25 21:00 UTC (permalink / raw)
  To: Stefan Berger
  Cc: jarkko.sakkinen, tpmdd-devel, linux-security-module, linux-kernel

On Thu, May 25, 2017 at 04:54:01PM -0400, Stefan Berger wrote:

> This particular command will never reach anyone listening on the proxy's
> file descriptor since the tear-down only begins when the front- and backend
> are closed.

> The logging happens somewhere else than where the error occurs. What is the
> best way to suppress the logging? Remove it entirely  -- probably not.
> Return a special error code that doesn't get logged?

Error code would be my choice.

> that indicates this condition? It's not clear to me. Why not just
> prevent the command from being sent if it will never reach its
> intended destination

There are many cases where the vtpm shutdown can race with something
else, if the logging for this is bothersome it should be fixed
directly.

Adding strange special case flags is confusing as to the purpose - eg
your commit message didn't even say this is only about fixing some
noisy logging.

Jason

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown
       [not found]           ` <9ff88c24-ca7a-1867-7284-17689fdac655-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
  2017-05-25 20:44             ` Jason Gunthorpe
@ 2017-05-25 22:33             ` Jarkko Sakkinen
       [not found]               ` <20170525223348.uh66n37dnvz3eptl-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
  1 sibling, 1 reply; 10+ messages in thread
From: Jarkko Sakkinen @ 2017-05-25 22:33 UTC (permalink / raw)
  To: Stefan Berger
  Cc: linux-security-module-u79uwXL29TY76Z2rM5mHXA,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On Thu, May 25, 2017 at 04:32:50PM -0400, Stefan Berger wrote:
> On 05/25/2017 04:09 PM, Jason Gunthorpe wrote:
> > On Thu, May 25, 2017 at 04:04:24PM -0400, Stefan Berger wrote:
> > > On 05/25/2017 11:50 AM, Jason Gunthorpe wrote:
> > > > On Thu, May 25, 2017 at 09:12:36AM -0400, Stefan Berger wrote:
> > > > > The tpm2_shutdown does not work with the VTPM proxy driver since the
> > > > > function only gets called when the backend file descriptor is already
> > > > > closed and at this point no data can be sent anymore. A proper shutdown
> > > > > would have to be initated by a user space application, such as a container
> > > > > management stack, that sends the command via the character device before
> > > > > terminating the TPM emulator.
> > > > > 
> > > > > To avoid the tpm2_shutdown we introduce a TPM_CHIP_FLAG_NO_SHUTDOWN flag
> > > > > that only the VTPM proxy driver sets. This also avoids misleading kernel
> > > > > log messages.
> > > > This seems strange to me..
> > > > 
> > > > Why isn't ops null if the fd has gone away?
> > > > 
> > > > What is the call flow that hits this?
> > > In this function here.
> > > 
> > > static void tpm_del_char_device(struct tpm_chip *chip)
> > > {
> > >      cdev_device_del(&chip->cdev, &chip->dev);
> > > 
> > >      /* Make the chip unavailable. */
> > >      mutex_lock(&idr_lock);
> > >      idr_replace(&dev_nums_idr, NULL, chip->dev_num);
> > >      mutex_unlock(&idr_lock);
> > > 
> > >      /* Make the driver uncallable. */
> > >      down_write(&chip->ops_sem);
> > >      if (chip->flags & TPM_CHIP_FLAG_TPM2)
> > >          tpm2_shutdown(chip, TPM2_SU_CLEAR);
> > >      chip->ops = NULL;
> > >      up_write(&chip->ops_sem);
> > > }
> > > 
> > > The request cannot be deliver because the anonymous fd has been closed
> > > already.
> > The driver must always be able to process requests until
> > tpm_del_char_device completes, so this is triggering an existing bug
> > in vtpm. This change in core behvior is not going to fix the bug.
> > 
> > eg a request from sysfs/etc could come in between vtpm fd closure and
> > tpm_del_char_device, and it still must be handled properly.
> > 
> > I guess you need to have transmit command fail fast once the fd is
> > closed.
> 
> It doesn't hang. Everything is torn down immediately. What is primarily
> annoying are these two log messages:
> tpm tpm0: tpm_transmit: tpm_send: error -32
> tpm tpm0: transmit returned -32 while stopping the TPM
> 
> 
>     Stefan

It's been a while since I've looked into vtpm code. Why was fd closed
before the above? I can go this through myself once I'm back in Finland
next week. Just have forgotten this detail and do not have time to study
this right now.

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown
       [not found]               ` <20170525223348.uh66n37dnvz3eptl-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
@ 2017-05-25 23:34                 ` Stefan Berger
  0 siblings, 0 replies; 10+ messages in thread
From: Stefan Berger @ 2017-05-25 23:34 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: linux-security-module-u79uwXL29TY76Z2rM5mHXA,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On 05/25/2017 06:33 PM, Jarkko Sakkinen wrote:
> On Thu, May 25, 2017 at 04:32:50PM -0400, Stefan Berger wrote:
>> On 05/25/2017 04:09 PM, Jason Gunthorpe wrote:
>>> On Thu, May 25, 2017 at 04:04:24PM -0400, Stefan Berger wrote:
>>>> On 05/25/2017 11:50 AM, Jason Gunthorpe wrote:
>>>>> On Thu, May 25, 2017 at 09:12:36AM -0400, Stefan Berger wrote:
>>>>>> The tpm2_shutdown does not work with the VTPM proxy driver since the
>>>>>> function only gets called when the backend file descriptor is already
>>>>>> closed and at this point no data can be sent anymore. A proper shutdown
>>>>>> would have to be initated by a user space application, such as a container
>>>>>> management stack, that sends the command via the character device before
>>>>>> terminating the TPM emulator.
>>>>>>
>>>>>> To avoid the tpm2_shutdown we introduce a TPM_CHIP_FLAG_NO_SHUTDOWN flag
>>>>>> that only the VTPM proxy driver sets. This also avoids misleading kernel
>>>>>> log messages.
>>>>> This seems strange to me..
>>>>>
>>>>> Why isn't ops null if the fd has gone away?
>>>>>
>>>>> What is the call flow that hits this?
>>>> In this function here.
>>>>
>>>> static void tpm_del_char_device(struct tpm_chip *chip)
>>>> {
>>>>       cdev_device_del(&chip->cdev, &chip->dev);
>>>>
>>>>       /* Make the chip unavailable. */
>>>>       mutex_lock(&idr_lock);
>>>>       idr_replace(&dev_nums_idr, NULL, chip->dev_num);
>>>>       mutex_unlock(&idr_lock);
>>>>
>>>>       /* Make the driver uncallable. */
>>>>       down_write(&chip->ops_sem);
>>>>       if (chip->flags & TPM_CHIP_FLAG_TPM2)
>>>>           tpm2_shutdown(chip, TPM2_SU_CLEAR);
>>>>       chip->ops = NULL;
>>>>       up_write(&chip->ops_sem);
>>>> }
>>>>
>>>> The request cannot be deliver because the anonymous fd has been closed
>>>> already.
>>> The driver must always be able to process requests until
>>> tpm_del_char_device completes, so this is triggering an existing bug
>>> in vtpm. This change in core behvior is not going to fix the bug.
>>>
>>> eg a request from sysfs/etc could come in between vtpm fd closure and
>>> tpm_del_char_device, and it still must be handled properly.
>>>
>>> I guess you need to have transmit command fail fast once the fd is
>>> closed.
>> It doesn't hang. Everything is torn down immediately. What is primarily
>> annoying are these two log messages:
>> tpm tpm0: tpm_transmit: tpm_send: error -32
>> tpm tpm0: transmit returned -32 while stopping the TPM
>>
>>
>>      Stefan
> It's been a while since I've looked into vtpm code. Why was fd closed
> before the above? I can go this through myself once I'm back in Finland

The tear-down only starts when the last file descriptor on /dev/tpm%d 
and the (anonymous) file descriptor on the backend was closed. The 
backend (TPM emulator) can also close before the frontend closes. So it 
can happen that there is no backend anymore that would process commands 
and that would cause error messages being logged for commands sent to 
it. For sure there is no more listener for the TPM2 Shutdown command.


> next week. Just have forgotten this detail and do not have time to study
> this right now.
>
> /Jarkko
>


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-05-25 23:34 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-25 13:12 [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown Stefan Berger
     [not found] ` <1495717956-14252-1-git-send-email-stefanb-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
2017-05-25 15:50   ` Jason Gunthorpe
2017-05-25 20:04     ` Stefan Berger
2017-05-25 20:09       ` Jason Gunthorpe
2017-05-25 20:32         ` Stefan Berger
     [not found]           ` <9ff88c24-ca7a-1867-7284-17689fdac655-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
2017-05-25 20:44             ` Jason Gunthorpe
     [not found]               ` <20170525204414.GA13742-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
2017-05-25 20:54                 ` Stefan Berger
2017-05-25 21:00                   ` Jason Gunthorpe
2017-05-25 22:33             ` Jarkko Sakkinen
     [not found]               ` <20170525223348.uh66n37dnvz3eptl-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2017-05-25 23:34                 ` Stefan Berger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).