| Message ID | 1493384538-27883-4-git-send-email-stefanb@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
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
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
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 > ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
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? ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
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 > ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
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 > ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
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, }; /**
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(-)