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 <[email protected]> >>> >>> 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 < >>> [email protected]> >>> --- >>> 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 [email protected] >>> + * >>> + * 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, >>> +}; >>> + >>> >> > ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot _______________________________________________ tpmdd-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
