[PATCH v2 0/3] Extend the vTPM proxy driver to pass locality to emulator

[PATCH v2 0/3] Extend the vTPM proxy driver to pass locality to emulator

[Stefan Berger]

The purpose of this series of patches is to enable the passing of the locality
a command is executing in to a TPM emulator. To enable this we introduce a new
flag for the device creation ioctl that requests that the locality be prepended
to every command. For applications to check which flags the driver supports, we
add a new ioctl that returns a bitmask of supported flags.

v1->v2:
  - fixed return value from function in patch 3/3


Stefan Berger (3):
  tpm: vtpm_proxy: Add ioctl to get supported flags
  tpm: vtpm_proxy: Implement request_locality
  tpm: vtpm_proxy: Add ioctl to request locality prepended to command

 drivers/char/tpm/tpm_vtpm_proxy.c | 53 +++++++++++++++++++++++++++++++++++----
 include/uapi/linux/vtpm_proxy.h   | 15 ++++++++++-
 2 files changed, 62 insertions(+), 6 deletions(-)

-- 
2.4.3

[PATCH v2 1/3] tpm: vtpm_proxy: Add ioctl to get supported flags

[Stefan Berger]

Add an ioctl to get the supported flags.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 drivers/char/tpm/tpm_vtpm_proxy.c | 29 +++++++++++++++++++++++++++++
 include/uapi/linux/vtpm_proxy.h   | 11 +++++++++++
 2 files changed, 40 insertions(+)

diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index 751059d..fb4d207 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -592,6 +592,33 @@ static long vtpmx_ioc_new_dev(struct file *file, unsigned int ioctl,
 	return 0;
 }
 
+/**
+ * vtpmx_ioc_get_supt_flags - handler for the %VTPM_PROXY_IOC_GET_SUPT_FLAGS
+ *                            ioctl
+ * @file:	/dev/vtpmx
+ * @ioctl:	the ioctl number
+ * @arg:	pointer to the struct vtpmx_proxy_get_supt_flags
+ *
+ * Return the bitfield of supported flags
+ */
+static long vtpmx_ioc_get_supt_flags(struct file *file, unsigned int ioctl,
+				     unsigned long arg)
+{
+	void __user *argp = (void __user *)arg;
+	struct vtpm_proxy_supt_flags __user *vtpm_supt_flags_p = argp;
+	struct vtpm_proxy_supt_flags flags = {
+		.flags = VTPM_PROXY_FLAGS_ALL,
+	};
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	if (copy_to_user(vtpm_supt_flags_p, &flags, sizeof(flags)))
+		return -EFAULT;
+
+	return 0;
+}
+
 /*
  * vtpmx_fops_ioctl: ioctl on /dev/vtpmx
  *
@@ -604,6 +631,8 @@ static long vtpmx_fops_ioctl(struct file *f, unsigned int ioctl,
 	switch (ioctl) {
 	case VTPM_PROXY_IOC_NEW_DEV:
 		return vtpmx_ioc_new_dev(f, ioctl, arg);
+	case VTPM_PROXY_IOC_GET_SUPT_FLAGS:
+		return vtpmx_ioc_get_supt_flags(f, ioctl, arg);
 	default:
 		return -ENOIOCTLCMD;
 	}
diff --git a/include/uapi/linux/vtpm_proxy.h b/include/uapi/linux/vtpm_proxy.h
index a69e991..83e64e7 100644
--- a/include/uapi/linux/vtpm_proxy.h
+++ b/include/uapi/linux/vtpm_proxy.h
@@ -44,6 +44,17 @@ struct vtpm_proxy_new_dev {
 	__u32 minor;         /* output */
 };
 
+/**
+ * struct vtpm_proxy_supt_flags - parameter structure for the
+ *                                %VTPM_PROXY_IOC_GET_SUPT_FLAGS ioctl
+ * @flags: flags supported by the vtpm proxy driver
+ */
+struct vtpm_proxy_supt_flags {
+	__u32 flags;         /* output */
+};
+
 #define VTPM_PROXY_IOC_NEW_DEV	_IOWR(0xa1, 0x00, struct vtpm_proxy_new_dev)
+#define VTPM_PROXY_IOC_GET_SUPT_FLAGS \
+				_IOR(0xa1, 0x01, struct vtpm_proxy_supt_flags)
 
 #endif /* _UAPI_LINUX_VTPM_PROXY_H */
-- 
2.4.3

[PATCH v2 2/3] tpm: vtpm_proxy: Implement request_locality

[Stefan Berger]

Implement the request_locality function. Accept all localities assuming
that the emulator handling the localities will check for a valid locality.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 drivers/char/tpm/tpm_vtpm_proxy.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index fb4d207..48b9818 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -371,6 +371,11 @@ static bool vtpm_proxy_tpm_req_canceled(struct tpm_chip  *chip, u8 status)
 	return ret;
 }
 
+static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
+{
+	return locality;
+}
+
 static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
 	.flags = TPM_OPS_AUTO_STARTUP,
 	.recv = vtpm_proxy_tpm_op_recv,
@@ -380,6 +385,7 @@ static const struct tpm_class_ops vtpm_proxy_tpm_ops = {
 	.req_complete_mask = VTPM_PROXY_REQ_COMPLETE_FLAG,
 	.req_complete_val = VTPM_PROXY_REQ_COMPLETE_FLAG,
 	.req_canceled = vtpm_proxy_tpm_req_canceled,
+	.request_locality = vtpm_proxy_request_locality,
 };
 
 /*
-- 
2.4.3

[PATCH v2 3/3] tpm: vtpm_proxy: Add ioctl to request locality prepended to command

[Stefan Berger]

Add an ioctl to request that the locality be prepended to every TPM
command.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 drivers/char/tpm/tpm_vtpm_proxy.c | 18 +++++++++++++-----
 include/uapi/linux/vtpm_proxy.h   |  4 +++-
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index 48b9818..6c90e02 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -52,7 +52,8 @@ struct proxy_dev {
 };
 
 /* all supported flags */
-#define VTPM_PROXY_FLAGS_ALL  (VTPM_PROXY_FLAG_TPM2)
+#define VTPM_PROXY_FLAGS_ALL  (VTPM_PROXY_FLAG_TPM2 | \
+			       VTPM_PROXY_FLAG_PREPEND_LOCALITY)
 
 static struct workqueue_struct *workqueue;
 
@@ -77,8 +78,9 @@ static ssize_t vtpm_proxy_fops_read(struct file *filp, char __user *buf,
 				    size_t count, loff_t *off)
 {
 	struct proxy_dev *proxy_dev = filp->private_data;
-	size_t len;
-	int sig, rc;
+	size_t len, offset = 0;
+	int sig, rc = 0;
+	uint8_t locality;
 
 	sig = wait_event_interruptible(proxy_dev->wq,
 		proxy_dev->req_len != 0 ||
@@ -102,7 +104,13 @@ static ssize_t vtpm_proxy_fops_read(struct file *filp, char __user *buf,
 		return -EIO;
 	}
 
-	rc = copy_to_user(buf, proxy_dev->buffer, len);
+	if (proxy_dev->flags & VTPM_PROXY_FLAG_PREPEND_LOCALITY) {
+		locality = proxy_dev->chip->locality;
+		offset = sizeof(locality);
+		rc = copy_to_user(buf, &locality, offset);
+	}
+	if (!rc)
+		rc = copy_to_user(&buf[offset], proxy_dev->buffer, len);
 	memset(proxy_dev->buffer, 0, len);
 	proxy_dev->req_len = 0;
 
@@ -114,7 +122,7 @@ static ssize_t vtpm_proxy_fops_read(struct file *filp, char __user *buf,
 	if (rc)
 		return -EFAULT;
 
-	return len;
+	return offset + len;
 }
 
 /**
diff --git a/include/uapi/linux/vtpm_proxy.h b/include/uapi/linux/vtpm_proxy.h
index 83e64e7..512a29e 100644
--- a/include/uapi/linux/vtpm_proxy.h
+++ b/include/uapi/linux/vtpm_proxy.h
@@ -22,9 +22,11 @@
 /**
  * enum vtpm_proxy_flags - flags for the proxy TPM
  * @VTPM_PROXY_FLAG_TPM2:	the proxy TPM uses TPM 2.0 protocol
+ * @VTPM_PROXY_PREPEND_LOCALITY:locality byte prepended on each command
  */
 enum vtpm_proxy_flags {
-	VTPM_PROXY_FLAG_TPM2	= 1,
+	VTPM_PROXY_FLAG_TPM2			= 1,
+	VTPM_PROXY_FLAG_PREPEND_LOCALITY	= 2,
 };
 
 /**
-- 
2.4.3

Re: [PATCH v2 3/3] tpm: vtpm_proxy: Add ioctl to request locality prepended to command

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 10598 bytes --]

Hi Stefan,

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on v4.11-rc8 next-20170428]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Stefan-Berger/Extend-the-vTPM-proxy-driver-to-pass-locality-to-emulator/20170429-115352
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

   include/linux/init.h:1: warning: no structured comments found
   kernel/sched/core.c:2085: warning: No description found for parameter 'rf'
   kernel/sched/core.c:2085: warning: Excess function parameter 'cookie' description in 'try_to_wake_up_local'
   include/linux/kthread.h:26: warning: Excess function parameter '...' description in 'kthread_create'
   kernel/sys.c:1: warning: no structured comments found
   include/linux/device.h:969: warning: No description found for parameter 'dma_ops'
   drivers/dma-buf/seqno-fence.c:1: warning: no structured comments found
   include/linux/iio/iio.h:597: warning: No description found for parameter 'trig_readonly'
   include/linux/iio/trigger.h:151: warning: No description found for parameter 'indio_dev'
   include/linux/iio/trigger.h:151: warning: No description found for parameter 'trig'
   include/linux/device.h:970: warning: No description found for parameter 'dma_ops'
   drivers/regulator/core.c:1467: warning: Excess function parameter 'ret' description in 'regulator_dev_lookup'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'open'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'preclose'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'postclose'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'lastclose'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'set_busid'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'irq_handler'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'irq_preinstall'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'irq_postinstall'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'irq_uninstall'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'debugfs_init'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'debugfs_cleanup'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_open_object'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_close_object'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'prime_handle_to_fd'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'prime_fd_to_handle'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_export'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_import'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_pin'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_unpin'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_res_obj'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_get_sg_table'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_import_sg_table'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_vmap'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_vunmap'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_prime_mmap'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'gem_vm_ops'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'major'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'minor'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'patchlevel'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'name'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'desc'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'date'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'driver_features'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'ioctls'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'num_ioctls'
   include/drm/drm_drv.h:438: warning: No description found for parameter 'fops'
   include/drm/drm_color_mgmt.h:1: warning: no structured comments found
   drivers/gpu/drm/drm_fb_cma_helper.c:557: warning: Excess function parameter 'num_crtc' description in 'drm_fbdev_cma_init'
   drivers/gpu/drm/drm_fb_cma_helper.c:558: warning: Excess function parameter 'num_crtc' description in 'drm_fbdev_cma_init'
   drivers/gpu/drm/i915/intel_lpe_audio.c:342: warning: No description found for parameter 'pipe'
   drivers/gpu/drm/i915/intel_lpe_audio.c:342: warning: No description found for parameter 'dp_output'
   drivers/gpu/drm/i915/intel_lpe_audio.c:342: warning: No description found for parameter 'link_rate'
   drivers/gpu/drm/i915/intel_lpe_audio.c:343: warning: No description found for parameter 'pipe'
   drivers/gpu/drm/i915/intel_lpe_audio.c:343: warning: No description found for parameter 'dp_output'
   drivers/gpu/drm/i915/intel_lpe_audio.c:343: warning: No description found for parameter 'link_rate'
   drivers/media/dvb-core/dvb_frontend.h:677: warning: No description found for parameter 'refcount'
>> include/uapi/linux/vtpm_proxy.h:30: warning: Enum value 'VTPM_PROXY_FLAG_PREPEND_LOCALITY' not described in enum 'vtpm_proxy_flags'
   Documentation/core-api/assoc_array.rst:13: WARNING: Enumerated list ends without a blank line; unexpected unindent.
   Documentation/doc-guide/sphinx.rst:110: ERROR: Unknown target name: "sphinx c domain".
   kernel/sched/fair.c:7616: WARNING: Inline emphasis start-string without end-string.
   kernel/time/timer.c:1200: ERROR: Unexpected indentation.
   kernel/time/timer.c:1202: ERROR: Unexpected indentation.
   kernel/time/timer.c:1203: WARNING: Block quote ends without a blank line; unexpected unindent.
   include/linux/wait.h:122: WARNING: Block quote ends without a blank line; unexpected unindent.
   include/linux/wait.h:125: ERROR: Unexpected indentation.
   include/linux/wait.h:127: WARNING: Block quote ends without a blank line; unexpected unindent.
   kernel/time/hrtimer.c:990: WARNING: Block quote ends without a blank line; unexpected unindent.
   kernel/signal.c:322: WARNING: Inline literal start-string without end-string.
   include/linux/iio/iio.h:219: ERROR: Unexpected indentation.
   include/linux/iio/iio.h:220: WARNING: Block quote ends without a blank line; unexpected unindent.
   include/linux/iio/iio.h:226: WARNING: Definition list ends without a blank line; unexpected unindent.
   drivers/iio/industrialio-core.c:639: ERROR: Unknown target name: "iio_val".
   drivers/iio/industrialio-core.c:646: ERROR: Unknown target name: "iio_val".
   drivers/message/fusion/mptbase.c:5051: WARNING: Definition list ends without a blank line; unexpected unindent.
   drivers/tty/serial/serial_core.c:1898: WARNING: Definition list ends without a blank line; unexpected unindent.
   include/linux/regulator/driver.h:271: ERROR: Unknown target name: "regulator_regmap_x_voltage".
   include/linux/spi/spi.h:369: ERROR: Unexpected indentation.
   drivers/usb/core/message.c:478: ERROR: Unexpected indentation.
   drivers/usb/core/message.c:479: WARNING: Block quote ends without a blank line; unexpected unindent.
   Documentation/driver-api/usb.rst:623: ERROR: Unknown target name: "usb_type".
   Documentation/driver-api/usb.rst:623: ERROR: Unknown target name: "usb_dir".
   Documentation/driver-api/usb.rst:623: ERROR: Unknown target name: "usb_recip".
   Documentation/driver-api/usb.rst:689: ERROR: Unknown target name: "usbdevfs_urb_type".
   sound/soc/soc-core.c:2670: ERROR: Unknown target name: "snd_soc_daifmt".
   sound/core/jack.c:312: ERROR: Unknown target name: "snd_jack_btn".
   WARNING: dvipng command 'dvipng' cannot be run (needed for math display), check the imgmath_dvipng setting

vim +30 include/uapi/linux/vtpm_proxy.h

6f99612e Stefan Berger   2016-04-18  14   */
6f99612e Stefan Berger   2016-04-18  15  
6f99612e Stefan Berger   2016-04-18  16  #ifndef _UAPI_LINUX_VTPM_PROXY_H
6f99612e Stefan Berger   2016-04-18  17  #define _UAPI_LINUX_VTPM_PROXY_H
6f99612e Stefan Berger   2016-04-18  18  
6f99612e Stefan Berger   2016-04-18  19  #include <linux/types.h>
6f99612e Stefan Berger   2016-04-18  20  #include <linux/ioctl.h>
6f99612e Stefan Berger   2016-04-18  21  
7ea7861c Jarkko Sakkinen 2016-11-03  22  /**
7ea7861c Jarkko Sakkinen 2016-11-03  23   * enum vtpm_proxy_flags - flags for the proxy TPM
7ea7861c Jarkko Sakkinen 2016-11-03  24   * @VTPM_PROXY_FLAG_TPM2:	the proxy TPM uses TPM 2.0 protocol
222e0c7c Stefan Berger   2017-04-28  25   * @VTPM_PROXY_PREPEND_LOCALITY:locality byte prepended on each command
7ea7861c Jarkko Sakkinen 2016-11-03  26   */
7ea7861c Jarkko Sakkinen 2016-11-03  27  enum vtpm_proxy_flags {
7ea7861c Jarkko Sakkinen 2016-11-03  28  	VTPM_PROXY_FLAG_TPM2			= 1,
222e0c7c Stefan Berger   2017-04-28  29  	VTPM_PROXY_FLAG_PREPEND_LOCALITY	= 2,
7ea7861c Jarkko Sakkinen 2016-11-03 @30  };
6f99612e Stefan Berger   2016-04-18  31  
7ea7861c Jarkko Sakkinen 2016-11-03  32  /**
7ea7861c Jarkko Sakkinen 2016-11-03  33   * struct vtpm_proxy_new_dev - parameter structure for the
7ea7861c Jarkko Sakkinen 2016-11-03  34   *                             %VTPM_PROXY_IOC_NEW_DEV ioctl
7ea7861c Jarkko Sakkinen 2016-11-03  35   * @flags:	flags for the proxy TPM
7ea7861c Jarkko Sakkinen 2016-11-03  36   * @tpm_num:	index of the TPM device
7ea7861c Jarkko Sakkinen 2016-11-03  37   * @fd:		the file descriptor used by the proxy TPM
7ea7861c Jarkko Sakkinen 2016-11-03  38   * @major:	the major number of the TPM device

:::::: The code at line 30 was first introduced by commit
:::::: 7ea7861c8c2462af932410c54542cb279683eaf8 tpm, tpm_vtpm_proxy: add kdoc comments for VTPM_PROXY_IOC_NEW_DEV

:::::: TO: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
:::::: CC: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6576 bytes --]

Re: [PATCH v2 0/3] Extend the vTPM proxy driver to pass locality to emulator

[Jarkko Sakkinen]

I will get into this with detail after 4.12-rc1.

/Jarkko

On Fri, Apr 28, 2017 at 09:02:15AM -0400, Stefan Berger wrote:
> The purpose of this series of patches is to enable the passing of the locality
> a command is executing in to a TPM emulator. To enable this we introduce a new
> flag for the device creation ioctl that requests that the locality be prepended
> to every command. For applications to check which flags the driver supports, we
> add a new ioctl that returns a bitmask of supported flags.
> 
> v1->v2:
>   - fixed return value from function in patch 3/3
> 
> 
> Stefan Berger (3):
>   tpm: vtpm_proxy: Add ioctl to get supported flags
>   tpm: vtpm_proxy: Implement request_locality
>   tpm: vtpm_proxy: Add ioctl to request locality prepended to command
> 
>  drivers/char/tpm/tpm_vtpm_proxy.c | 53 +++++++++++++++++++++++++++++++++++----
>  include/uapi/linux/vtpm_proxy.h   | 15 ++++++++++-
>  2 files changed, 62 insertions(+), 6 deletions(-)
> 
> -- 
> 2.4.3
> 

Re: [PATCH v2 1/3] tpm: vtpm_proxy: Add ioctl to get supported flags

[Jarkko Sakkinen]

On Fri, Apr 28, 2017 at 09:02:16AM -0400, Stefan Berger wrote:
> Add an ioctl to get the supported flags.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>

The commit message is a bit lacking on explaining things like why we
need it.

> ---
>  drivers/char/tpm/tpm_vtpm_proxy.c | 29 +++++++++++++++++++++++++++++
>  include/uapi/linux/vtpm_proxy.h   | 11 +++++++++++
>  2 files changed, 40 insertions(+)
> 
> diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
> index 751059d..fb4d207 100644
> --- a/drivers/char/tpm/tpm_vtpm_proxy.c
> +++ b/drivers/char/tpm/tpm_vtpm_proxy.c
> @@ -592,6 +592,33 @@ static long vtpmx_ioc_new_dev(struct file *file, unsigned int ioctl,
>  	return 0;
>  }
>  
> +/**
> + * vtpmx_ioc_get_supt_flags - handler for the %VTPM_PROXY_IOC_GET_SUPT_FLAGS
> + *                            ioctl
> + * @file:	/dev/vtpmx
> + * @ioctl:	the ioctl number
> + * @arg:	pointer to the struct vtpmx_proxy_get_supt_flags
> + *
> + * Return the bitfield of supported flags
> + */
> +static long vtpmx_ioc_get_supt_flags(struct file *file, unsigned int ioctl,
> +				     unsigned long arg)
> +{
> +	void __user *argp = (void __user *)arg;
> +	struct vtpm_proxy_supt_flags __user *vtpm_supt_flags_p = argp;
> +	struct vtpm_proxy_supt_flags flags = {
> +		.flags = VTPM_PROXY_FLAGS_ALL,
> +	};
> +
> +	if (!capable(CAP_SYS_ADMIN))
> +		return -EPERM;
> +
> +	if (copy_to_user(vtpm_supt_flags_p, &flags, sizeof(flags)))
> +		return -EFAULT;
> +
> +	return 0;
> +}
> +
>  /*
>   * vtpmx_fops_ioctl: ioctl on /dev/vtpmx
>   *
> @@ -604,6 +631,8 @@ static long vtpmx_fops_ioctl(struct file *f, unsigned int ioctl,
>  	switch (ioctl) {
>  	case VTPM_PROXY_IOC_NEW_DEV:
>  		return vtpmx_ioc_new_dev(f, ioctl, arg);
> +	case VTPM_PROXY_IOC_GET_SUPT_FLAGS:
> +		return vtpmx_ioc_get_supt_flags(f, ioctl, arg);
>  	default:
>  		return -ENOIOCTLCMD;
>  	}
> diff --git a/include/uapi/linux/vtpm_proxy.h b/include/uapi/linux/vtpm_proxy.h
> index a69e991..83e64e7 100644
> --- a/include/uapi/linux/vtpm_proxy.h
> +++ b/include/uapi/linux/vtpm_proxy.h
> @@ -44,6 +44,17 @@ struct vtpm_proxy_new_dev {
>  	__u32 minor;         /* output */
>  };
>  
> +/**
> + * struct vtpm_proxy_supt_flags - parameter structure for the
> + *                                %VTPM_PROXY_IOC_GET_SUPT_FLAGS ioctl
> + * @flags: flags supported by the vtpm proxy driver
> + */
> +struct vtpm_proxy_supt_flags {
> +	__u32 flags;         /* output */
> +};
> +
>  #define VTPM_PROXY_IOC_NEW_DEV	_IOWR(0xa1, 0x00, struct vtpm_proxy_new_dev)
> +#define VTPM_PROXY_IOC_GET_SUPT_FLAGS \
> +				_IOR(0xa1, 0x01, struct vtpm_proxy_supt_flags)
>  
>  #endif /* _UAPI_LINUX_VTPM_PROXY_H */
> -- 
> 2.4.3
> 

/Jarkko

Re: [PATCH v2 3/3] tpm: vtpm_proxy: Add ioctl to request locality prepended to command

[Jarkko Sakkinen]

On Fri, Apr 28, 2017 at 09:02:18AM -0400, Stefan Berger wrote:
> Add an ioctl to request that the locality be prepended to every TPM
> command.

Don't really understand this change. Why locality is prenpended?
Where is the ioctl declaration in this commit?

> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
>  drivers/char/tpm/tpm_vtpm_proxy.c | 18 +++++++++++++-----
>  include/uapi/linux/vtpm_proxy.h   |  4 +++-
>  2 files changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
> index 48b9818..6c90e02 100644
> --- a/drivers/char/tpm/tpm_vtpm_proxy.c
> +++ b/drivers/char/tpm/tpm_vtpm_proxy.c
> @@ -52,7 +52,8 @@ struct proxy_dev {
>  };
>  
>  /* all supported flags */
> -#define VTPM_PROXY_FLAGS_ALL  (VTPM_PROXY_FLAG_TPM2)
> +#define VTPM_PROXY_FLAGS_ALL  (VTPM_PROXY_FLAG_TPM2 | \
> +			       VTPM_PROXY_FLAG_PREPEND_LOCALITY)
>  
>  static struct workqueue_struct *workqueue;
>  
> @@ -77,8 +78,9 @@ static ssize_t vtpm_proxy_fops_read(struct file *filp, char __user *buf,
>  				    size_t count, loff_t *off)
>  {
>  	struct proxy_dev *proxy_dev = filp->private_data;
> -	size_t len;
> -	int sig, rc;
> +	size_t len, offset = 0;
> +	int sig, rc = 0;

One line per declaration:

size_t len;
size_t offset = 0;
int sig;
int rc = 0;

> +	uint8_t locality;
>  
>  	sig = wait_event_interruptible(proxy_dev->wq,
>  		proxy_dev->req_len != 0 ||
> @@ -102,7 +104,13 @@ static ssize_t vtpm_proxy_fops_read(struct file *filp, char __user *buf,
>  		return -EIO;
>  	}
>  
> -	rc = copy_to_user(buf, proxy_dev->buffer, len);
> +	if (proxy_dev->flags & VTPM_PROXY_FLAG_PREPEND_LOCALITY) {
> +		locality = proxy_dev->chip->locality;
> +		offset = sizeof(locality);
> +		rc = copy_to_user(buf, &locality, offset);
> +	}
> +	if (!rc)
> +		rc = copy_to_user(&buf[offset], proxy_dev->buffer, len);
>  	memset(proxy_dev->buffer, 0, len);
>  	proxy_dev->req_len = 0;
>  
> @@ -114,7 +122,7 @@ static ssize_t vtpm_proxy_fops_read(struct file *filp, char __user *buf,
>  	if (rc)
>  		return -EFAULT;
>  
> -	return len;
> +	return offset + len;
>  }
>  
>  /**
> diff --git a/include/uapi/linux/vtpm_proxy.h b/include/uapi/linux/vtpm_proxy.h
> index 83e64e7..512a29e 100644
> --- a/include/uapi/linux/vtpm_proxy.h
> +++ b/include/uapi/linux/vtpm_proxy.h
> @@ -22,9 +22,11 @@
>  /**
>   * enum vtpm_proxy_flags - flags for the proxy TPM
>   * @VTPM_PROXY_FLAG_TPM2:	the proxy TPM uses TPM 2.0 protocol
> + * @VTPM_PROXY_PREPEND_LOCALITY:locality byte prepended on each command
>   */
>  enum vtpm_proxy_flags {
> -	VTPM_PROXY_FLAG_TPM2	= 1,
> +	VTPM_PROXY_FLAG_TPM2			= 1,
> +	VTPM_PROXY_FLAG_PREPEND_LOCALITY	= 2,
>  };
>  
>  /**
> -- 
> 2.4.3
> 

/Jarkko

Re: [PATCH v2 0/3] Extend the vTPM proxy driver to pass locality to emulator

[Jarkko Sakkinen]

On Fri, Apr 28, 2017 at 09:02:15AM -0400, Stefan Berger wrote:
> The purpose of this series of patches is to enable the passing of the locality
> a command is executing in to a TPM emulator. To enable this we introduce a new
> flag for the device creation ioctl that requests that the locality be prepended
> to every command. For applications to check which flags the driver supports, we
> add a new ioctl that returns a bitmask of supported flags.

This is a weird change proposal as you could use tpm_vtpm_proxy for
other than some TPM emulator.

> 
> v1->v2:
>   - fixed return value from function in patch 3/3
> 
> 
> Stefan Berger (3):
>   tpm: vtpm_proxy: Add ioctl to get supported flags
>   tpm: vtpm_proxy: Implement request_locality
>   tpm: vtpm_proxy: Add ioctl to request locality prepended to command
> 
>  drivers/char/tpm/tpm_vtpm_proxy.c | 53 +++++++++++++++++++++++++++++++++++----
>  include/uapi/linux/vtpm_proxy.h   | 15 ++++++++++-
>  2 files changed, 62 insertions(+), 6 deletions(-)
> 
> -- 
> 2.4.3
> 

/Jarkko

Re: [PATCH v2 3/3] tpm: vtpm_proxy: Add ioctl to request locality prepended to command

[Stefan Berger]

On 05/03/2017 06:37 PM, Jarkko Sakkinen wrote:
> On Fri, Apr 28, 2017 at 09:02:18AM -0400, Stefan Berger wrote:
>> Add an ioctl to request that the locality be prepended to every TPM
>> command.
> Don't really understand this change. Why locality is prenpended?

Commands can be executed under locality 0-3 and for some commands it is 
important to know which locality a user may have chosen. How else should 
we convey that locality to the TPM emulator ?

> Where is the ioctl declaration in this commit?

My bad, it's a flag that's being added here.

>
>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>> ---
>>   drivers/char/tpm/tpm_vtpm_proxy.c | 18 +++++++++++++-----
>>   include/uapi/linux/vtpm_proxy.h   |  4 +++-
>>   2 files changed, 16 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
>> index 48b9818..6c90e02 100644
>> --- a/drivers/char/tpm/tpm_vtpm_proxy.c
>> +++ b/drivers/char/tpm/tpm_vtpm_proxy.c
>> @@ -52,7 +52,8 @@ struct proxy_dev {
>>   };
>>   
>>   /* all supported flags */
>> -#define VTPM_PROXY_FLAGS_ALL  (VTPM_PROXY_FLAG_TPM2)
>> +#define VTPM_PROXY_FLAGS_ALL  (VTPM_PROXY_FLAG_TPM2 | \
>> +			       VTPM_PROXY_FLAG_PREPEND_LOCALITY)
>>   
>>   static struct workqueue_struct *workqueue;
>>   
>> @@ -77,8 +78,9 @@ static ssize_t vtpm_proxy_fops_read(struct file *filp, char __user *buf,
>>   				    size_t count, loff_t *off)
>>   {
>>   	struct proxy_dev *proxy_dev = filp->private_data;
>> -	size_t len;
>> -	int sig, rc;
>> +	size_t len, offset = 0;
>> +	int sig, rc = 0;
> One line per declaration:
>
> size_t len;
> size_t offset = 0;
> int sig;
> int rc = 0;
>
>> +	uint8_t locality;
>>   
>>   	sig = wait_event_interruptible(proxy_dev->wq,
>>   		proxy_dev->req_len != 0 ||
>> @@ -102,7 +104,13 @@ static ssize_t vtpm_proxy_fops_read(struct file *filp, char __user *buf,
>>   		return -EIO;
>>   	}
>>   
>> -	rc = copy_to_user(buf, proxy_dev->buffer, len);
>> +	if (proxy_dev->flags & VTPM_PROXY_FLAG_PREPEND_LOCALITY) {
>> +		locality = proxy_dev->chip->locality;
>> +		offset = sizeof(locality);
>> +		rc = copy_to_user(buf, &locality, offset);
>> +	}
>> +	if (!rc)
>> +		rc = copy_to_user(&buf[offset], proxy_dev->buffer, len);
>>   	memset(proxy_dev->buffer, 0, len);
>>   	proxy_dev->req_len = 0;
>>   
>> @@ -114,7 +122,7 @@ static ssize_t vtpm_proxy_fops_read(struct file *filp, char __user *buf,
>>   	if (rc)
>>   		return -EFAULT;
>>   
>> -	return len;
>> +	return offset + len;
>>   }
>>   
>>   /**
>> diff --git a/include/uapi/linux/vtpm_proxy.h b/include/uapi/linux/vtpm_proxy.h
>> index 83e64e7..512a29e 100644
>> --- a/include/uapi/linux/vtpm_proxy.h
>> +++ b/include/uapi/linux/vtpm_proxy.h
>> @@ -22,9 +22,11 @@
>>   /**
>>    * enum vtpm_proxy_flags - flags for the proxy TPM
>>    * @VTPM_PROXY_FLAG_TPM2:	the proxy TPM uses TPM 2.0 protocol
>> + * @VTPM_PROXY_PREPEND_LOCALITY:locality byte prepended on each command
>>    */
>>   enum vtpm_proxy_flags {
>> -	VTPM_PROXY_FLAG_TPM2	= 1,
>> +	VTPM_PROXY_FLAG_TPM2			= 1,
>> +	VTPM_PROXY_FLAG_PREPEND_LOCALITY	= 2,
>>   };
>>   
>>   /**
>> -- 
>> 2.4.3
>>
> /Jarkko
>

Re: [PATCH v2 0/3] Extend the vTPM proxy driver to pass locality to emulator

[Stefan Berger]

On 05/03/2017 06:38 PM, Jarkko Sakkinen wrote:
> On Fri, Apr 28, 2017 at 09:02:15AM -0400, Stefan Berger wrote:
>> The purpose of this series of patches is to enable the passing of the locality
>> a command is executing in to a TPM emulator. To enable this we introduce a new
>> flag for the device creation ioctl that requests that the locality be prepended
>> to every command. For applications to check which flags the driver supports, we
>> add a new ioctl that returns a bitmask of supported flags.
> This is a weird change proposal as you could use tpm_vtpm_proxy for
> other than some TPM emulator.

I think in most cases the recipient of the TPM commands from the 
vtpm_proxy driver will be a TPM emulator. What do you have in mind?

>
>> v1->v2:
>>    - fixed return value from function in patch 3/3
>>
>>
>> Stefan Berger (3):
>>    tpm: vtpm_proxy: Add ioctl to get supported flags
>>    tpm: vtpm_proxy: Implement request_locality
>>    tpm: vtpm_proxy: Add ioctl to request locality prepended to command
>>
>>   drivers/char/tpm/tpm_vtpm_proxy.c | 53 +++++++++++++++++++++++++++++++++++----
>>   include/uapi/linux/vtpm_proxy.h   | 15 ++++++++++-
>>   2 files changed, 62 insertions(+), 6 deletions(-)
>>
>> -- 
>> 2.4.3
>>
> /Jarkko
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH v2 3/3] tpm: vtpm_proxy: Add ioctl to request locality prepended to command

[Jarkko Sakkinen]

On Wed, May 03, 2017 at 07:40:48PM -0400, Stefan Berger wrote:
> On 05/03/2017 06:37 PM, Jarkko Sakkinen wrote:
> > On Fri, Apr 28, 2017 at 09:02:18AM -0400, Stefan Berger wrote:
> > > Add an ioctl to request that the locality be prepended to every TPM
> > > command.
> > Don't really understand this change. Why locality is prenpended?
> 
> Commands can be executed under locality 0-3 and for some commands it is
> important to know which locality a user may have chosen. How else should we
> convey that locality to the TPM emulator ?

Why this is not in the commit message?

More scalable way to do this would be to have a set of vtpm proxy
commands. There could be a command for requesting and releasing
locality. That would be more clean.

/Jarkko

Re: [PATCH v2 0/3] Extend the vTPM proxy driver to pass locality to emulator

[Jarkko Sakkinen]

On Wed, May 03, 2017 at 07:42:06PM -0400, Stefan Berger wrote:
> On 05/03/2017 06:38 PM, Jarkko Sakkinen wrote:
> > On Fri, Apr 28, 2017 at 09:02:15AM -0400, Stefan Berger wrote:
> > > The purpose of this series of patches is to enable the passing of the locality
> > > a command is executing in to a TPM emulator. To enable this we introduce a new
> > > flag for the device creation ioctl that requests that the locality be prepended
> > > to every command. For applications to check which flags the driver supports, we
> > > add a new ioctl that returns a bitmask of supported flags.
> > This is a weird change proposal as you could use tpm_vtpm_proxy for
> > other than some TPM emulator.
> 
> I think in most cases the recipient of the TPM commands from the vtpm_proxy
> driver will be a TPM emulator. What do you have in mind?

Like using Intel SGX to implement TPM in ring-3. I've thought
vtpm_tpm_proxy as generic proxy that you can also use for emulators.

/Jarkko

Re: [PATCH v2 3/3] tpm: vtpm_proxy: Add ioctl to request locality prepended to command

[Stefan Berger]

On 05/04/2017 05:17 AM, Jarkko Sakkinen wrote:
> On Wed, May 03, 2017 at 07:40:48PM -0400, Stefan Berger wrote:
>> On 05/03/2017 06:37 PM, Jarkko Sakkinen wrote:
>>> On Fri, Apr 28, 2017 at 09:02:18AM -0400, Stefan Berger wrote:
>>>> Add an ioctl to request that the locality be prepended to every TPM
>>>> command.
>>> Don't really understand this change. Why locality is prenpended?
>> Commands can be executed under locality 0-3 and for some commands it is
>> important to know which locality a user may have chosen. How else should we
>> convey that locality to the TPM emulator ?
> Why this is not in the commit message?
>
> More scalable way to do this would be to have a set of vtpm proxy
> commands. There could be a command for requesting and releasing
> locality. That would be more clean.

I would think that if someone wanted to use locality it's the client 
using /dev/tpm(rm)0 calling an ioctl or so and the vtpm proxy then 
merely passing that locality to the backend (TPM emulator). I suppose 
the intention is to support something like that following the addition 
of the new functions request_locality and release_locality?

Re: [PATCH v2 3/3] tpm: vtpm_proxy: Add ioctl to request locality prepended to command

[Jarkko Sakkinen]

On Thu, May 04, 2017 at 07:14:27AM -0400, Stefan Berger wrote:
> On 05/04/2017 05:17 AM, Jarkko Sakkinen wrote:
> > On Wed, May 03, 2017 at 07:40:48PM -0400, Stefan Berger wrote:
> > > On 05/03/2017 06:37 PM, Jarkko Sakkinen wrote:
> > > > On Fri, Apr 28, 2017 at 09:02:18AM -0400, Stefan Berger wrote:
> > > > > Add an ioctl to request that the locality be prepended to every TPM
> > > > > command.
> > > > Don't really understand this change. Why locality is prenpended?
> > > Commands can be executed under locality 0-3 and for some commands it is
> > > important to know which locality a user may have chosen. How else should we
> > > convey that locality to the TPM emulator ?
> > Why this is not in the commit message?
> > 
> > More scalable way to do this would be to have a set of vtpm proxy
> > commands. There could be a command for requesting and releasing
> > locality. That would be more clean.
> 
> I would think that if someone wanted to use locality it's the client using
> /dev/tpm(rm)0 calling an ioctl or so and the vtpm proxy then merely passing
> that locality to the backend (TPM emulator). I suppose the intention is to
> support something like that following the addition of the new functions
> request_locality and release_locality?

What if we later on want to pass something else than locality to the
backend? How that will work out?

/Jarkko

Re: [PATCH v2 3/3] tpm: vtpm_proxy: Add ioctl to request locality prepended to command

[Stefan Berger]

On 05/04/2017 02:40 PM, Jarkko Sakkinen wrote:
> On Thu, May 04, 2017 at 07:14:27AM -0400, Stefan Berger wrote:
>> On 05/04/2017 05:17 AM, Jarkko Sakkinen wrote:
>>> On Wed, May 03, 2017 at 07:40:48PM -0400, Stefan Berger wrote:
>>>> On 05/03/2017 06:37 PM, Jarkko Sakkinen wrote:
>>>>> On Fri, Apr 28, 2017 at 09:02:18AM -0400, Stefan Berger wrote:
>>>>>> Add an ioctl to request that the locality be prepended to every TPM
>>>>>> command.
>>>>> Don't really understand this change. Why locality is prenpended?
>>>> Commands can be executed under locality 0-3 and for some commands it is
>>>> important to know which locality a user may have chosen. How else should we
>>>> convey that locality to the TPM emulator ?
>>> Why this is not in the commit message?
>>>
>>> More scalable way to do this would be to have a set of vtpm proxy
>>> commands. There could be a command for requesting and releasing
>>> locality. That would be more clean.
>> I would think that if someone wanted to use locality it's the client using
>> /dev/tpm(rm)0 calling an ioctl or so and the vtpm proxy then merely passing
>> that locality to the backend (TPM emulator). I suppose the intention is to
>> support something like that following the addition of the new functions
>> request_locality and release_locality?
> What if we later on want to pass something else than locality to the
> backend? How that will work out?

'push' more data in front. 'pop' off by recipient. We could wrap the 
command in some form.

     Stefan

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

Re: [PATCH v2 3/3] tpm: vtpm_proxy: Add ioctl to request locality prepended to command

[Jarkko Sakkinen]

On Thu, May 04, 2017 at 04:03:18PM -0400, Stefan Berger wrote:
> On 05/04/2017 02:40 PM, Jarkko Sakkinen wrote:
> > On Thu, May 04, 2017 at 07:14:27AM -0400, Stefan Berger wrote:
> > > On 05/04/2017 05:17 AM, Jarkko Sakkinen wrote:
> > > > On Wed, May 03, 2017 at 07:40:48PM -0400, Stefan Berger wrote:
> > > > > On 05/03/2017 06:37 PM, Jarkko Sakkinen wrote:
> > > > > > On Fri, Apr 28, 2017 at 09:02:18AM -0400, Stefan Berger wrote:
> > > > > > > Add an ioctl to request that the locality be prepended to every TPM
> > > > > > > command.
> > > > > > Don't really understand this change. Why locality is prenpended?
> > > > > Commands can be executed under locality 0-3 and for some commands it is
> > > > > important to know which locality a user may have chosen. How else should we
> > > > > convey that locality to the TPM emulator ?
> > > > Why this is not in the commit message?
> > > > 
> > > > More scalable way to do this would be to have a set of vtpm proxy
> > > > commands. There could be a command for requesting and releasing
> > > > locality. That would be more clean.
> > > I would think that if someone wanted to use locality it's the client using
> > > /dev/tpm(rm)0 calling an ioctl or so and the vtpm proxy then merely passing
> > > that locality to the backend (TPM emulator). I suppose the intention is to
> > > support something like that following the addition of the new functions
> > > request_locality and release_locality?
> > What if we later on want to pass something else than locality to the
> > backend? How that will work out?
> 
> 'push' more data in front. 'pop' off by recipient. We could wrap the command
> in some form.
> 
>     Stefan

I would find having a set of special commands cleaner. Prepending sounds
like a quick hack to me, not really something that should exist in the
mainline.

/Jarkko

Re: [PATCH v2 3/3] tpm: vtpm_proxy: Add ioctl to request locality prepended to command

[Stefan Berger]

On 05/08/2017 07:43 PM, Jarkko Sakkinen wrote:
> On Thu, May 04, 2017 at 04:03:18PM -0400, Stefan Berger wrote:
>> On 05/04/2017 02:40 PM, Jarkko Sakkinen wrote:
>>> On Thu, May 04, 2017 at 07:14:27AM -0400, Stefan Berger wrote:
>>>> On 05/04/2017 05:17 AM, Jarkko Sakkinen wrote:
>>>>> On Wed, May 03, 2017 at 07:40:48PM -0400, Stefan Berger wrote:
>>>>>> On 05/03/2017 06:37 PM, Jarkko Sakkinen wrote:
>>>>>>> On Fri, Apr 28, 2017 at 09:02:18AM -0400, Stefan Berger wrote:
>>>>>>>> Add an ioctl to request that the locality be prepended to every TPM
>>>>>>>> command.
>>>>>>> Don't really understand this change. Why locality is prenpended?
>>>>>> Commands can be executed under locality 0-3 and for some commands it is
>>>>>> important to know which locality a user may have chosen. How else should we
>>>>>> convey that locality to the TPM emulator ?
>>>>> Why this is not in the commit message?
>>>>>
>>>>> More scalable way to do this would be to have a set of vtpm proxy
>>>>> commands. There could be a command for requesting and releasing
>>>>> locality. That would be more clean.
>>>> I would think that if someone wanted to use locality it's the client using
>>>> /dev/tpm(rm)0 calling an ioctl or so and the vtpm proxy then merely passing
>>>> that locality to the backend (TPM emulator). I suppose the intention is to
>>>> support something like that following the addition of the new functions
>>>> request_locality and release_locality?
>>> What if we later on want to pass something else than locality to the
>>> backend? How that will work out?
>> 'push' more data in front. 'pop' off by recipient. We could wrap the command
>> in some form.
>>
>>      Stefan
> I would find having a set of special commands cleaner. Prepending sounds
> like a quick hack to me, not really something that should exist in the
> mainline.

Along the lines of this here?

uint32_2 command
uint32_2 totlength
uint8_t locality
uint8_t buffer[]  <- the actual TPM command


With a command code like VTPM_PROXY_CMD_TPM_CMD = 1.

    Stefan

Re: [PATCH v2 3/3] tpm: vtpm_proxy: Add ioctl to request locality prepended to command

[Jarkko Sakkinen]

On Tue, May 09, 2017 at 11:49:05AM -0400, Stefan Berger wrote:
> On 05/08/2017 07:43 PM, Jarkko Sakkinen wrote:
> > On Thu, May 04, 2017 at 04:03:18PM -0400, Stefan Berger wrote:
> > > On 05/04/2017 02:40 PM, Jarkko Sakkinen wrote:
> > > > On Thu, May 04, 2017 at 07:14:27AM -0400, Stefan Berger wrote:
> > > > > On 05/04/2017 05:17 AM, Jarkko Sakkinen wrote:
> > > > > > On Wed, May 03, 2017 at 07:40:48PM -0400, Stefan Berger wrote:
> > > > > > > On 05/03/2017 06:37 PM, Jarkko Sakkinen wrote:
> > > > > > > > On Fri, Apr 28, 2017 at 09:02:18AM -0400, Stefan Berger wrote:
> > > > > > > > > Add an ioctl to request that the locality be prepended to every TPM
> > > > > > > > > command.
> > > > > > > > Don't really understand this change. Why locality is prenpended?
> > > > > > > Commands can be executed under locality 0-3 and for some commands it is
> > > > > > > important to know which locality a user may have chosen. How else should we
> > > > > > > convey that locality to the TPM emulator ?
> > > > > > Why this is not in the commit message?
> > > > > > 
> > > > > > More scalable way to do this would be to have a set of vtpm proxy
> > > > > > commands. There could be a command for requesting and releasing
> > > > > > locality. That would be more clean.
> > > > > I would think that if someone wanted to use locality it's the client using
> > > > > /dev/tpm(rm)0 calling an ioctl or so and the vtpm proxy then merely passing
> > > > > that locality to the backend (TPM emulator). I suppose the intention is to
> > > > > support something like that following the addition of the new functions
> > > > > request_locality and release_locality?
> > > > What if we later on want to pass something else than locality to the
> > > > backend? How that will work out?
> > > 'push' more data in front. 'pop' off by recipient. We could wrap the command
> > > in some form.
> > > 
> > >      Stefan
> > I would find having a set of special commands cleaner. Prepending sounds
> > like a quick hack to me, not really something that should exist in the
> > mainline.
> 
> Along the lines of this here?
> 
> uint32_2 command
> uint32_2 totlength
> uint8_t locality
> uint8_t buffer[]  <- the actual TPM command
> 
> 
> With a command code like VTPM_PROXY_CMD_TPM_CMD = 1.
> 
>    Stefan

That would break binary compability.

I would suggest allocating CC's backwards starting from 0xFFFFFFFF for
these control messages and send them in regular TPM command layout. A
bit similar idea as we have in the RM.

/Jarkko

Re: [PATCH v2 3/3] tpm: vtpm_proxy: Add ioctl to request locality prepended to command

[Stefan Berger]

On 05/10/2017 08:47 AM, Jarkko Sakkinen wrote:
> On Tue, May 09, 2017 at 11:49:05AM -0400, Stefan Berger wrote:
>> On 05/08/2017 07:43 PM, Jarkko Sakkinen wrote:
>>> On Thu, May 04, 2017 at 04:03:18PM -0400, Stefan Berger wrote:
>>>> On 05/04/2017 02:40 PM, Jarkko Sakkinen wrote:
>>>>> On Thu, May 04, 2017 at 07:14:27AM -0400, Stefan Berger wrote:
>>>>>> On 05/04/2017 05:17 AM, Jarkko Sakkinen wrote:
>>>>>>> On Wed, May 03, 2017 at 07:40:48PM -0400, Stefan Berger wrote:
>>>>>>>> On 05/03/2017 06:37 PM, Jarkko Sakkinen wrote:
>>>>>>>>> On Fri, Apr 28, 2017 at 09:02:18AM -0400, Stefan Berger wrote:
>>>>>>>>>> Add an ioctl to request that the locality be prepended to every TPM
>>>>>>>>>> command.
>>>>>>>>> Don't really understand this change. Why locality is prenpended?
>>>>>>>> Commands can be executed under locality 0-3 and for some commands it is
>>>>>>>> important to know which locality a user may have chosen. How else should we
>>>>>>>> convey that locality to the TPM emulator ?
>>>>>>> Why this is not in the commit message?
>>>>>>>
>>>>>>> More scalable way to do this would be to have a set of vtpm proxy
>>>>>>> commands. There could be a command for requesting and releasing
>>>>>>> locality. That would be more clean.
>>>>>> I would think that if someone wanted to use locality it's the client using
>>>>>> /dev/tpm(rm)0 calling an ioctl or so and the vtpm proxy then merely passing
>>>>>> that locality to the backend (TPM emulator). I suppose the intention is to
>>>>>> support something like that following the addition of the new functions
>>>>>> request_locality and release_locality?
>>>>> What if we later on want to pass something else than locality to the
>>>>> backend? How that will work out?
>>>> 'push' more data in front. 'pop' off by recipient. We could wrap the command
>>>> in some form.
>>>>
>>>>       Stefan
>>> I would find having a set of special commands cleaner. Prepending sounds
>>> like a quick hack to me, not really something that should exist in the
>>> mainline.
>> Along the lines of this here?
>>
>> uint32_2 command
>> uint32_2 totlength
>> uint8_t locality
>> uint8_t buffer[]  <- the actual TPM command
>>
>>
>> With a command code like VTPM_PROXY_CMD_TPM_CMD = 1.
>>
>>     Stefan
> That would break binary compability.

That's why I am adding that additional flag that allows a client to 
choose whether it wants the TPM command wrapped (or locality prepended) 
so that it knows what to expect from the driver. I don't think that 
breaks compatibility.

>
> I would suggest allocating CC's backwards starting from 0xFFFFFFFF for
> these control messages and send them in regular TPM command layout. A
> bit similar idea as we have in the RM.
>
> /Jarkko
>

Re: [PATCH v2 3/3] tpm: vtpm_proxy: Add ioctl to request locality prepended to command

[Jarkko Sakkinen]

On Wed, May 10, 2017 at 09:20:14AM -0400, Stefan Berger wrote:
> On 05/10/2017 08:47 AM, Jarkko Sakkinen wrote:
> > On Tue, May 09, 2017 at 11:49:05AM -0400, Stefan Berger wrote:
> > > On 05/08/2017 07:43 PM, Jarkko Sakkinen wrote:
> > > > On Thu, May 04, 2017 at 04:03:18PM -0400, Stefan Berger wrote:
> > > > > On 05/04/2017 02:40 PM, Jarkko Sakkinen wrote:
> > > > > > On Thu, May 04, 2017 at 07:14:27AM -0400, Stefan Berger wrote:
> > > > > > > On 05/04/2017 05:17 AM, Jarkko Sakkinen wrote:
> > > > > > > > On Wed, May 03, 2017 at 07:40:48PM -0400, Stefan Berger wrote:
> > > > > > > > > On 05/03/2017 06:37 PM, Jarkko Sakkinen wrote:
> > > > > > > > > > On Fri, Apr 28, 2017 at 09:02:18AM -0400, Stefan Berger wrote:
> > > > > > > > > > > Add an ioctl to request that the locality be prepended to every TPM
> > > > > > > > > > > command.
> > > > > > > > > > Don't really understand this change. Why locality is prenpended?
> > > > > > > > > Commands can be executed under locality 0-3 and for some commands it is
> > > > > > > > > important to know which locality a user may have chosen. How else should we
> > > > > > > > > convey that locality to the TPM emulator ?
> > > > > > > > Why this is not in the commit message?
> > > > > > > > 
> > > > > > > > More scalable way to do this would be to have a set of vtpm proxy
> > > > > > > > commands. There could be a command for requesting and releasing
> > > > > > > > locality. That would be more clean.
> > > > > > > I would think that if someone wanted to use locality it's the client using
> > > > > > > /dev/tpm(rm)0 calling an ioctl or so and the vtpm proxy then merely passing
> > > > > > > that locality to the backend (TPM emulator). I suppose the intention is to
> > > > > > > support something like that following the addition of the new functions
> > > > > > > request_locality and release_locality?
> > > > > > What if we later on want to pass something else than locality to the
> > > > > > backend? How that will work out?
> > > > > 'push' more data in front. 'pop' off by recipient. We could wrap the command
> > > > > in some form.
> > > > > 
> > > > >       Stefan
> > > > I would find having a set of special commands cleaner. Prepending sounds
> > > > like a quick hack to me, not really something that should exist in the
> > > > mainline.
> > > Along the lines of this here?
> > > 
> > > uint32_2 command
> > > uint32_2 totlength
> > > uint8_t locality
> > > uint8_t buffer[]  <- the actual TPM command
> > > 
> > > 
> > > With a command code like VTPM_PROXY_CMD_TPM_CMD = 1.
> > > 
> > >     Stefan
> > That would break binary compability.
> 
> That's why I am adding that additional flag that allows a client to choose
> whether it wants the TPM command wrapped (or locality prepended) so that it
> knows what to expect from the driver. I don't think that breaks
> compatibility.

I think having TPM command codes for control messages would be a
better idea and it is trivial to filter them so that client cannot
use those commands.

/Jarkko