Re: [PATCH v4 08/10] tpm: Driver for supporting multiple emulated TPMs

["Stefan Berger" <stefanb-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 6679 bytes --]

Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote on 03/04/2016 
03:29:28 PM:
> 
> Hi
> 
> I noticed some glitches.
> 
> On Mon, Feb 29, 2016 at 12:29:54PM -0500, Stefan Berger wrote:
> > This patch implements a driver for supporting multiple emulated TPMs 
in a
> > system.
> > 
> > The driver implements a device /dev/vtpmx that is used to created
> > a client device pair /dev/tpmX (e.g., /dev/tpm10) and a server side 
that
> > is accessed using a file descriptor returned by an ioctl.
> > The device /dev/tpmX is the usual TPM device created by the core TPM
> > driver. Applications or kernel subsystems can send TPM commands to it
> > and the corresponding server-side file descriptor receives these
> > commands and delivers them to an emulated TPM.
> 
> Every other driver in the subsystem except xen-tpmfront uses
> underscore instead of dash. For the sake of consistency this should be
> renamed as tpm_vtpm.

fixed.


> 
> 
> > Signed-off-by: Stefan Berger <stefanb-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> > ---
> >  drivers/char/tpm/Kconfig    |  10 +
> >  drivers/char/tpm/Makefile   |   1 +
> >  drivers/char/tpm/tpm-vtpm.c | 546 +++++++++++++++++++++++++++++++
> +++++++++++++
> >  include/uapi/linux/Kbuild   |   1 +
> >  include/uapi/linux/vtpm.h   |  41 ++++
> >  5 files changed, 599 insertions(+)
> >  create mode 100644 drivers/char/tpm/tpm-vtpm.c
> >  create mode 100644 include/uapi/linux/vtpm.h
> > 
> > diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
> > index 3b84a8b..4c4e843 100644
> > --- a/drivers/char/tpm/Kconfig
> > +++ b/drivers/char/tpm/Kconfig
> > @@ -122,5 +122,15 @@ config TCG_CRB
> >       from within Linux.  To compile this driver as a module, choose
> >       M here; the module will be called tpm_crb.
> > 
> > +config TCG_VTPM
> > +   tristate "VTPM Interface"
> > +   depends on TCG_TPM
> > +   ---help---
> > +     This driver supports an emulated TPM (vTPM) running in 
userspace.
> > +     A device /dev/vtpmx is provided that creates a device pair
> > +     /dev/vtpmX and a server-side file descriptor on which the vTPM
> > +     can receive commands.
> > +
> > +
> >  source "drivers/char/tpm/st33zp24/Kconfig"
> >  endif # TCG_TPM
> > diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
> > index 56e8f1f..d947db2 100644
> > --- a/drivers/char/tpm/Makefile
> > +++ b/drivers/char/tpm/Makefile
> > @@ -23,3 +23,4 @@ obj-$(CONFIG_TCG_IBMVTPM) += tpm_ibmvtpm.o
> >  obj-$(CONFIG_TCG_TIS_ST33ZP24) += st33zp24/
> >  obj-$(CONFIG_TCG_XEN) += xen-tpmfront.o
> >  obj-$(CONFIG_TCG_CRB) += tpm_crb.o
> > +obj-$(CONFIG_TCG_VTPM) += tpm-vtpm.o
> > diff --git a/drivers/char/tpm/tpm-vtpm.c b/drivers/char/tpm/tpm-vtpm.c
> > new file mode 100644
> > index 0000000..2c69363
> > --- /dev/null
> > +++ b/drivers/char/tpm/tpm-vtpm.c
> > @@ -0,0 +1,546 @@
> > +/*
> > + * Copyright (C) 2015, 2016 IBM Corporation
> > + *
> > + * Author: Stefan Berger <stefanb-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> > + *
> > + * Maintained by: <tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
> > + *
> > + * Device driver for vTPM.
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation, version 2 of the
> > + * License.
> > + *
> > + */
> > +
> > +#include <linux/types.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/uaccess.h>
> > +#include <linux/wait.h>
> > +#include <linux/miscdevice.h>
> > +#include <linux/vtpm.h>
> > +#include <linux/file.h>
> > +#include <linux/anon_inodes.h>
> > +#include <linux/poll.h>
> > +#include <linux/compat.h>
> > +
> > +#include "tpm.h"
> > +
> > +#define VTPM_REQ_COMPLETE_FLAG  BIT(0)
> > +
> > +struct vtpm_dev {
> > +   struct tpm_chip *chip;
> > +
> > +   u32 flags;                   /* public API flags */
> > +
> > +   wait_queue_head_t wq;
> > +
> > +   struct mutex buf_lock;       /* protect buffer and flag 
modifications */
> > +
> > +   long state;                  /* internal state */
> > +#define STATE_OPENED_FLAG        BIT(0)
> > +#define STATE_WAIT_RESPONSE_FLAG BIT(1)  /* waiting for emulator 
> response */
> > +
> > +   size_t req_len;              /* length of queued TPM request */
> > +   size_t resp_len;             /* length of queued TPM response */
> > +   u8 buffer[TPM_BUFSIZE];      /* request/response buffer */
> > +};
> > +
> > +
> > +static void vtpm_delete_device_pair(struct vtpm_dev *vtpm_dev);
> > +
> > +/*
> > + * Functions related to 'server side'
> > + */
> > +
> > +/**
> > + * vtpm_fops_read - Read TPM commands on 'server side'
> > + *
> > + * Return value:
> > + *   Number of bytes read or negative error code
> > + */
> > +static ssize_t vtpm_fops_read(struct file *filp, char __user *buf,
> > +               size_t count, loff_t *off)
> > +{
> > +   struct vtpm_dev *vtpm_dev = filp->private_data;
> > +   size_t len;
> > +   int sig, rc;
> > +
> > +   sig = wait_event_interruptible(vtpm_dev->wq, vtpm_dev->req_len != 
0);
> > +   if (sig)
> > +      return -EINTR;
> > +
> > +   mutex_lock(&vtpm_dev->buf_lock);
> > +
> > +   len = vtpm_dev->req_len;
> > +
> > +   if (count < len) {
> > +      mutex_unlock(&vtpm_dev->buf_lock);
> > +      pr_debug("Invalid size in recv: count=%zd, req_len=%zd\n",
> > +          count, len);
> > +      return -EIO;
> > +   }
> > +
> > +   rc = copy_to_user(buf, vtpm_dev->buffer, len);
> > +   memset(vtpm_dev->buffer, 0, len);
> > +   vtpm_dev->req_len = 0;
> > +
> > +   if (!rc)
> > +      vtpm_dev->state |= STATE_WAIT_RESPONSE_FLAG;
> > +
> > +   mutex_unlock(&vtpm_dev->buf_lock);
> > +
> > +   if (rc)
> > +      return -EFAULT;
> > +
> > +   return len;
> > +}
> > +
> > +/**
> > + * vtpm_fops_write - Write TPM responses on 'server side'
> > + *
> > + * Return value:
> > + *   Number of bytes read or negative error value
> > + */
> > +static ssize_t vtpm_fops_write(struct file *filp, const char __user 
*buf,
> > +                size_t count, loff_t *off)
> > +{
> > +   struct vtpm_dev *vtpm_dev = filp->private_data;
> > +
> > +   if (count > sizeof(vtpm_dev->buffer) ||
> > +       !(vtpm_dev->state & STATE_WAIT_RESPONSE_FLAG))
> > +      return -EIO;
> 
> This is not atomic in any sense. You should move this statement after
> taking buf_lock.


set_bit and friends were all atomic. Now we need to put all of these into 
the mutex... ?

   Stefan




[-- Attachment #1.2: Type: text/html, Size: 9271 bytes --]

[-- Attachment #2: Type: text/plain, Size: 79 bytes --]

------------------------------------------------------------------------------

[-- Attachment #3: Type: text/plain, Size: 192 bytes --]

_______________________________________________
tpmdd-devel mailing list
tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel