Re: [tpmdd-devel] [PATCH v2 4/7] tpm: infrastructure for TPM spaces

[Jarkko Sakkinen]

On Fri, Feb 24, 2017 at 07:53:22AM -0500, James Bottomley wrote:
> On Thu, 2017-02-16 at 21:25 +0200, Jarkko Sakkinen wrote:
> > Added an ability to virtualize TPM commands into an isolated context
> > that we call a TPM space because the word context is already heavily
> > used in the TPM specification. Both the handle areas and bodies
> > (where
> > necessary) are virtualized.
> > 
> > The mechanism works by adding a new parameter struct tpm_space to the
> > tpm_transmit() function. This new structure contains the list of
> > virtual
> > handles and a buffer of page size (currently) for backing storage.
> > 
> > When tpm_transmit() is called with a struct tpm_space instance it
> > will
> > execute the following sequence:
> > 
> > 1. Take locks.
> > 2. Load transient objects from the backing storage by using
> > ContextLoad
> >and map virtual handles to physical handles.
> > 3. Perform the transaction.
> > 4. Save transient objects to backing storage by using ContextSave and
> >map resulting physical handle to virtual handle if there is such.
> > 
> > This commit does not implement virtualization support for hmac and
> > policy sessions.
> > 
> > Signed-off-by: Jarkko Sakkinen 
> 
> For patches 1-4 you can add 
> 
> Reviewed-by: James Bottomley 
> 
> Just re-running a build with the latest kernel for my laptop to add
> tested by.
> 
> James

Thank you!

/Jarkko

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: [tpmdd-devel] [PATCH v2 4/7] tpm: infrastructure for TPM spaces

[Ken Goldman]

On 2/22/2017 12:39 PM, James Bottomley wrote:
>
> Right at the moment the kernel use of tpm2 looks like
>
> acquire chip->tpm_mutex
> load key
> process key
> unload key
> release chip->tpm_mutex

The advantage to context save/ context load over load / flush
is that load requires the parent(s).  The parent chain may be long,
a parent may require authorization, or authorization may be impossible 
because PCRs are no longer in the correct state.

In TPM 1.2, there was a performance difference because load was an 
asymmetric key operation, but it's symmetric in TPM 2.0.

> When the kernel needs to use resources that persisted beyond it
> dropping the chip->tpm_mutex (say using policy or audit sessions),
> then it would need to become a customer of the RM.

BTW, use of an EK private key requires a policy session.




--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: [tpmdd-devel] [PATCH v2 4/7] tpm: infrastructure for TPM spaces

[James Bottomley]

On Tue, 2017-02-21 at 23:54 +0530, Nayna wrote:
> 
> On 02/17/2017 12:55 AM, Jarkko Sakkinen wrote:
> > Added an ability to virtualize TPM commands into an isolated 
> > context that we call a TPM space because the word context is 
> > already heavily used in the TPM specification. Both the handle 
> > areas and bodies (where necessary) are virtualized.
> > 
> > The mechanism works by adding a new parameter struct tpm_space to 
> > the tpm_transmit() function. This new structure contains the list 
> > of virtual handles and a buffer of page size (currently) for
> > backing storage.
> > 
> > When tpm_transmit() is called with a struct tpm_space instance it 
> > will execute the following sequence:
> > 
> > 1. Take locks.
> > 2. Load transient objects from the backing storage by using
> > ContextLoad
> > and map virtual handles to physical handles.
> > 3. Perform the transaction.
> > 4. Save transient objects to backing storage by using ContextSave
> > and
> > map resulting physical handle to virtual handle if there is
> > such.
> > 
> > This commit does not implement virtualization support for hmac and
> > policy sessions.
> > 
> 
> If I have understood discussions correctly, I assume, that kernel TPM
> operations will also be routed via RM. And I think that is not 
> happening now with these patches.
> 
> Am I missing something ?

Right at the moment the kernel use of tpm2 looks like

acquire chip->tpm_mutex
load key
process key
unload key
release chip->tpm_mutex

While it does this, there's no need for it to have a RM interface
because what it does between the acquisition and drop of the mutex
can't be seen by or have any effect on userspace (whether it uses the
RM or not).  So currently, the question doesn't arise, which is the
situation you see.

When the kernel needs to use resources that persisted beyond it
dropping the chip->tpm_mutex (say using policy or audit sessions), then
it would need to become a customer of the RM.

James


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: [tpmdd-devel] [PATCH v2 4/7] tpm: infrastructure for TPM spaces

[Ken Goldman]

On 2/21/2017 1:24 PM, Nayna wrote:
>
> [snip]
>>
>> 1. Take locks.
>> 2. Load transient objects from the backing storage by using ContextLoad
>> and map virtual handles to physical handles.
>> 3. Perform the transaction.
>> 4. Save transient objects to backing storage by using ContextSave and
>> map resulting physical handle to virtual handle if there is such.
>>
>> This commit does not implement virtualization support for hmac and
>> policy sessions.
>>
>
> If I have understood discussions correctly, I assume, that kernel TPM
> operations will also be routed via RM. And I think that is not happening
> now with these patches.

There is one corner case that requires kernel operations to go through
the RM, and that's the session context regapping.  If the kernel did not 
go through the RM, it could not handle TPM_RC_CONTEXT_GAP.

I believe that kernel operations such as PCR extend, that don't require
sessions, do not have to go through the RM.

Kernel operations that use objects perhaps can bypass the RM as long as 
there is a lock and the kernel also context saves objects and returns 
the TPM to the empty state before it releases the lock.

Finally, I suspect that the RM should reserve 3 sessions for kernel
operations, so the kernel can't block because user space applications 
have filled all the session slots.


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: [tpmdd-devel] [PATCH v2 4/7] tpm: infrastructure for TPM spaces

[Nayna]

On 02/17/2017 12:55 AM, Jarkko Sakkinen wrote:
> Added an ability to virtualize TPM commands into an isolated context
> that we call a TPM space because the word context is already heavily
> used in the TPM specification. Both the handle areas and bodies (where
> necessary) are virtualized.
>
> The mechanism works by adding a new parameter struct tpm_space to the
> tpm_transmit() function. This new structure contains the list of virtual
> handles and a buffer of page size (currently) for backing storage.
>
> When tpm_transmit() is called with a struct tpm_space instance it will
> execute the following sequence:
>
> 1. Take locks.
> 2. Load transient objects from the backing storage by using ContextLoad
> and map virtual handles to physical handles.
> 3. Perform the transaction.
> 4. Save transient objects to backing storage by using ContextSave and
> map resulting physical handle to virtual handle if there is such.
>
> This commit does not implement virtualization support for hmac and
> policy sessions.
>

If I have understood discussions correctly, I assume, that kernel TPM 
operations will also be routed via RM. And I think that is not happening 
now with these patches.

Am I missing something ?

Thanks & Regards,
- Nayna


> Signed-off-by: Jarkko Sakkinen 
> ---
>   drivers/char/tpm/Makefile|   2 +-
>   drivers/char/tpm/tpm-chip.c  |   7 +
>   drivers/char/tpm/tpm-dev.c   |   2 +-
>   drivers/char/tpm/tpm-interface.c |  68 +++---
>   drivers/char/tpm/tpm-sysfs.c |   2 +-
>   drivers/char/tpm/tpm.h   |  26 ++-
>   drivers/char/tpm/tpm2-cmd.c  |  33 +--
>   drivers/char/tpm/tpm2-space.c| 431 
> +++
>   8 files changed, 520 insertions(+), 51 deletions(-)
>   create mode 100644 drivers/char/tpm/tpm2-space.c
>
> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
> index 3d386a8..8f07fcf 100644
> --- a/drivers/char/tpm/Makefile
> +++ b/drivers/char/tpm/Makefile
> @@ -3,7 +3,7 @@
>   #
>   obj-$(CONFIG_TCG_TPM) += tpm.o
>   tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
> - tpm1_eventlog.o tpm2_eventlog.o
> +  tpm1_eventlog.o tpm2_eventlog.o tpm2-space.o
>   tpm-$(CONFIG_ACPI) += tpm_ppi.o tpm_acpi.o
>   tpm-$(CONFIG_OF) += tpm_of.o
>   obj-$(CONFIG_TCG_TIS_CORE) += tpm_tis_core.o
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index c406343..993b9ae 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -128,6 +128,7 @@ static void tpm_dev_release(struct device *dev)
>   mutex_unlock(_lock);
>
>   kfree(chip->log.bios_event_log);
> + kfree(chip->work_space.context_buf);
>   kfree(chip);
>   }
>
> @@ -189,6 +190,12 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
>   chip->cdev.owner = THIS_MODULE;
>   chip->cdev.kobj.parent = >dev.kobj;
>
> + chip->work_space.context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
> + if (!chip->work_space.context_buf) {
> + rc = -ENOMEM;
> + goto out;
> + }
> +
>   return chip;
>
>   out:
> diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
> index 02a8850..414553b 100644
> --- a/drivers/char/tpm/tpm-dev.c
> +++ b/drivers/char/tpm/tpm-dev.c
> @@ -147,7 +147,7 @@ static ssize_t tpm_write(struct file *file, const char 
> __user *buf,
>   mutex_unlock(>buffer_mutex);
>   return -EPIPE;
>   }
> - out_size = tpm_transmit(priv->chip, priv->data_buffer,
> + out_size = tpm_transmit(priv->chip, NULL, priv->data_buffer,
>   sizeof(priv->data_buffer), 0);
>
>   tpm_put_ops(priv->chip);
> diff --git a/drivers/char/tpm/tpm-interface.c 
> b/drivers/char/tpm/tpm-interface.c
> index 20b1fe3..db5ffe9 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -376,11 +376,12 @@ static bool tpm_validate_command(struct tpm_chip *chip, 
> const u8 *cmd,
>* 0 when the operation is successful.
>* A negative number for system errors (errno).
>*/
> -ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
> -  unsigned int flags)
> +ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
> +  u8 *buf, size_t bufsiz, unsigned int flags)
>   {
> - const struct tpm_output_header *header = (void *)buf;
> - ssize_t rc;
> + struct tpm_output_header *header = (void *)buf;
> + int rc;
> + ssize_t len = 0;
>   u32 count, ordinal;
>   unsigned long stop;
>
> @@ -406,10 +407,14 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const u8 
> *buf, size_t bufsiz,
>   if (chip->dev.parent)
>   pm_runtime_get_sync(chip->dev.parent);
>
> + rc = tpm2_prepare_space(chip, space, ordinal, buf);
> + if (rc)
> + goto out;
> +
>   rc = chip->ops->send(chip, (u8 *) buf,