All of lore.kernel.org
 help / color / mirror / Atom feed
From: jgunthorpe@obsidianresearch.com (Jason Gunthorpe)
To: linux-security-module@vger.kernel.org
Subject: [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown
Date: Thu, 25 May 2017 14:09:31 -0600	[thread overview]
Message-ID: <20170525200931.GA12733@obsidianresearch.com> (raw)
In-Reply-To: <f6c024ae-fe93-251a-a176-1f27f25b1e21@linux.vnet.ibm.com>

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
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
To: Stefan Berger <stefanb@linux.vnet.ibm.com>
Cc: jarkko.sakkinen@linux.intel.com,
	tpmdd-devel@lists.sourceforge.net,
	linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown
Date: Thu, 25 May 2017 14:09:31 -0600	[thread overview]
Message-ID: <20170525200931.GA12733@obsidianresearch.com> (raw)
In-Reply-To: <f6c024ae-fe93-251a-a176-1f27f25b1e21@linux.vnet.ibm.com>

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

  reply	other threads:[~2017-05-25 20:09 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-25 13:12 [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown Stefan Berger
2017-05-25 13:12 ` Stefan Berger
2017-05-25 13:12 ` Stefan Berger
2017-05-25 15:50 ` Jason Gunthorpe
2017-05-25 15:50   ` Jason Gunthorpe
2017-05-25 15:50   ` Jason Gunthorpe
2017-05-25 20:04   ` Stefan Berger
2017-05-25 20:04     ` Stefan Berger
2017-05-25 20:09     ` Jason Gunthorpe [this message]
2017-05-25 20:09       ` Jason Gunthorpe
2017-05-25 20:32       ` Stefan Berger
2017-05-25 20:32         ` Stefan Berger
2017-05-25 20:44         ` Jason Gunthorpe
2017-05-25 20:44           ` Jason Gunthorpe
2017-05-25 20:44           ` Jason Gunthorpe
2017-05-25 20:54           ` Stefan Berger
2017-05-25 20:54             ` Stefan Berger
2017-05-25 20:54             ` Stefan Berger
2017-05-25 21:00             ` Jason Gunthorpe
2017-05-25 21:00               ` Jason Gunthorpe
2017-05-25 22:33         ` Jarkko Sakkinen
2017-05-25 22:33           ` Jarkko Sakkinen
2017-05-25 22:33           ` Jarkko Sakkinen
2017-05-25 23:34           ` Stefan Berger
2017-05-25 23:34             ` Stefan Berger
2017-05-25 23:34             ` 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=20170525200931.GA12733@obsidianresearch.com \
    --to=jgunthorpe@obsidianresearch.com \
    --cc=linux-security-module@vger.kernel.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.