lkml.org 
[lkml]   [2017]   [Feb]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [tpmdd-devel] [PATCH v2 6/7] tpm: expose spaces via a device link /dev/tpms<n>
    From
    Date


    On 02/24/2017 06:23 PM, James Bottomley wrote:
    > On Fri, 2017-02-24 at 12:29 +0530, Nayna wrote:
    >>
    >> On 02/17/2017 12:55 AM, Jarkko Sakkinen wrote:
    >>> From: James Bottomley <James.Bottomley@HansenPartnership.com>
    >>>
    >>> Currently the tpm spaces are not exposed to userspace. Make this
    >>> exposure via a separate device, which can now be opened multiple
    >>> times because each read/write transaction goes separately via the
    >>> space.
    >>>
    >>> Concurrency is protected by the chip->tpm_mutex for each read/write
    >>> transaction separately. The TPM is cleared of all transient
    >>> objects by the time the mutex is dropped, so there should be no
    >>> interference between the kernel and userspace.
    >>
    >> To understand, I have two questions:
    >>
    >> 1. How would a userspace application using TPM know whether to use
    >> /dev/tpm0 or /dev/tpms0 ?
    >
    > Likely they can't use /dev/tpm0 becuase it will be root only, but the
    > major indicator will be whether /dev/tpms0 exists or not.

    Thanks James !!
    Currently, I see even /dev/tpms0 is also only root accessible. I did see
    the discussion to make it 0666, and I understand adding command
    filtering is part of enabling /dev/tpms0 as all-accessible.

    Sorry, I didn't understand when you said, "major indicator will be
    whether /dev/tpms0" exists or not ? I mean in what case it might not
    exist..

    >
    >> 2. How would a userspace RM know to build on top of /dev/tpm0 or
    >> /dev/tpms0. And if it is built on top of /dev/tpms0, can there be
    >> issues with one RM on top of other RM.
    >
    > There's a known problem with RMs in that they're not fully stackable,
    > so I suspect the answer is that if tpms0 exists you won't use an RM,
    > but this is currently an area of active research. The other potential
    > problem is that if you build a RM on tpm0 in userspace, it will fight
    > with the kernel when the kernel uses sessions.

    Does it imply that there should be restriction to disallow any RM
    specific commands from userspace on /dev/tpm0 ?

    Thanks & Regards,
    - Nayna

    >
    > James
    >
    >> Thanks & Regards,
    >> - Nayna
    >>
    >>
    >>>
    >>> Signed-off-by: James Bottomley <
    >>> James.Bottomley@HansenPartnership.com>
    >>> ---
    >>> drivers/char/tpm/Makefile | 3 +-
    >>> drivers/char/tpm/tpm-chip.c | 73
    >>> ++++++++++++++++++++++++++++++++++++++--
    >>> drivers/char/tpm/tpm-interface.c | 13 +++++--
    >>> drivers/char/tpm/tpm.h | 4 +++
    >>> drivers/char/tpm/tpms-dev.c | 65
    >>> +++++++++++++++++++++++++++++++++++
    >>> 5 files changed, 152 insertions(+), 6 deletions(-)
    >>> create mode 100644 drivers/char/tpm/tpms-dev.c
    >>>
    >>> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
    >>> index 10e5827..bbe6531 100644
    >>> --- a/drivers/char/tpm/Makefile
    >>> +++ b/drivers/char/tpm/Makefile
    >>> @@ -3,7 +3,8 @@
    >>> #
    >>> obj-$(CONFIG_TCG_TPM) += tpm.o
    >>> tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2
    >>> -cmd.o \
    >>> - tpm-dev-common.o tpm1_eventlog.o tpm2_eventlog.o tpm2
    >>> -space.o
    >>> + tpm-dev-common.o tpms-dev.o tpm1_eventlog.o
    >>> tpm2_eventlog.o \
    >>> + tpm2-space.o
    >>> tpm-$(CONFIG_ACPI) += tpm_ppi.o tpm_acpi.o
    >>> tpm-$(CONFIG_OF) += tpm_of.o
    >>> obj-$(CONFIG_TCG_TIS_CORE) += tpm_tis_core.o
    >>> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm
    >>> -chip.c
    >>> index 993b9ae..c71c353 100644
    >>> --- a/drivers/char/tpm/tpm-chip.c
    >>> +++ b/drivers/char/tpm/tpm-chip.c
    >>> @@ -33,6 +33,7 @@ DEFINE_IDR(dev_nums_idr);
    >>> static DEFINE_MUTEX(idr_lock);
    >>>
    >>> struct class *tpm_class;
    >>> +struct class *tpms_class;
    >>> dev_t tpm_devt;
    >>>
    >>> /**
    >>> @@ -132,6 +133,14 @@ static void tpm_dev_release(struct device
    >>> *dev)
    >>> kfree(chip);
    >>> }
    >>>
    >>> +static void tpm_devs_release(struct device *dev)
    >>> +{
    >>> + struct tpm_chip *chip = container_of(dev, struct tpm_chip,
    >>> devs);
    >>> +
    >>> + /* release the master device reference */
    >>> + put_device(&chip->dev);
    >>> +}
    >>> +
    >>> /**
    >>> * tpm_chip_alloc() - allocate a new struct tpm_chip instance
    >>> * @pdev: device to which the chip is associated
    >>> @@ -168,27 +177,47 @@ struct tpm_chip *tpm_chip_alloc(struct device
    >>> *pdev,
    >>> chip->dev_num = rc;
    >>>
    >>> device_initialize(&chip->dev);
    >>> + device_initialize(&chip->devs);
    >>>
    >>> chip->dev.class = tpm_class;
    >>> chip->dev.release = tpm_dev_release;
    >>> chip->dev.parent = pdev;
    >>> chip->dev.groups = chip->groups;
    >>>
    >>> + chip->devs.parent = pdev;
    >>> + chip->devs.class = tpms_class;
    >>> + chip->devs.release = tpm_devs_release;
    >>> + /* get extra reference on main device to hold on
    >>> + * behalf of devs. This holds the chip structure
    >>> + * while cdevs is in use. The corresponding put
    >>> + * is in the tpm_devs_release
    >>> + */
    >>> + get_device(&chip->dev);
    >>> +
    >>> if (chip->dev_num == 0)
    >>> chip->dev.devt = MKDEV(MISC_MAJOR, TPM_MINOR);
    >>> else
    >>> chip->dev.devt = MKDEV(MAJOR(tpm_devt), chip
    >>> ->dev_num);
    >>>
    >>> + chip->devs.devt =
    >>> + MKDEV(MAJOR(tpm_devt), chip->dev_num +
    >>> TPM_NUM_DEVICES);
    >>> +
    >>> rc = dev_set_name(&chip->dev, "tpm%d", chip->dev_num);
    >>> if (rc)
    >>> goto out;
    >>> + rc = dev_set_name(&chip->devs, "tpms%d", chip->dev_num);
    >>> + if (rc)
    >>> + goto out;
    >>>
    >>> if (!pdev)
    >>> chip->flags |= TPM_CHIP_FLAG_VIRTUAL;
    >>>
    >>> cdev_init(&chip->cdev, &tpm_fops);
    >>> + cdev_init(&chip->cdevs, &tpms_fops);
    >>> chip->cdev.owner = THIS_MODULE;
    >>> + chip->cdevs.owner = THIS_MODULE;
    >>> chip->cdev.kobj.parent = &chip->dev.kobj;
    >>> + chip->cdevs.kobj.parent = &chip->devs.kobj;
    >>>
    >>> chip->work_space.context_buf = kzalloc(PAGE_SIZE,
    >>> GFP_KERNEL);
    >>> if (!chip->work_space.context_buf) {
    >>> @@ -199,6 +228,7 @@ struct tpm_chip *tpm_chip_alloc(struct device
    >>> *pdev,
    >>> return chip;
    >>>
    >>> out:
    >>> + put_device(&chip->devs);
    >>> put_device(&chip->dev);
    >>> return ERR_PTR(rc);
    >>> }
    >>> @@ -244,7 +274,7 @@ static int tpm_add_char_device(struct tpm_chip
    >>> *chip)
    >>> dev_name(&chip->dev), MAJOR(chip
    >>> ->dev.devt),
    >>> MINOR(chip->dev.devt), rc);
    >>>
    >>> - return rc;
    >>> + goto err_1;
    >>> }
    >>>
    >>> rc = device_add(&chip->dev);
    >>> @@ -254,16 +284,44 @@ static int tpm_add_char_device(struct
    >>> tpm_chip *chip)
    >>> dev_name(&chip->dev), MAJOR(chip
    >>> ->dev.devt),
    >>> MINOR(chip->dev.devt), rc);
    >>>
    >>> - cdev_del(&chip->cdev);
    >>> - return rc;
    >>> + goto err_2;
    >>> + }
    >>> +
    >>> + if (chip->flags & TPM_CHIP_FLAG_TPM2)
    >>> + rc = cdev_add(&chip->cdevs, chip->devs.devt, 1);
    >>> + if (rc) {
    >>> + dev_err(&chip->dev,
    >>> + "unable to cdev_add() %s, major %d, minor
    >>> %d, err=%d\n",
    >>> + dev_name(&chip->devs), MAJOR(chip
    >>> ->devs.devt),
    >>> + MINOR(chip->devs.devt), rc);
    >>> +
    >>> + goto err_3;
    >>> }
    >>>
    >>> + if (chip->flags & TPM_CHIP_FLAG_TPM2)
    >>> + rc = device_add(&chip->devs);
    >>> + if (rc) {
    >>> + dev_err(&chip->dev,
    >>> + "unable to device_register() %s, major %d,
    >>> minor %d, err=%d\n",
    >>> + dev_name(&chip->devs), MAJOR(chip
    >>> ->devs.devt),
    >>> + MINOR(chip->devs.devt), rc);
    >>> +
    >>> + goto err_4;
    >>> + }
    >>> /* Make the chip available. */
    >>> mutex_lock(&idr_lock);
    >>> idr_replace(&dev_nums_idr, chip, chip->dev_num);
    >>> mutex_unlock(&idr_lock);
    >>>
    >>> return rc;
    >>> + err_4:
    >>> + cdev_del(&chip->cdevs);
    >>> + err_3:
    >>> + device_del(&chip->dev);
    >>> + err_2:
    >>> + cdev_del(&chip->cdev);
    >>> + err_1:
    >>> + return rc;
    >>> }
    >>>
    >>> static void tpm_del_char_device(struct tpm_chip *chip)
    >>> @@ -271,6 +329,11 @@ static void tpm_del_char_device(struct
    >>> tpm_chip *chip)
    >>> cdev_del(&chip->cdev);
    >>> device_del(&chip->dev);
    >>>
    >>> + if (chip->flags & TPM_CHIP_FLAG_TPM2) {
    >>> + cdev_del(&chip->cdevs);
    >>> + device_del(&chip->devs);
    >>> + }
    >>> +
    >>> /* Make the chip unavailable. */
    >>> mutex_lock(&idr_lock);
    >>> idr_replace(&dev_nums_idr, NULL, chip->dev_num);
    >>> @@ -282,6 +345,10 @@ static void tpm_del_char_device(struct
    >>> tpm_chip *chip)
    >>> tpm2_shutdown(chip, TPM2_SU_CLEAR);
    >>> chip->ops = NULL;
    >>> up_write(&chip->ops_sem);
    >>> + /* will release the devs reference to the chip->dev unless
    >>> + * something has cdevs open
    >>> + */
    >>> + put_device(&chip->devs);
    >>> }
    >>>
    >>> static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
    >>> diff --git a/drivers/char/tpm/tpm-interface.c
    >>> b/drivers/char/tpm/tpm-interface.c
    >>> index db5ffe9..deb2021 100644
    >>> --- a/drivers/char/tpm/tpm-interface.c
    >>> +++ b/drivers/char/tpm/tpm-interface.c
    >>> @@ -1257,9 +1257,17 @@ static int __init tpm_init(void)
    >>> return PTR_ERR(tpm_class);
    >>> }
    >>>
    >>> - rc = alloc_chrdev_region(&tpm_devt, 0, TPM_NUM_DEVICES,
    >>> "tpm");
    >>> + tpms_class = class_create(THIS_MODULE, "tpms");
    >>> + if (IS_ERR(tpms_class)) {
    >>> + pr_err("couldn't create tpms class\n");
    >>> + class_destroy(tpm_class);
    >>> + return PTR_ERR(tpms_class);
    >>> + }
    >>> +
    >>> + rc = alloc_chrdev_region(&tpm_devt, 0, 2*TPM_NUM_DEVICES,
    >>> "tpm");
    >>> if (rc < 0) {
    >>> pr_err("tpm: failed to allocate char dev
    >>> region\n");
    >>> + class_destroy(tpms_class);
    >>> class_destroy(tpm_class);
    >>> return rc;
    >>> }
    >>> @@ -1271,7 +1279,8 @@ static void __exit tpm_exit(void)
    >>> {
    >>> idr_destroy(&dev_nums_idr);
    >>> class_destroy(tpm_class);
    >>> - unregister_chrdev_region(tpm_devt, TPM_NUM_DEVICES);
    >>> + class_destroy(tpms_class);
    >>> + unregister_chrdev_region(tpm_devt, 2*TPM_NUM_DEVICES);
    >>> }
    >>>
    >>> subsys_initcall(tpm_init);
    >>> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
    >>> index 97e48a4..822ca67 100644
    >>> --- a/drivers/char/tpm/tpm.h
    >>> +++ b/drivers/char/tpm/tpm.h
    >>> @@ -182,7 +182,9 @@ struct tpm_chip_seqops {
    >>>
    >>> struct tpm_chip {
    >>> struct device dev;
    >>> + struct device devs;
    >>> struct cdev cdev;
    >>> + struct cdev cdevs;
    >>>
    >>> /* A driver callback under ops cannot be run unless
    >>> ops_sem is held
    >>> * (sometimes implicitly, eg for the sysfs code). ops
    >>> becomes null
    >>> @@ -510,8 +512,10 @@ static inline void tpm_buf_append_u32(struct
    >>> tpm_buf *buf, const u32 value)
    >>> }
    >>>
    >>> extern struct class *tpm_class;
    >>> +extern struct class *tpms_class;
    >>> extern dev_t tpm_devt;
    >>> extern const struct file_operations tpm_fops;
    >>> +extern const struct file_operations tpms_fops;
    >>> extern struct idr dev_nums_idr;
    >>>
    >>> enum tpm_transmit_flags {
    >>> diff --git a/drivers/char/tpm/tpms-dev.c b/drivers/char/tpm/tpms
    >>> -dev.c
    >>> new file mode 100644
    >>> index 0000000..5720885
    >>> --- /dev/null
    >>> +++ b/drivers/char/tpm/tpms-dev.c
    >>> @@ -0,0 +1,65 @@
    >>> +/*
    >>> + * Copyright (C) 2017 James.Bottomley@HansenPartnership.com
    >>> + *
    >>> + * GPLv2
    >>> + */
    >>> +#include <linux/slab.h>
    >>> +#include "tpm-dev.h"
    >>> +
    >>> +struct tpms_priv {
    >>> + struct file_priv priv;
    >>> + struct tpm_space space;
    >>> +};
    >>> +
    >>> +static int tpms_open(struct inode *inode, struct file *file)
    >>> +{
    >>> + struct tpm_chip *chip;
    >>> + struct tpms_priv *priv;
    >>> + int rc;
    >>> +
    >>> + chip = container_of(inode->i_cdev, struct tpm_chip,
    >>> cdevs);
    >>> + priv = kzalloc(sizeof(*priv), GFP_KERNEL);
    >>> + if (priv == NULL)
    >>> + return -ENOMEM;
    >>> +
    >>> + rc = tpm2_init_space(&priv->space);
    >>> + if (rc) {
    >>> + kfree(priv);
    >>> + return -ENOMEM;
    >>> + }
    >>> +
    >>> + tpm_common_open(file, chip, &priv->priv);
    >>> +
    >>> + return 0;
    >>> +}
    >>> +
    >>> +static int tpms_release(struct inode *inode, struct file *file)
    >>> +{
    >>> + struct file_priv *fpriv = file->private_data;
    >>> + struct tpms_priv *priv = container_of(fpriv, struct
    >>> tpms_priv, priv);
    >>> +
    >>> + tpm_common_release(file, fpriv);
    >>> + tpm2_del_space(&priv->space);
    >>> + kfree(priv);
    >>> +
    >>> + return 0;
    >>> +}
    >>> +
    >>> +ssize_t tpms_write(struct file *file, const char __user *buf,
    >>> + size_t size, loff_t *off)
    >>> +{
    >>> + struct file_priv *fpriv = file->private_data;
    >>> + struct tpms_priv *priv = container_of(fpriv, struct
    >>> tpms_priv, priv);
    >>> +
    >>> + return tpm_common_write(file, buf, size, off, &priv
    >>> ->space);
    >>> +}
    >>> +
    >>> +const struct file_operations tpms_fops = {
    >>> + .owner = THIS_MODULE,
    >>> + .llseek = no_llseek,
    >>> + .open = tpms_open,
    >>> + .read = tpm_common_read,
    >>> + .write = tpms_write,
    >>> + .release = tpms_release,
    >>> +};
    >>> +
    >>>
    >>
    >

    \
     
     \ /
      Last update: 2017-02-27 12:49    [W:4.962 / U:0.368 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site