* [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown @ 2017-05-25 13:12 ` Stefan Berger 0 siblings, 0 replies; 26+ messages in thread From: Stefan Berger @ 2017-05-25 13:12 UTC (permalink / raw) To: linux-security-module 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@linux.vnet.ibm.com> --- 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 -- 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 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown @ 2017-05-25 13:12 ` Stefan Berger 0 siblings, 0 replies; 26+ messages in thread From: Stefan Berger @ 2017-05-25 13:12 UTC (permalink / raw) To: jarkko.sakkinen, tpmdd-devel Cc: linux-security-module, linux-kernel, jgunthorpe, Stefan Berger 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@linux.vnet.ibm.com> --- 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 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown @ 2017-05-25 13:12 ` Stefan Berger 0 siblings, 0 replies; 26+ 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] 26+ messages in thread
* [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown @ 2017-05-25 15:50 ` Jason Gunthorpe 0 siblings, 0 replies; 26+ messages in thread From: Jason Gunthorpe @ 2017-05-25 15:50 UTC (permalink / raw) To: linux-security-module 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 -- 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown @ 2017-05-25 15:50 ` Jason Gunthorpe 0 siblings, 0 replies; 26+ messages in thread From: Jason Gunthorpe @ 2017-05-25 15:50 UTC (permalink / raw) To: Stefan Berger Cc: jarkko.sakkinen, tpmdd-devel, linux-security-module, linux-kernel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown @ 2017-05-25 15:50 ` Jason Gunthorpe 0 siblings, 0 replies; 26+ 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] 26+ messages in thread
* [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown 2017-05-25 15:50 ` Jason Gunthorpe @ 2017-05-25 20:04 ` Stefan Berger -1 siblings, 0 replies; 26+ messages in thread From: Stefan Berger @ 2017-05-25 20:04 UTC (permalink / raw) To: linux-security-module 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 -- 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown @ 2017-05-25 20:04 ` Stefan Berger 0 siblings, 0 replies; 26+ 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] 26+ messages in thread
* [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown 2017-05-25 20:04 ` Stefan Berger @ 2017-05-25 20:09 ` Jason Gunthorpe -1 siblings, 0 replies; 26+ messages in thread From: Jason Gunthorpe @ 2017-05-25 20:09 UTC (permalink / raw) To: linux-security-module 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown @ 2017-05-25 20:09 ` Jason Gunthorpe 0 siblings, 0 replies; 26+ 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] 26+ messages in thread
* [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown 2017-05-25 20:09 ` Jason Gunthorpe @ 2017-05-25 20:32 ` Stefan Berger -1 siblings, 0 replies; 26+ messages in thread From: Stefan Berger @ 2017-05-25 20:32 UTC (permalink / raw) To: linux-security-module 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 -- 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown @ 2017-05-25 20:32 ` Stefan Berger 0 siblings, 0 replies; 26+ 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] 26+ messages in thread
* [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown @ 2017-05-25 20:44 ` Jason Gunthorpe 0 siblings, 0 replies; 26+ messages in thread From: Jason Gunthorpe @ 2017-05-25 20:44 UTC (permalink / raw) To: linux-security-module 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 -- 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown @ 2017-05-25 20:44 ` Jason Gunthorpe 0 siblings, 0 replies; 26+ messages in thread From: Jason Gunthorpe @ 2017-05-25 20:44 UTC (permalink / raw) To: Stefan Berger Cc: jarkko.sakkinen, tpmdd-devel, linux-security-module, linux-kernel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown @ 2017-05-25 20:44 ` Jason Gunthorpe 0 siblings, 0 replies; 26+ 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] 26+ messages in thread
* [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown @ 2017-05-25 20:54 ` Stefan Berger 0 siblings, 0 replies; 26+ messages in thread From: Stefan Berger @ 2017-05-25 20:54 UTC (permalink / raw) To: linux-security-module 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 > -- 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown @ 2017-05-25 20:54 ` Stefan Berger 0 siblings, 0 replies; 26+ messages in thread From: Stefan Berger @ 2017-05-25 20:54 UTC (permalink / raw) To: Jason Gunthorpe Cc: jarkko.sakkinen, tpmdd-devel, linux-security-module, linux-kernel 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 > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown @ 2017-05-25 20:54 ` Stefan Berger 0 siblings, 0 replies; 26+ 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] 26+ messages in thread
* [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown 2017-05-25 20:54 ` Stefan Berger @ 2017-05-25 21:00 ` Jason Gunthorpe -1 siblings, 0 replies; 26+ messages in thread From: Jason Gunthorpe @ 2017-05-25 21:00 UTC (permalink / raw) To: linux-security-module 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 -- 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown @ 2017-05-25 21:00 ` Jason Gunthorpe 0 siblings, 0 replies; 26+ 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] 26+ messages in thread
* [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown @ 2017-05-25 22:33 ` Jarkko Sakkinen 0 siblings, 0 replies; 26+ messages in thread From: Jarkko Sakkinen @ 2017-05-25 22:33 UTC (permalink / raw) To: linux-security-module 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 -- 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown @ 2017-05-25 22:33 ` Jarkko Sakkinen 0 siblings, 0 replies; 26+ messages in thread From: Jarkko Sakkinen @ 2017-05-25 22:33 UTC (permalink / raw) To: Stefan Berger Cc: Jason Gunthorpe, tpmdd-devel, linux-security-module, linux-kernel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown @ 2017-05-25 22:33 ` Jarkko Sakkinen 0 siblings, 0 replies; 26+ 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] 26+ messages in thread
* [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown @ 2017-05-25 23:34 ` Stefan Berger 0 siblings, 0 replies; 26+ messages in thread From: Stefan Berger @ 2017-05-25 23:34 UTC (permalink / raw) To: linux-security-module 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 > -- 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown @ 2017-05-25 23:34 ` Stefan Berger 0 siblings, 0 replies; 26+ messages in thread From: Stefan Berger @ 2017-05-25 23:34 UTC (permalink / raw) To: Jarkko Sakkinen Cc: Jason Gunthorpe, tpmdd-devel, linux-security-module, linux-kernel 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 > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tpm: vtpm_proxy: Do not run tpm2_shutdown @ 2017-05-25 23:34 ` Stefan Berger 0 siblings, 0 replies; 26+ 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] 26+ messages in thread
end of thread, other threads:[~2017-05-25 23:35 UTC | newest] Thread overview: 26+ 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 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 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
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.