Messages in this thread | | | Subject | Re: [tpmdd-devel] [PATCH v2 6/7] tpm: expose spaces via a device link /dev/tpms<n> | From | Nayna <> | Date | Fri, 24 Feb 2017 12:29:13 +0530 |
| |
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 ?
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.
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, > +}; > + >
| |