diff mbox

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

Message ID 1493384538-27883-4-git-send-email-stefanb@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Stefan Berger April 28, 2017, 1:02 p.m. UTC
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(-)

Comments

kernel test robot April 29, 2017, 7:02 a.m. UTC | #1
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
Jarkko Sakkinen May 3, 2017, 10:37 p.m. UTC | #2
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
--
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
Stefan Berger May 3, 2017, 11:40 p.m. UTC | #3
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
>

--
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
Jarkko Sakkinen May 4, 2017, 9:17 a.m. UTC | #4
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
--
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
Stefan Berger May 4, 2017, 11:14 a.m. UTC | #5
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?

--
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
Jarkko Sakkinen May 4, 2017, 6:40 p.m. UTC | #6
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
--
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
Stefan Berger May 4, 2017, 8:03 p.m. UTC | #7
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
>

--
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
Jarkko Sakkinen May 8, 2017, 11:43 p.m. UTC | #8
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
--
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
Stefan Berger May 9, 2017, 3:49 p.m. UTC | #9
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

--
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
Jarkko Sakkinen May 10, 2017, 12:47 p.m. UTC | #10
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
--
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
Stefan Berger May 10, 2017, 1:20 p.m. UTC | #11
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
>

--
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
Jarkko Sakkinen May 10, 2017, 6:33 p.m. UTC | #12
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
--
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
diff mbox

Patch

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,
 };
 
 /**