From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Stefan Berger" Subject: Re: [PATCH v4 08/10] tpm: Driver for supporting multiple emulated TPMs Date: Fri, 4 Mar 2016 16:16:41 -0500 Message-ID: <201603042116.u24LGlSP017830@d03av05.boulder.ibm.com> References: <1456766996-9300-1-git-send-email-stefanb@linux.vnet.ibm.com><1456766996-9300-9-git-send-email-stefanb@linux.vnet.ibm.com> <20160304202928.GA32617@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1893294898492107644==" Return-path: In-Reply-To: <20160304202928.GA32617-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: tpmdd-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org To: Jarkko Sakkinen Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: tpmdd-devel@lists.sourceforge.net --===============1893294898492107644== Content-Type: multipart/alternative; boundary="=_alternative 0074E51085257F6C_=" --=_alternative 0074E51085257F6C_= Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="US-ASCII" Jarkko Sakkinen wrote on 03/04/2016=20 03:29:28 PM: >=20 > Hi >=20 > I noticed some glitches. >=20 > On Mon, Feb 29, 2016 at 12:29:54PM -0500, Stefan Berger wrote: > > This patch implements a driver for supporting multiple emulated TPMs=20 in a > > system. > >=20 > > 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=20 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. >=20 > 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=5Fvtpm. fixed. >=20 >=20 > > Signed-off-by: Stefan Berger > > --- > > 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 > >=20 > > 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=5FCRB > > from within Linux. To compile this driver as a module, choose > > M here; the module will be called tpm=5Fcrb. > >=20 > > +config TCG=5FVTPM > > + tristate "VTPM Interface" > > + depends on TCG=5FTPM > > + ---help--- > > + This driver supports an emulated TPM (vTPM) running in=20 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=5FTPM > > 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=5FTCG=5FIBMVTPM) +=3D tpm=5Fibmvtpm.o > > obj-$(CONFIG=5FTCG=5FTIS=5FST33ZP24) +=3D st33zp24/ > > obj-$(CONFIG=5FTCG=5FXEN) +=3D xen-tpmfront.o > > obj-$(CONFIG=5FTCG=5FCRB) +=3D tpm=5Fcrb.o > > +obj-$(CONFIG=5FTCG=5FVTPM) +=3D 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 > > + * > > + * Maintained by: > > + * > > + * 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 > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include "tpm.h" > > + > > +#define VTPM=5FREQ=5FCOMPLETE=5FFLAG BIT(0) > > + > > +struct vtpm=5Fdev { > > + struct tpm=5Fchip *chip; > > + > > + u32 flags; /* public API flags */ > > + > > + wait=5Fqueue=5Fhead=5Ft wq; > > + > > + struct mutex buf=5Flock; /* protect buffer and flag=20 modifications */ > > + > > + long state; /* internal state */ > > +#define STATE=5FOPENED=5FFLAG BIT(0) > > +#define STATE=5FWAIT=5FRESPONSE=5FFLAG BIT(1) /* waiting for emulator= =20 > response */ > > + > > + size=5Ft req=5Flen; /* length of queued TPM request */ > > + size=5Ft resp=5Flen; /* length of queued TPM response */ > > + u8 buffer[TPM=5FBUFSIZE]; /* request/response buffer */ > > +}; > > + > > + > > +static void vtpm=5Fdelete=5Fdevice=5Fpair(struct vtpm=5Fdev *vtpm=5Fde= v); > > + > > +/* > > + * Functions related to 'server side' > > + */ > > + > > +/** > > + * vtpm=5Ffops=5Fread - Read TPM commands on 'server side' > > + * > > + * Return value: > > + * Number of bytes read or negative error code > > + */ > > +static ssize=5Ft vtpm=5Ffops=5Fread(struct file *filp, char =5F=5Fuser= *buf, > > + size=5Ft count, loff=5Ft *off) > > +{ > > + struct vtpm=5Fdev *vtpm=5Fdev =3D filp->private=5Fdata; > > + size=5Ft len; > > + int sig, rc; > > + > > + sig =3D wait=5Fevent=5Finterruptible(vtpm=5Fdev->wq, vtpm=5Fdev->re= q=5Flen !=3D=20 0); > > + if (sig) > > + return -EINTR; > > + > > + mutex=5Flock(&vtpm=5Fdev->buf=5Flock); > > + > > + len =3D vtpm=5Fdev->req=5Flen; > > + > > + if (count < len) { > > + mutex=5Funlock(&vtpm=5Fdev->buf=5Flock); > > + pr=5Fdebug("Invalid size in recv: count=3D%zd, req=5Flen=3D%zd\n= ", > > + count, len); > > + return -EIO; > > + } > > + > > + rc =3D copy=5Fto=5Fuser(buf, vtpm=5Fdev->buffer, len); > > + memset(vtpm=5Fdev->buffer, 0, len); > > + vtpm=5Fdev->req=5Flen =3D 0; > > + > > + if (!rc) > > + vtpm=5Fdev->state |=3D STATE=5FWAIT=5FRESPONSE=5FFLAG; > > + > > + mutex=5Funlock(&vtpm=5Fdev->buf=5Flock); > > + > > + if (rc) > > + return -EFAULT; > > + > > + return len; > > +} > > + > > +/** > > + * vtpm=5Ffops=5Fwrite - Write TPM responses on 'server side' > > + * > > + * Return value: > > + * Number of bytes read or negative error value > > + */ > > +static ssize=5Ft vtpm=5Ffops=5Fwrite(struct file *filp, const char =5F= =5Fuser=20 *buf, > > + size=5Ft count, loff=5Ft *off) > > +{ > > + struct vtpm=5Fdev *vtpm=5Fdev =3D filp->private=5Fdata; > > + > > + if (count > sizeof(vtpm=5Fdev->buffer) || > > + !(vtpm=5Fdev->state & STATE=5FWAIT=5FRESPONSE=5FFLAG)) > > + return -EIO; >=20 > This is not atomic in any sense. You should move this statement after > taking buf=5Flock. set=5Fbit and friends were all atomic. Now we need to put all of these into= =20 the mutex... ? Stefan --=_alternative 0074E51085257F6C_= Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset="US-ASCII" Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote on 03/04/2016 03:29:28 PM:
>
> Hi
>
> I not= iced some glitches.
>
> On Mon, Feb 29, 2016 at 12:29:54PM -05= 00, Stefan Berger wrote:
> > This patch implements a driver for su= pporting multiple emulated TPMs in a
> > system.
> >
> > The driver implem= ents a device /dev/vtpmx that is used to created
> > a client devi= ce pair /dev/tpmX (e.g., /dev/tpm10) and a server side that
> > is accessed using a file descriptor returned by an i= octl.
> > The device /dev/tpmX is the usual TPM device created by = the core TPM
> > driver. Applications or kernel subsystems can send TPM com= mands to it
> > and the corresponding server-side file descriptor receiv= es 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=5Fvtpm.

fix= ed.


>
>
> > S= igned-off-by: Stefan Berger <stefanb-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> >= ---
> >  drivers/char/tpm/Kconfig    |  10 +<= br>> >  drivers/char/tpm/Makefile   |   1 +
> &g= t;  drivers/char/tpm/tpm-vtpm.c | 546 +++++++++++++++++++++++++++++++<= br>> +++++++++++++
> >  include/uapi/linux/Kbuild   |=   1 +
> >  include/uapi/linux/vtpm.h   |  41 = ++++
> >  5 files changed, 599 insertions(+)
> > &nb= sp;create mode 100644 drivers/char/tpm/tpm-vtpm.c
> >  create= mode 100644 include/uapi/linux/vtpm.h
> >
> > diff --gi= t 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 @@ conf= ig TCG=5FCRB
> >       from within Linux.  To = compile this driver as a module, choose
> >       M here; the mo= dule will be called tpm=5Fcrb.
> >  
> > +config TCG= =5FVTPM
> > +   tristate "VTPM Interface"
> &= gt; +   depends on TCG=5FTPM
> > +   ---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 fil= e descriptor on which the vTPM
> > +     can receive commands.
>= ; > +
> > +
> >  source "drivers/char/tpm/st= 33zp24/Kconfig"
> >  endif # TCG=5FTPM
> > 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=5FTCG=5FIBMVTPM) +=3D tpm=5Fibmvtpm.o
> >  obj= -$(CONFIG=5FTCG=5FTIS=5FST33ZP24) +=3D st33zp24/
> >  obj-$(C= ONFIG=5FTCG=5FXEN) +=3D xen-tpmfront.o
> >  obj-$(CONFIG=5FTC= G=5FCRB) +=3D tpm=5Fcrb.o
> > +obj-$(CONFIG=5FTCG=5FVTPM) +=3D tpm= -vtpm.o
> > diff --git a/drivers/char/tpm/tpm-vtpm.c b/drivers/cha= r/tpm/tpm-vtpm.c
> > new file mode 100644
> > index 00000= 00..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>
> &= gt; + *
> > + * Device driver for vTPM.
> > + *
> &= gt; + * This program is free software; you can redistribute it and/or
&g= t; > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation, version 2 of= the
> > + * License.
> > + *
> > + */
> &= gt; +
> > +#include <linux/types.h>
> > +#include &= lt;linux/spinlock.h>
> > +#include <linux/uaccess.h>
&= gt; > +#include <linux/wait.h>
> > +#include <linux/mi= scdevice.h>
> > +#include <linux/vtpm.h>
> > +#i= nclude <linux/file.h>
> > +#include <linux/anon=5Finodes.= h>
> > +#include <linux/poll.h>
> > +#include &l= t;linux/compat.h>
> > +
> > +#include "tpm.h"= ;
> > +
> > +#define VTPM=5FREQ=5FCOMPLETE=5FFLAG  B= IT(0)
> > +
> > +struct vtpm=5Fdev {
> > +  = ; struct tpm=5Fchip *chip;
> > +
> > +   u32 flags; =                   /* public API flags */
> > +
> > + &= nbsp; wait=5Fqueue=5Fhead=5Ft wq;
> > +
> > +   stru= ct mutex buf=5Flock;       /* protect buffer and flag modifications */
> > +
> > +   long = state;                  /* internal state */
> > +#define STATE=5FOPEN= ED=5FFLAG        BIT(0)
> > +#define STATE=5FW= AIT=5FRESPONSE=5FFLAG BIT(1)  /* waiting for emulator
> response */
> > +
> > +   size=5Ft= req=5Flen;              /* length of queued TPM request */
> > +   size=5Ft res= p=5Flen;             /* length of queued TPM response */
> > +   u8 buffer[= TPM=5FBUFSIZE];      /* request/response buffer */
> > +};
> > +
> > +
> > +stat= ic void vtpm=5Fdelete=5Fdevice=5Fpair(struct vtpm=5Fdev *vtpm=5Fdev);
&g= t; > +
> > +/*
> > + * Functions related to 'server si= de'
> > + */
> > +
> > +/**
> > + * vtp= m=5Ffops=5Fread - Read TPM commands on 'server side'
> > + *
&g= t; > + * Return value:
> > + *   Number of bytes read or n= egative error code
> > + */
> > +static ssize=5Ft vtpm=5F= fops=5Fread(struct file *filp, char =5F=5Fuser *buf,
> > +               size= =5Ft count, loff=5Ft *off)
> > +{
> > +   struct vtpm=5Fdev *vtp= m=5Fdev =3D filp->private=5Fdata;
> > +   size=5Ft len;> > +   int sig, rc;
> > +
> > +   sig = =3D wait=5Fevent=5Finterruptible(vtpm=5Fdev->wq, vtpm=5Fdev->req=5Flen !=3D 0);
> > +   if (sig)
> > +      = return -EINTR;
> > +
> > +   mutex=5Flock(&vtpm= =5Fdev->buf=5Flock);
> > +
> > +   len =3D vtpm= =5Fdev->req=5Flen;
> > +
> > +   if (count < l= en) {
> > +      mutex=5Funlock(&vtpm=5Fdev->= ;buf=5Flock);
> > +      pr=5Fdebug("Invalid s= ize in recv: count=3D%zd, req=5Flen=3D%zd\n",
> > +          c= ount, len);
> > +      return -EIO;
> > + =   }
> > +
> > +   rc =3D copy=5Fto=5Fuser(buf, = vtpm=5Fdev->buffer, len);
> > +   memset(vtpm=5Fdev->bu= ffer, 0, len);
> > +   vtpm=5Fdev->req=5Flen =3D 0;
>= ; > +
> > +   if (!rc)
> > +      = vtpm=5Fdev->state |=3D STATE=5FWAIT=5FRESPONSE=5FFLAG;
> > +> > +   mutex=5Funlock(&vtpm=5Fdev->buf=5Flock);
>= > +
> > +   if (rc)
> > +      re= turn -EFAULT;
> > +
> > +   return len;
> >= +}
> > +
> > +/**
> > + * vtpm=5Ffops=5Fwrite -= Write TPM responses on 'server side'
> > + *
> > + * Ret= urn value:
> > + *   Number of bytes read or negative error v= alue
> > + */
> > +static ssize=5Ft vtpm=5Ffops=5Fwrite(s= truct file *filp, const char =5F=5Fuser *buf,
> > +             &= nbsp;  size=5Ft count, loff=5Ft *off)
> > +{
> > +   struct vtpm=5Fd= ev *vtpm=5Fdev =3D filp->private=5Fdata;
> > +
> > + &= nbsp; if (count > sizeof(vtpm=5Fdev->buffer) ||
> > +  =     !(vtpm=5Fdev->state & STATE=5FWAIT=5FRESPONSE=5FFLAG)= )
> > +      return -EIO;
>
> This is = not atomic in any sense. You should move this statement after
> takin= g buf=5Flock.


set=5Fbit and frien= ds were all atomic. Now we need to put all of these into the mutex... ?

=    Stefan


--=_alternative 0074E51085257F6C_=-- --===============1893294898492107644== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline ------------------------------------------------------------------------------ --===============1893294898492107644== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ tpmdd-devel mailing list tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org https://lists.sourceforge.net/lists/listinfo/tpmdd-devel --===============1893294898492107644==--