| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <58B41184.7020200@linux.vnet.ibm.com>
Date: Mon, 27 Feb 2017 17:16:12 +0530
From: Nayna <nayna@...ux.vnet.ibm.com>
To: James Bottomley <James.Bottomley@...senPartnership.com>,
Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>,
tpmdd-devel@...ts.sourceforge.net
Cc: dhowells@...hat.com, open list <linux-kernel@...r.kernel.org>,
linux-security-module@...r.kernel.org
Subject: Re: [tpmdd-devel] [PATCH v2 6/7] tpm: expose spaces via a device link
/dev/tpms<n>
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@...senPartnership.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@...senPartnership.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@...senPartnership.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,
>>> +};
>>> +
>>>
>>
>
Powered by blists - more mailing lists