Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1557653 > unrolled thread

[PATCH RFC v2 0/5] RFC: in-kernel resource manager

Back to article view | Back to linux.kernel

Contents

  [PATCH RFC v2 0/5] RFC: in-kernel resource manager Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-12 18:50 +0100
    [PATCH RFC v2 1/5] tpm: validate TPM 2.0 commands Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-12 18:50 +0100
      Re: [PATCH RFC v2 1/5] tpm: validate TPM 2.0 commands Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-12 21:40 +0100
    [PATCH RFC v2 3/5] tpm: infrastructure for TPM spaces Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-12 18:50 +0100
      Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM  spaces James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-01-12 19:50 +0100
        Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM  spaces Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-12 21:40 +0100
      Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM  spaces James Bottomley <jejb@linux.vnet.ibm.com> - 2017-01-12 21:40 +0100
        Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM  spaces Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-13 17:30 +0100
          Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM  spaces Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-16 11:00 +0100
      Re: [PATCH RFC v2 3/5] tpm: infrastructure for TPM spaces Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-12 22:00 +0100
      Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM  spaces James Bottomley <jejb@linux.vnet.ibm.com> - 2017-01-13 02:20 +0100
        Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM  spaces Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-13 17:40 +0100
        Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM  spaces Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-16 10:10 +0100
          Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM  spaces James Bottomley <jejb@linux.vnet.ibm.com> - 2017-01-16 15:30 +0100
            Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM  spaces Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-16 15:50 +0100
              Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM  spaces James Bottomley <jejb@linux.vnet.ibm.com> - 2017-01-16 16:00 +0100
                Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM  spaces Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-16 18:00 +0100
    [PATCH RFC v2 2/5] tpm: export tpm2_flush_context_cmd Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-12 18:50 +0100
    [PATCH RFC v2 5/5] tpm2: expose resource manager via a device link /dev/tpms<n> Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-12 18:50 +0100
      Re: [PATCH RFC v2 5/5] tpm2: expose resource manager via a device  link /dev/tpms<n> Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-12 19:50 +0100
        Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager  via a device link /dev/tpms<n> James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-01-13 20:30 +0100
          Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager  via a device link /dev/tpms<n> Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-13 20:50 +0100
            Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager  via a device link /dev/tpms<n> James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-01-13 21:10 +0100
              Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager  via a device link /dev/tpms<n> Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-13 22:30 +0100
                Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager  via a device link /dev/tpms<n> James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-01-14 02:20 +0100
                  Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager  via a device link /dev/tpms<n> Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-16 18:00 +0100
      Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager  via    a device link /dev/tpms<n> James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-01-12 21:00 +0100
      Re: [PATCH RFC v2 5/5] tpm2: expose resource manager via a device  link /dev/tpms<n> Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-12 22:00 +0100
        Re: [PATCH RFC v2 5/5] tpm2: expose resource manager via a device  link /dev/tpms<n> Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-13 18:30 +0100
          Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager  via a device link /dev/tpms<n> James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-01-13 18:50 +0100
            Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager  via a device link /dev/tpms<n> Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-13 19:10 +0100
              Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager  via a device link /dev/tpms<n> James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-01-13 19:20 +0100
            Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager  via a device link /dev/tpms<n> Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-16 10:50 +0100
    [PATCH RFC v2 4/5] tpm: split out tpm-dev.c into tpm-dev.c and tpm-common-dev.c Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-12 18:50 +0100
      Re: [tpmdd-devel] [PATCH RFC v2 4/5] tpm: split out tpm-dev.c into  tpm-dev.c and tpm-common-dev.c James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-01-13 20:20 +0100

Page 2 of 2 — ← Prev page 1 [2]

#1558701 — Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager via a device link /dev/tpms<n>

On Thu, 2017-01-12 at 11:39 -0700, Jason Gunthorpe wrote:
> On Thu, Jan 12, 2017 at 07:46:08PM +0200, Jarkko Sakkinen wrote:
> 
> >  struct tpm_chip {
> > -	struct device dev;
> > -	struct cdev cdev;
> > +	struct device dev, devrm;
> 
> Hum.. devrm adds a new kref but doesn't do anything with the release
> function, so that is going to use after free, ie here:
> 
> > 	put_device(&chip->dev);
> > +	put_device(&chip->devrm);
> > 	return ERR_PTR(rc);
> 
> And other places.
> 
> One solution is to get_device(chip->dev) after
> device_initialize(dev->rm) and add a devrm->dev.release function to
> do put_device(chip->dev)

Actually, no, the devrm is a completely lifetime managed device as part
of the chip structure.  once you've done a device_del on it, it can be
kfreed because it's no longer visible to anything else.  The fix is
simply not to do the put.

With that and the other errors, here's a v3

James

---

From 572cbf2ac64df040be084182d750f55df836a759 Mon Sep 17 00:00:00 2001
From: James Bottomley <James.Bottomley@HansenPartnership.com>
Date: Tue, 3 Jan 2017 09:07:32 -0800
Subject: [PATCH 5/5] tpm2: expose resource manager via a device link
 /dev/tpms<n>

Currently the Resource Manager (RM) is 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 RM.

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.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/char/tpm/Makefile        |  2 +-
 drivers/char/tpm/tpm-chip.c      | 53 ++++++++++++++++++++++++++++++++--
 drivers/char/tpm/tpm-interface.c | 13 +++++++--
 drivers/char/tpm/tpm.h           |  6 ++--
 drivers/char/tpm/tpms-dev.c      | 62 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 128 insertions(+), 8 deletions(-)
 create mode 100644 drivers/char/tpm/tpms-dev.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 13ff5da..e50d768 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -3,7 +3,7 @@
 #
 obj-$(CONFIG_TCG_TPM) += tpm.o
 tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
-	 tpm_eventlog.o tpm2-space.o tpm-dev-common.o
+	 tpm_eventlog.o tpm2-space.o tpm-dev-common.o tpms-dev.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..4548df0 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 *tpm_rm_class;
 dev_t tpm_devt;
 
 /**
@@ -168,27 +169,40 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
 	chip->dev_num = rc;
 
 	device_initialize(&chip->dev);
+	device_initialize(&chip->devrm);
 
 	chip->dev.class = tpm_class;
 	chip->dev.release = tpm_dev_release;
 	chip->dev.parent = pdev;
 	chip->dev.groups = chip->groups;
 
+	chip->devrm.parent = pdev;
+	chip->devrm.class = tpm_rm_class;
+
 	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->devrm.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->devrm, "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->cdevrm, &tpm_rm_fops);
 	chip->cdev.owner = THIS_MODULE;
+	chip->cdevrm.owner = THIS_MODULE;
 	chip->cdev.kobj.parent = &chip->dev.kobj;
+	chip->cdevrm.kobj.parent = &chip->devrm.kobj;
 
 	chip->work_space.context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
 	if (!chip->work_space.context_buf) {
@@ -244,7 +258,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 +268,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->cdevrm, chip->devrm.devt, 1);
+	if (rc) {
+		dev_err(&chip->dev,
+			"unable to cdev_add() %s, major %d, minor %d, err=%d\n",
+			dev_name(&chip->devrm), MAJOR(chip->devrm.devt),
+			MINOR(chip->devrm.devt), rc);
+
+		goto err_3;
 	}
 
+	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		rc = device_add(&chip->devrm);
+	if (rc) {
+		dev_err(&chip->dev,
+			"unable to device_register() %s, major %d, minor %d, err=%d\n",
+			dev_name(&chip->devrm), MAJOR(chip->devrm.devt),
+			MINOR(chip->devrm.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->cdevrm);
+ 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 +313,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->cdevrm);
+		device_del(&chip->devrm);
+	}
+
 	/* Make the chip unavailable. */
 	mutex_lock(&idr_lock);
 	idr_replace(&dev_nums_idr, NULL, chip->dev_num);
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 65fcd04c..f5c9355 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -1197,9 +1197,17 @@ static int __init tpm_init(void)
 		return PTR_ERR(tpm_class);
 	}
 
-	rc = alloc_chrdev_region(&tpm_devt, 0, TPM_NUM_DEVICES, "tpm");
+	tpm_rm_class = class_create(THIS_MODULE, "tpms");
+	if (IS_ERR(tpm_rm_class)) {
+		pr_err("couldn't create tpmrm class\n");
+		class_destroy(tpm_class);
+		return PTR_ERR(tpm_rm_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(tpm_rm_class);
 		class_destroy(tpm_class);
 		return rc;
 	}
@@ -1211,7 +1219,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(tpm_rm_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 0e93b93..61422e6 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -172,8 +172,8 @@ struct tpm_chip_seqops {
 };
 
 struct tpm_chip {
-	struct device dev;
-	struct cdev cdev;
+	struct device dev, devrm;
+	struct cdev cdev, cdevrm;
 
 	/* 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 +510,10 @@ static inline void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
 }
 
 extern struct class *tpm_class;
+extern struct class *tpm_rm_class;
 extern dev_t tpm_devt;
 extern const struct file_operations tpm_fops;
+extern const struct file_operations tpm_rm_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..c10b308
--- /dev/null
+++ b/drivers/char/tpm/tpms-dev.c
@@ -0,0 +1,62 @@
+/*
+ * 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;
+
+	chip = container_of(inode->i_cdev, struct tpm_chip, cdevrm);
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (priv == NULL)
+		return -ENOMEM;
+	priv->space.context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
+	if (priv->space.context_buf == NULL) {
+		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);
+	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 tpm_rm_fops = {
+	.owner = THIS_MODULE,
+	.llseek = no_llseek,
+	.open = tpms_open,
+	.read = tpm_common_read,
+	.write = tpms_write,
+	.release = tpms_release,
+};
+
-- 
2.6.6

[toc] | [prev] | [next] | [standalone]

#1558706 — Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager via a device link /dev/tpms<n>

On Fri, Jan 13, 2017 at 11:20:47AM -0800, James Bottomley wrote:
> On Thu, 2017-01-12 at 11:39 -0700, Jason Gunthorpe wrote:
> > On Thu, Jan 12, 2017 at 07:46:08PM +0200, Jarkko Sakkinen wrote:
> > 
> > >  struct tpm_chip {
> > > -	struct device dev;
> > > -	struct cdev cdev;
> > > +	struct device dev, devrm;
> > 
> > Hum.. devrm adds a new kref but doesn't do anything with the release
> > function, so that is going to use after free, ie here:
> > 
> > > 	put_device(&chip->dev);
> > > +	put_device(&chip->devrm);
> > > 	return ERR_PTR(rc);
> > 
> > And other places.
> > 
> > One solution is to get_device(chip->dev) after
> > device_initialize(dev->rm) and add a devrm->dev.release function to
> > do put_device(chip->dev)
> 
> Actually, no, the devrm is a completely lifetime managed device as part
> of the chip structure.  once you've done a device_del on it, it can be
> kfreed because it's no longer visible to anything else.

No, that isn't enough. Anything else could have obtained a kref on
devrm outside of the sphere the device_del manages.

For instance, the cdev does exactly that, via this:

>  	chip->cdev.kobj.parent = &chip->dev.kobj;
> +	chip->cdevrm.kobj.parent = &chip->devrm.kobj;

In the worst case the kref the cdev grabs is not released until after
tpm_chip_unregister() returns.

Having a kref that doesn't work is just asking for trouble, please
make it work properly.

Jason

[toc] | [prev] | [next] | [standalone]

#1558722 — Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager via a device link /dev/tpms<n>

On Fri, 2017-01-13 at 12:47 -0700, Jason Gunthorpe wrote:
> On Fri, Jan 13, 2017 at 11:20:47AM -0800, James Bottomley wrote:
> > On Thu, 2017-01-12 at 11:39 -0700, Jason Gunthorpe wrote:
> > > On Thu, Jan 12, 2017 at 07:46:08PM +0200, Jarkko Sakkinen wrote:
> > > 
> > > >  struct tpm_chip {
> > > > -	struct device dev;
> > > > -	struct cdev cdev;
> > > > +	struct device dev, devrm;
> > > 
> > > Hum.. devrm adds a new kref but doesn't do anything with the
> > > release
> > > function, so that is going to use after free, ie here:
> > > 
> > > > 	put_device(&chip->dev);
> > > > +	put_device(&chip->devrm);
> > > > 	return ERR_PTR(rc);
> > > 
> > > And other places.
> > > 
> > > One solution is to get_device(chip->dev) after
> > > device_initialize(dev->rm) and add a devrm->dev.release function
> > > to
> > > do put_device(chip->dev)
> > 
> > Actually, no, the devrm is a completely lifetime managed device as
> > part
> > of the chip structure.  once you've done a device_del on it, it can
> > be
> > kfreed because it's no longer visible to anything else.
> 
> No, that isn't enough. Anything else could have obtained a kref on
> devrm outside of the sphere the device_del manages.
> 
> For instance, the cdev does exactly that, via this:
> 
> >  	chip->cdev.kobj.parent = &chip->dev.kobj;
> > +	chip->cdevrm.kobj.parent = &chip->devrm.kobj;
> 
> In the worst case the kref the cdev grabs is not released until after
> tpm_chip_unregister() returns.

chip_unregister doesn't tear down either device.  It's the final
release of the chip->dev that does that.  chip->devrm is simply a
subordinate in that process, which is why it doesn't need to be
separately managed.  We have to be careful to call cdev_del() before
device_del on devrm, but we do that, so we're guaranteed no visible
references by the time the chip->dev release is called.

> Having a kref that doesn't work is just asking for trouble, please
> make it work properly.

Actually, as shown above, these krefs are managed ... However, they're
not actually what holds the tpm module in place.  The try_module_get on
open via the owner field does that.  So, by the time tpm_exit() is
called we know there are no devrm references simply because we manage
the cdevrm entity as well.

Now there is a related problem that the owner is actually the *wrong*
module: it holds the tpm module in place not the actual driver module,
so I can happily attach tcsd to the TPM device then rmmod tpm_tis,
which causes some interesting issues.  I can fix this, but it's not a
problem of the current patch.

James

[toc] | [prev] | [next] | [standalone]

#1558754 — Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager via a device link /dev/tpms<n>

On Fri, Jan 13, 2017 at 12:02:36PM -0800, James Bottomley wrote:
> > > Actually, no, the devrm is a completely lifetime managed device as
> > > part
> > > of the chip structure.  once you've done a device_del on it, it can
> > > be
> > > kfreed because it's no longer visible to anything else.
> > 
> > No, that isn't enough. Anything else could have obtained a kref on
> > devrm outside of the sphere the device_del manages.
> > 
> > For instance, the cdev does exactly that, via this:
> > 
> > >  	chip->cdev.kobj.parent = &chip->dev.kobj;
> > > +	chip->cdevrm.kobj.parent = &chip->devrm.kobj;
> > 
> > In the worst case the kref the cdev grabs is not released until after
> > tpm_chip_unregister() returns.
> 
> chip_unregister doesn't tear down either device. It's the final
> release of the chip->dev that does that.

I don't think you are seeing the problem.

I think you are assuming cdev_del waits for userspace to close any
open fds. It does not.

Instead cdev independently holds on to a module lock and a kref on the
chip, once userspace has done close() then the kref is dropped and the
module lock let go. This can happen after chip_unregister, after devm
has done the final put_device, and after the driver core has completed
driver detach.

This is why it is necessary for this:

 +	chip->cdevrm.kobj.parent = &chip->devrm.kobj;

To point to a working kref, such as chip->dev.kobj.

My point is this patch has two very subtle bugs caused by devrm having
a broken kref. Yes we can fix those two cases, but this entire class
of bugs is prevented if devrm has a release function that does
put_device(chip->dev).

We have no idea what will happen down the road; most poeple assume
krefs *work*, having a struct with 4 krefs where only 3 work is pretty
wild, IMHO.

> Now there is a related problem that the owner is actually the *wrong*
> module: it holds the tpm module in place not the actual driver module,
> so I can happily attach tcsd to the TPM device then rmmod tpm_tis,
> which causes some interesting issues.  I can fix this, but it's not a
> problem of the current patch.

No, it is correct as is. The cdev fops rely only on the tpm module.
When tpm_chip_unregister returns to the driver the chips->ops is set
to NULL with proper locking - the driver code becomes uncallable at
that point.

Jason

[toc] | [prev] | [next] | [standalone]

#1558815 — Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager via a device link /dev/tpms<n>

On Fri, 2017-01-13 at 14:23 -0700, Jason Gunthorpe wrote:
> On Fri, Jan 13, 2017 at 12:02:36PM -0800, James Bottomley wrote:
> > > > Actually, no, the devrm is a completely lifetime managed device 
> > > > as part of the chip structure.  once you've done a device_del 
> > > > on it, it can be kfreed because it's no longer visible to
> > > > anything else.
> > > 
> > > No, that isn't enough. Anything else could have obtained a kref 
> > > on devrm outside of the sphere the device_del manages.
> > > 
> > > For instance, the cdev does exactly that, via this:
> > > 
> > > >  	chip->cdev.kobj.parent = &chip->dev.kobj;
> > > > +	chip->cdevrm.kobj.parent = &chip->devrm.kobj;
> > > 
> > > In the worst case the kref the cdev grabs is not released until 
> > > after tpm_chip_unregister() returns.
> > 
> > chip_unregister doesn't tear down either device. It's the final
> > release of the chip->dev that does that.
> 
> I don't think you are seeing the problem.
> 
> I think you are assuming cdev_del waits for userspace to close any
> open fds. It does not.
> 
> Instead cdev independently holds on to a module lock and a kref on 
> the chip, once userspace has done close() then the kref is dropped 
> and the module lock let go. This can happen after chip_unregister, 
> after devm has done the final put_device, and after the driver core 
> has completed driver detach.
> 
> This is why it is necessary for this:
> 
>  +	chip->cdevrm.kobj.parent = &chip->devrm.kobj;
> 
> To point to a working kref, such as chip->dev.kobj.
> 
> My point is this patch has two very subtle bugs caused by devrm 
> having a broken kref. Yes we can fix those two cases, but this entire 
> class of bugs is prevented if devrm has a release function that does
> put_device(chip->dev).
> 
> We have no idea what will happen down the road; most poeple assume
> krefs *work*, having a struct with 4 krefs where only 3 work is 
> pretty wild, IMHO.
> 
> > Now there is a related problem that the owner is actually the 
> > *wrong* module: it holds the tpm module in place not the actual 
> > driver module, so I can happily attach tcsd to the TPM device then 
> > rmmod tpm_tis, which causes some interesting issues.  I can fix 
> > this, but it's not a problem of the current patch.
> 
> No, it is correct as is. The cdev fops rely only on the tpm module.
> When tpm_chip_unregister returns to the driver the chips->ops is set
> to NULL with proper locking - the driver code becomes uncallable at
> that point.

So that's the nastily subtle point I was missing.  The patch below will
add the reference tracking to make sure the chip is held while the
/dev/tpms<n> is open.  I really hate the additional layer of subtlety
this adds, though ... I've tried to document the extra nasties, but I
bet they confuse someone.

Doing it the standard way, where the owner is set to the pdev driver
module and the references all track up to the pdev, meaning the actual
device could never go away while a cdev file was open would be far
easier to understand.

James

---


diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 07d7607..338592c 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -132,6 +132,14 @@ static void tpm_dev_release(struct device *dev)
 	kfree(chip);
 }
 
+static void tpm_devrm_release(struct device *dev)
+{
+	struct tpm_chip *chip = container_of(dev, struct tpm_chip, devrm);
+
+	/* 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
@@ -178,6 +186,12 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
 
 	chip->devrm.parent = pdev;
 	chip->devrm.class = tpm_rm_class;
+	chip->devrm.release = tpm_devrm_release;
+	/* get extra reference on main device to hold on
+	 * behalf of devrm.  This holds the chip structure
+	 * while cdevrm is in use.  The corresponding put
+	 * is in the tpm_devrm_release */
+	get_device(&chip->dev);
 
 	if (chip->dev_num == 0)
 		chip->dev.devt = MKDEV(MISC_MAJOR, TPM_MINOR);
@@ -207,6 +221,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
 	return chip;
 
 out:
+	put_device(&chip->devrm);
 	put_device(&chip->dev);
 	return ERR_PTR(rc);
 }
@@ -323,6 +338,9 @@ 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 devrm reference to the chip->dev unless
+	 * something has cdevrm open */
+	put_device(&chip->devrm);
 }
 
 static void tpm_del_legacy_sysfs(struct tpm_chip *chip)

[toc] | [prev] | [next] | [standalone]

#1559899 — Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager via a device link /dev/tpms<n>

On Fri, Jan 13, 2017 at 05:10:30PM -0800, James Bottomley wrote:

> > No, it is correct as is. The cdev fops rely only on the tpm module.
> > When tpm_chip_unregister returns to the driver the chips->ops is set
> > to NULL with proper locking - the driver code becomes uncallable at
> > that point.
>
> So that's the nastily subtle point I was missing.  The patch below will
> add the reference tracking to make sure the chip is held while the
> /dev/tpms<n> is open.  I really hate the additional layer of subtlety
> this adds, though ... I've tried to document the extra nasties, but I
> bet they confuse someone.

Thanks

> Doing it the standard way, where the owner is set to the pdev driver
> module and the references all track up to the pdev, meaning the actual
> device could never go away while a cdev file was open would be far
> easier to understand.

For a long time now devices can be unbound without a module unload. Eg the
'unbind' file in sysfs. Subsystems can no longer rely on module locking to
block unbind.

vtpm makes use of hot-unplug when the server side is closed. We
audited this area extensively when vtpm was introduced to make it work
properly.

Not all subsystems are correct in this regard, and you may still find
vestigial assumptions that module locking is enough to fix
locking/refcounting sins. It is not, those places are just broken. :(

Jason

[toc] | [prev] | [next] | [standalone]

#1557755 — Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager via a device link /dev/tpms<n>

On Thu, 2017-01-12 at 19:46 +0200, Jarkko Sakkinen wrote:
> From: James Bottomley <James.Bottomley@HansenPartnership.com>
> 
> Currently the Resource Manager (RM) is 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 RM.
> 
> 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.

There's a bug in this code that will crash on first command.  This is
the incremental fix.  It must have got lost when I did the split.

James

---

diff --git a/drivers/char/tpm/tpms-dev.c b/drivers/char/tpm/tpms-dev.c
index f4cb7a3..3eb5955 100644
--- a/drivers/char/tpm/tpms-dev.c
+++ b/drivers/char/tpm/tpms-dev.c
@@ -20,6 +20,11 @@ static int tpms_open(struct inode *inode, struct file *file)
 	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
 	if (priv == NULL)
 		return -ENOMEM;
+	priv->space.context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
+	if (priv->space.context_buf == NULL) {
+		kfree(priv);
+		return -ENOMEM;
+	}
 
 	tpm_common_open(file, chip, &priv->priv);
 

[toc] | [prev] | [next] | [standalone]

#1557804 — Re: [PATCH RFC v2 5/5] tpm2: expose resource manager via a device link /dev/tpms<n>

On Thu, Jan 12, 2017 at 07:46:08PM +0200, Jarkko Sakkinen wrote:
> From: James Bottomley <James.Bottomley@HansenPartnership.com>
> 
> Currently the Resource Manager (RM) is 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 RM.
> 
> 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.
> 
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>

I think you should talk about TPM spaces here.

> ---
>  drivers/char/tpm/Makefile        |  2 +-
>  drivers/char/tpm/tpm-chip.c      | 54 ++++++++++++++++++++++++++++++++++---
>  drivers/char/tpm/tpm-interface.c | 13 +++++++--
>  drivers/char/tpm/tpm.h           |  6 +++--
>  drivers/char/tpm/tpms-dev.c      | 57 ++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 124 insertions(+), 8 deletions(-)
>  create mode 100644 drivers/char/tpm/tpms-dev.c
> 
> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
> index 13ff5da..e50d768 100644
> --- a/drivers/char/tpm/Makefile
> +++ b/drivers/char/tpm/Makefile
> @@ -3,7 +3,7 @@
>  #
>  obj-$(CONFIG_TCG_TPM) += tpm.o
>  tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
> -	 tpm_eventlog.o tpm2-space.o tpm-dev-common.o
> +	 tpm_eventlog.o tpm2-space.o tpm-dev-common.o tpms-dev.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..0d2be04 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 *tpm_rm_class;

They belong to the same device class.

>  dev_t tpm_devt;

But they should have different major device numbers.

/Jarkko

[toc] | [prev] | [next] | [standalone]

#1558614 — Re: [PATCH RFC v2 5/5] tpm2: expose resource manager via a device link /dev/tpms<n>

On Thu, Jan 12, 2017 at 10:56:28PM +0200, Jarkko Sakkinen wrote:

> >  dev_t tpm_devt;
> 
> But they should have different major device numbers.

major/minors don't really matter these days since they are dynamic

Jason

[toc] | [prev] | [next] | [standalone]

#1558637 — Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager via a device link /dev/tpms<n>

On Fri, 2017-01-13 at 10:25 -0700, Jason Gunthorpe wrote:
> On Thu, Jan 12, 2017 at 10:56:28PM +0200, Jarkko Sakkinen wrote:
> 
> > >  dev_t tpm_devt;
> > 
> > But they should have different major device numbers.
> 
> major/minors don't really matter these days since they are dynamic

Right, although we have this weird piece of code:


	if (chip->dev_num == 0)
		chip->dev.devt = MKDEV(MISC_MAJOR, TPM_MINOR);
	else
		chip->dev.devt = MKDEV(MAJOR(tpm_devt), chip->dev_num);

So the first TPM device gets the MISC_MAJOR with TPM_MINOR and the rest
get the dynamic major/minor.  It means when you do an ls on a complex
system you get something like:

crw------- 1 root root  10,   224 Jan 13 06:21 /dev/tpm0
crw------- 1 root root 246,     1 Jan 13 09:38 /dev/tpm1
crw-rw-rw- 1 root root 246, 65536 Jan 13 06:21 /dev/tpms0
crw-rw-rw- 1 root root 246, 65537 Jan 13 09:38 /dev/tpms1

Perhaps it's time just to junk the reserved misc minor?

James

[toc] | [prev] | [next] | [standalone]

#1558653 — Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager via a device link /dev/tpms<n>

On Fri, Jan 13, 2017 at 09:40:08AM -0800, James Bottomley wrote:
> On Fri, 2017-01-13 at 10:25 -0700, Jason Gunthorpe wrote:
> > On Thu, Jan 12, 2017 at 10:56:28PM +0200, Jarkko Sakkinen wrote:
> > 
> > > >  dev_t tpm_devt;
> > > 
> > > But they should have different major device numbers.
> > 
> > major/minors don't really matter these days since they are dynamic
> 
> Right, although we have this weird piece of code:
> 
> 
> 	if (chip->dev_num == 0)
> 		chip->dev.devt = MKDEV(MISC_MAJOR, TPM_MINOR);
> 	else
> 		chip->dev.devt = MKDEV(MAJOR(tpm_devt), chip->dev_num);
> 
> So the first TPM device gets the MISC_MAJOR with TPM_MINOR and the rest
> get the dynamic major/minor.  It means when you do an ls on a complex
> system you get something like:

The TPM has always used a static major/minor for tpm0 and dynamic for
the following. Originally this used a misc_device and did:

        } else if (chip->dev_num == 0)
                chip->vendor.miscdev.minor = TPM_MINOR;
        else
                chip->vendor.miscdev.minor = MISC_DYNAMIC_MINOR;

When we moved to struct device the !0 tpms got a dynamic major as
well.

I don't really know what the broad kernel feeling is on historical
major/minor stability - this was mainly continuing what had always
been done in this code for pre-udev userspace compatibility.

Does it harm anything?

Jason

[toc] | [prev] | [next] | [standalone]

#1558656 — Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager via a device link /dev/tpms<n>

On Fri, 2017-01-13 at 11:01 -0700, Jason Gunthorpe wrote:
> On Fri, Jan 13, 2017 at 09:40:08AM -0800, James Bottomley wrote:
> > On Fri, 2017-01-13 at 10:25 -0700, Jason Gunthorpe wrote:
> > > On Thu, Jan 12, 2017 at 10:56:28PM +0200, Jarkko Sakkinen wrote:
> > > 
> > > > >  dev_t tpm_devt;
> > > > 
> > > > But they should have different major device numbers.
> > > 
> > > major/minors don't really matter these days since they are
> > > dynamic
> > 
> > Right, although we have this weird piece of code:
> > 
> > 
> > 	if (chip->dev_num == 0)
> > 		chip->dev.devt = MKDEV(MISC_MAJOR, TPM_MINOR);
> > 	else
> > 		chip->dev.devt = MKDEV(MAJOR(tpm_devt), chip->dev_num);
> > 
> > So the first TPM device gets the MISC_MAJOR with TPM_MINOR and the
> > rest
> > get the dynamic major/minor.  It means when you do an ls on a
> > complex
> > system you get something like:
> 
> The TPM has always used a static major/minor for tpm0 and dynamic for
> the following. Originally this used a misc_device and did:
> 
>         } else if (chip->dev_num == 0)
>                 chip->vendor.miscdev.minor = TPM_MINOR;
>         else
>                 chip->vendor.miscdev.minor = MISC_DYNAMIC_MINOR;
> 
> When we moved to struct device the !0 tpms got a dynamic major as
> well.
> 
> I don't really know what the broad kernel feeling is on historical
> major/minor stability - this was mainly continuing what had always
> been done in this code for pre-udev userspace compatibility.
> 
> Does it harm anything?

Devices are either supposed to be static, so you can rely on the
major/minor without needing udev or fully dynamic, so you need udev to
get the nodes and you may not rely on the major/minor number.  The
potential harm is that we're neither one nor the other, which sounds
like an accident waiting to happen if someone decides to rely on our
numbers but gets the wrong node.  Chances are that, since most linux
devices are fully dynamic, no-one ever thinks of relying on fixed
major/minor numbers anymore, so everyone avoids this banana skin and
perhaps it doesn't matter.

James

[toc] | [prev] | [next] | [standalone]

#1559562 — Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager via a device link /dev/tpms<n>

On Fri, Jan 13, 2017 at 09:40:08AM -0800, James Bottomley wrote:
> On Fri, 2017-01-13 at 10:25 -0700, Jason Gunthorpe wrote:
> > On Thu, Jan 12, 2017 at 10:56:28PM +0200, Jarkko Sakkinen wrote:
> > 
> > > >  dev_t tpm_devt;
> > > 
> > > But they should have different major device numbers.
> > 
> > major/minors don't really matter these days since they are dynamic
> 
> Right, although we have this weird piece of code:
> 
> 
> 	if (chip->dev_num == 0)
> 		chip->dev.devt = MKDEV(MISC_MAJOR, TPM_MINOR);
> 	else
> 		chip->dev.devt = MKDEV(MAJOR(tpm_devt), chip->dev_num);
> 
> So the first TPM device gets the MISC_MAJOR with TPM_MINOR and the rest
> get the dynamic major/minor.  It means when you do an ls on a complex
> system you get something like:
> 
> crw------- 1 root root  10,   224 Jan 13 06:21 /dev/tpm0
> crw------- 1 root root 246,     1 Jan 13 09:38 /dev/tpm1
> crw-rw-rw- 1 root root 246, 65536 Jan 13 06:21 /dev/tpms0
> crw-rw-rw- 1 root root 246, 65537 Jan 13 09:38 /dev/tpms1
> 
> Perhaps it's time just to junk the reserved misc minor?

+1 

And Jason is correct about major numbers. I still am puzzled whether
these should share the device class and devt with raw /dev/tpm0.

/Jarkko

[toc] | [prev] | [next] | [standalone]

#1557667 — [PATCH RFC v2 4/5] tpm: split out tpm-dev.c into tpm-dev.c and tpm-common-dev.c

From: James Bottomley <James.Bottomley@HansenPartnership.com>

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/char/tpm/Makefile         |   2 +-
 drivers/char/tpm/tpm-dev-common.c | 145 ++++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm-dev.c        | 141 ++++--------------------------------
 drivers/char/tpm/tpm-dev.h        |  27 +++++++
 4 files changed, 187 insertions(+), 128 deletions(-)
 create mode 100644 drivers/char/tpm/tpm-dev-common.c
 create mode 100644 drivers/char/tpm/tpm-dev.h

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 251d0ed..13ff5da 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -3,7 +3,7 @@
 #
 obj-$(CONFIG_TCG_TPM) += tpm.o
 tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
-	 tpm_eventlog.o tpm2-space.o
+	 tpm_eventlog.o tpm2-space.o tpm-dev-common.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-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
new file mode 100644
index 0000000..0156562
--- /dev/null
+++ b/drivers/char/tpm/tpm-dev-common.c
@@ -0,0 +1,145 @@
+/*
+ * Copyright (C) 2004 IBM Corporation
+ * Authors:
+ * Leendert van Doorn <leendert@watson.ibm.com>
+ * Dave Safford <safford@watson.ibm.com>
+ * Reiner Sailer <sailer@watson.ibm.com>
+ * Kylene Hall <kjhall@us.ibm.com>
+ *
+ * Copyright (C) 2013 Obsidian Research Corp
+ * Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
+ *
+ * Device file system interface to the TPM
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ *
+ */
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include "tpm.h"
+#include "tpm-dev.h"
+
+static void user_reader_timeout(unsigned long ptr)
+{
+	struct file_priv *priv = (struct file_priv *)ptr;
+
+	schedule_work(&priv->work);
+}
+
+static void timeout_work(struct work_struct *work)
+{
+	struct file_priv *priv = container_of(work, struct file_priv, work);
+
+	mutex_lock(&priv->buffer_mutex);
+	atomic_set(&priv->data_pending, 0);
+	memset(priv->data_buffer, 0, sizeof(priv->data_buffer));
+	mutex_unlock(&priv->buffer_mutex);
+}
+
+void tpm_common_open(struct file *file, struct tpm_chip *chip,
+		     struct file_priv *priv)
+{
+	priv->chip = chip;
+	atomic_set(&priv->data_pending, 0);
+	mutex_init(&priv->buffer_mutex);
+	setup_timer(&priv->user_read_timer, user_reader_timeout,
+			(unsigned long)priv);
+	INIT_WORK(&priv->work, timeout_work);
+
+	file->private_data = priv;
+}
+
+ssize_t tpm_common_read(struct file *file, char __user *buf,
+			size_t size, loff_t *off)
+{
+	struct file_priv *priv = file->private_data;
+	ssize_t ret_size;
+	ssize_t orig_ret_size;
+	int rc;
+
+	del_singleshot_timer_sync(&priv->user_read_timer);
+	flush_work(&priv->work);
+	ret_size = atomic_read(&priv->data_pending);
+	if (ret_size > 0) {	/* relay data */
+		orig_ret_size = ret_size;
+		if (size < ret_size)
+			ret_size = size;
+
+		mutex_lock(&priv->buffer_mutex);
+		rc = copy_to_user(buf, priv->data_buffer, ret_size);
+		memset(priv->data_buffer, 0, orig_ret_size);
+		if (rc)
+			ret_size = -EFAULT;
+
+		mutex_unlock(&priv->buffer_mutex);
+	}
+
+	atomic_set(&priv->data_pending, 0);
+
+	return ret_size;
+}
+
+ssize_t tpm_common_write(struct file *file, const char __user *buf,
+			 size_t size, loff_t *off, struct tpm_space *space)
+{
+	struct file_priv *priv = file->private_data;
+	size_t in_size = size;
+	ssize_t out_size;
+
+	/* Cannot perform a write until the read has cleared either via
+	 * tpm_read or a user_read_timer timeout. This also prevents split
+	 * buffered writes from blocking here.
+	 */
+	if (atomic_read(&priv->data_pending) != 0)
+		return -EBUSY;
+
+	if (in_size > TPM_BUFSIZE)
+		return -E2BIG;
+
+	mutex_lock(&priv->buffer_mutex);
+
+	if (copy_from_user
+	    (priv->data_buffer, (void __user *) buf, in_size)) {
+		mutex_unlock(&priv->buffer_mutex);
+		return -EFAULT;
+	}
+
+	/* atomic tpm command send and result receive. We only hold the ops
+	 * lock during this period so that the tpm can be unregistered even if
+	 * the char dev is held open.
+	 */
+	if (tpm_try_get_ops(priv->chip)) {
+		mutex_unlock(&priv->buffer_mutex);
+		return -EPIPE;
+	}
+	out_size = tpm_transmit(priv->chip, space, priv->data_buffer,
+				sizeof(priv->data_buffer), 0);
+
+	tpm_put_ops(priv->chip);
+	if (out_size < 0) {
+		mutex_unlock(&priv->buffer_mutex);
+		return out_size;
+	}
+
+	atomic_set(&priv->data_pending, out_size);
+	mutex_unlock(&priv->buffer_mutex);
+
+	/* Set a timeout by which the reader must come claim the result */
+	mod_timer(&priv->user_read_timer, jiffies + (60 * HZ));
+
+	return in_size;
+}
+
+/*
+ * Called on file close
+ */
+void tpm_common_release(struct file *file, struct file_priv *priv)
+{
+	del_singleshot_timer_sync(&priv->user_read_timer);
+	flush_work(&priv->work);
+	file->private_data = NULL;
+	atomic_set(&priv->data_pending, 0);
+}
diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
index 249eeb0..1a8d325 100644
--- a/drivers/char/tpm/tpm-dev.c
+++ b/drivers/char/tpm/tpm-dev.c
@@ -18,45 +18,15 @@
  *
  */
 #include <linux/slab.h>
-#include <linux/uaccess.h>
-#include "tpm.h"
-
-struct file_priv {
-	struct tpm_chip *chip;
-
-	/* Data passed to and from the tpm via the read/write calls */
-	atomic_t data_pending;
-	struct mutex buffer_mutex;
-
-	struct timer_list user_read_timer;      /* user needs to claim result */
-	struct work_struct work;
-
-	u8 data_buffer[TPM_BUFSIZE];
-};
-
-static void user_reader_timeout(unsigned long ptr)
-{
-	struct file_priv *priv = (struct file_priv *)ptr;
-
-	schedule_work(&priv->work);
-}
-
-static void timeout_work(struct work_struct *work)
-{
-	struct file_priv *priv = container_of(work, struct file_priv, work);
-
-	mutex_lock(&priv->buffer_mutex);
-	atomic_set(&priv->data_pending, 0);
-	memset(priv->data_buffer, 0, sizeof(priv->data_buffer));
-	mutex_unlock(&priv->buffer_mutex);
-}
+#include "tpm-dev.h"
 
 static int tpm_open(struct inode *inode, struct file *file)
 {
-	struct tpm_chip *chip =
-		container_of(inode->i_cdev, struct tpm_chip, cdev);
+	struct tpm_chip *chip;
 	struct file_priv *priv;
 
+	chip = container_of(inode->i_cdev, struct tpm_chip, cdev);
+
 	/* It's assured that the chip will be opened just once,
 	 * by the check of is_open variable, which is protected
 	 * by driver_lock. */
@@ -66,100 +36,22 @@ static int tpm_open(struct inode *inode, struct file *file)
 	}
 
 	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-	if (priv == NULL) {
-		clear_bit(0, &chip->is_open);
-		return -ENOMEM;
-	}
+	if (priv == NULL)
+		goto out;
 
-	priv->chip = chip;
-	atomic_set(&priv->data_pending, 0);
-	mutex_init(&priv->buffer_mutex);
-	setup_timer(&priv->user_read_timer, user_reader_timeout,
-			(unsigned long)priv);
-	INIT_WORK(&priv->work, timeout_work);
+	tpm_common_open(file, chip, priv);
 
-	file->private_data = priv;
 	return 0;
-}
-
-static ssize_t tpm_read(struct file *file, char __user *buf,
-			size_t size, loff_t *off)
-{
-	struct file_priv *priv = file->private_data;
-	ssize_t ret_size;
-	int rc;
-
-	del_singleshot_timer_sync(&priv->user_read_timer);
-	flush_work(&priv->work);
-	ret_size = atomic_read(&priv->data_pending);
-	if (ret_size > 0) {	/* relay data */
-		ssize_t orig_ret_size = ret_size;
-		if (size < ret_size)
-			ret_size = size;
 
-		mutex_lock(&priv->buffer_mutex);
-		rc = copy_to_user(buf, priv->data_buffer, ret_size);
-		memset(priv->data_buffer, 0, orig_ret_size);
-		if (rc)
-			ret_size = -EFAULT;
-
-		mutex_unlock(&priv->buffer_mutex);
-	}
-
-	atomic_set(&priv->data_pending, 0);
-
-	return ret_size;
+ out:
+	clear_bit(0, &chip->is_open);
+	return -ENOMEM;
 }
 
 static ssize_t tpm_write(struct file *file, const char __user *buf,
 			 size_t size, loff_t *off)
 {
-	struct file_priv *priv = file->private_data;
-	size_t in_size = size;
-	ssize_t out_size;
-
-	/* cannot perform a write until the read has cleared
-	   either via tpm_read or a user_read_timer timeout.
-	   This also prevents splitted buffered writes from blocking here.
-	*/
-	if (atomic_read(&priv->data_pending) != 0)
-		return -EBUSY;
-
-	if (in_size > TPM_BUFSIZE)
-		return -E2BIG;
-
-	mutex_lock(&priv->buffer_mutex);
-
-	if (copy_from_user
-	    (priv->data_buffer, (void __user *) buf, in_size)) {
-		mutex_unlock(&priv->buffer_mutex);
-		return -EFAULT;
-	}
-
-	/* atomic tpm command send and result receive. We only hold the ops
-	 * lock during this period so that the tpm can be unregistered even if
-	 * the char dev is held open.
-	 */
-	if (tpm_try_get_ops(priv->chip)) {
-		mutex_unlock(&priv->buffer_mutex);
-		return -EPIPE;
-	}
-	out_size = tpm_transmit(priv->chip, NULL, priv->data_buffer,
-				sizeof(priv->data_buffer), 0);
-
-	tpm_put_ops(priv->chip);
-	if (out_size < 0) {
-		mutex_unlock(&priv->buffer_mutex);
-		return out_size;
-	}
-
-	atomic_set(&priv->data_pending, out_size);
-	mutex_unlock(&priv->buffer_mutex);
-
-	/* Set a timeout by which the reader must come claim the result */
-	mod_timer(&priv->user_read_timer, jiffies + (60 * HZ));
-
-	return in_size;
+	return tpm_common_write(file, buf, size, off, NULL);
 }
 
 /*
@@ -169,12 +61,9 @@ static int tpm_release(struct inode *inode, struct file *file)
 {
 	struct file_priv *priv = file->private_data;
 
-	del_singleshot_timer_sync(&priv->user_read_timer);
-	flush_work(&priv->work);
-	file->private_data = NULL;
-	atomic_set(&priv->data_pending, 0);
-	clear_bit(0, &priv->chip->is_open);
+	tpm_common_release(file, priv);
 	kfree(priv);
+
 	return 0;
 }
 
@@ -182,9 +71,7 @@ const struct file_operations tpm_fops = {
 	.owner = THIS_MODULE,
 	.llseek = no_llseek,
 	.open = tpm_open,
-	.read = tpm_read,
+	.read = tpm_common_read,
 	.write = tpm_write,
 	.release = tpm_release,
 };
-
-
diff --git a/drivers/char/tpm/tpm-dev.h b/drivers/char/tpm/tpm-dev.h
new file mode 100644
index 0000000..ff15cf7
--- /dev/null
+++ b/drivers/char/tpm/tpm-dev.h
@@ -0,0 +1,27 @@
+#ifndef _TPM_DEV_H
+#define _TPM_DEV_H
+
+#include "tpm.h"
+
+struct file_priv {
+	struct tpm_chip *chip;
+
+	/* Data passed to and from the tpm via the read/write calls */
+	atomic_t data_pending;
+	struct mutex buffer_mutex;
+
+	struct timer_list user_read_timer;      /* user needs to claim result */
+	struct work_struct work;
+
+	u8 data_buffer[TPM_BUFSIZE];
+};
+
+void tpm_common_open(struct file *file, struct tpm_chip *chip,
+		     struct file_priv *priv);
+ssize_t tpm_common_read(struct file *file, char __user *buf,
+			size_t size, loff_t *off);
+ssize_t tpm_common_write(struct file *file, const char __user *buf,
+			 size_t size, loff_t *off, struct tpm_space *space);
+void tpm_common_release(struct file *file, struct file_priv *priv);
+
+#endif
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1558694 — Re: [tpmdd-devel] [PATCH RFC v2 4/5] tpm: split out tpm-dev.c into tpm-dev.c and tpm-common-dev.c

This version has a bug in that it doesn't do a clear_bit(0, &chip
->is_open) on release. so fixed that

James

---

From c006f181988dd2fd54e5a84f3e4a6dc0157c96f5 Mon Sep 17 00:00:00 2001
From: James Bottomley <James.Bottomley@HansenPartnership.com>
Date: Tue, 10 Jan 2017 19:08:53 -0800
Subject: [PATCH 4/5] tpm: split out tpm-dev.c into tpm-dev.c and
 tpm-common-dev.c

In preparation for adding a space management device, split out all of
the common routines that would be shareable between these device types
into a common file.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/char/tpm/Makefile         |   2 +-
 drivers/char/tpm/tpm-dev-common.c | 145 ++++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm-dev.c        | 142 +++++--------------------------------
 drivers/char/tpm/tpm-dev.h        |  27 +++++++
 4 files changed, 189 insertions(+), 127 deletions(-)
 create mode 100644 drivers/char/tpm/tpm-dev-common.c
 create mode 100644 drivers/char/tpm/tpm-dev.h

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 251d0ed..13ff5da 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -3,7 +3,7 @@
 #
 obj-$(CONFIG_TCG_TPM) += tpm.o
 tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
-	 tpm_eventlog.o tpm2-space.o
+	 tpm_eventlog.o tpm2-space.o tpm-dev-common.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-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
new file mode 100644
index 0000000..0156562
--- /dev/null
+++ b/drivers/char/tpm/tpm-dev-common.c
@@ -0,0 +1,145 @@
+/*
+ * Copyright (C) 2004 IBM Corporation
+ * Authors:
+ * Leendert van Doorn <leendert@watson.ibm.com>
+ * Dave Safford <safford@watson.ibm.com>
+ * Reiner Sailer <sailer@watson.ibm.com>
+ * Kylene Hall <kjhall@us.ibm.com>
+ *
+ * Copyright (C) 2013 Obsidian Research Corp
+ * Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
+ *
+ * Device file system interface to the TPM
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ *
+ */
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include "tpm.h"
+#include "tpm-dev.h"
+
+static void user_reader_timeout(unsigned long ptr)
+{
+	struct file_priv *priv = (struct file_priv *)ptr;
+
+	schedule_work(&priv->work);
+}
+
+static void timeout_work(struct work_struct *work)
+{
+	struct file_priv *priv = container_of(work, struct file_priv, work);
+
+	mutex_lock(&priv->buffer_mutex);
+	atomic_set(&priv->data_pending, 0);
+	memset(priv->data_buffer, 0, sizeof(priv->data_buffer));
+	mutex_unlock(&priv->buffer_mutex);
+}
+
+void tpm_common_open(struct file *file, struct tpm_chip *chip,
+		     struct file_priv *priv)
+{
+	priv->chip = chip;
+	atomic_set(&priv->data_pending, 0);
+	mutex_init(&priv->buffer_mutex);
+	setup_timer(&priv->user_read_timer, user_reader_timeout,
+			(unsigned long)priv);
+	INIT_WORK(&priv->work, timeout_work);
+
+	file->private_data = priv;
+}
+
+ssize_t tpm_common_read(struct file *file, char __user *buf,
+			size_t size, loff_t *off)
+{
+	struct file_priv *priv = file->private_data;
+	ssize_t ret_size;
+	ssize_t orig_ret_size;
+	int rc;
+
+	del_singleshot_timer_sync(&priv->user_read_timer);
+	flush_work(&priv->work);
+	ret_size = atomic_read(&priv->data_pending);
+	if (ret_size > 0) {	/* relay data */
+		orig_ret_size = ret_size;
+		if (size < ret_size)
+			ret_size = size;
+
+		mutex_lock(&priv->buffer_mutex);
+		rc = copy_to_user(buf, priv->data_buffer, ret_size);
+		memset(priv->data_buffer, 0, orig_ret_size);
+		if (rc)
+			ret_size = -EFAULT;
+
+		mutex_unlock(&priv->buffer_mutex);
+	}
+
+	atomic_set(&priv->data_pending, 0);
+
+	return ret_size;
+}
+
+ssize_t tpm_common_write(struct file *file, const char __user *buf,
+			 size_t size, loff_t *off, struct tpm_space *space)
+{
+	struct file_priv *priv = file->private_data;
+	size_t in_size = size;
+	ssize_t out_size;
+
+	/* Cannot perform a write until the read has cleared either via
+	 * tpm_read or a user_read_timer timeout. This also prevents split
+	 * buffered writes from blocking here.
+	 */
+	if (atomic_read(&priv->data_pending) != 0)
+		return -EBUSY;
+
+	if (in_size > TPM_BUFSIZE)
+		return -E2BIG;
+
+	mutex_lock(&priv->buffer_mutex);
+
+	if (copy_from_user
+	    (priv->data_buffer, (void __user *) buf, in_size)) {
+		mutex_unlock(&priv->buffer_mutex);
+		return -EFAULT;
+	}
+
+	/* atomic tpm command send and result receive. We only hold the ops
+	 * lock during this period so that the tpm can be unregistered even if
+	 * the char dev is held open.
+	 */
+	if (tpm_try_get_ops(priv->chip)) {
+		mutex_unlock(&priv->buffer_mutex);
+		return -EPIPE;
+	}
+	out_size = tpm_transmit(priv->chip, space, priv->data_buffer,
+				sizeof(priv->data_buffer), 0);
+
+	tpm_put_ops(priv->chip);
+	if (out_size < 0) {
+		mutex_unlock(&priv->buffer_mutex);
+		return out_size;
+	}
+
+	atomic_set(&priv->data_pending, out_size);
+	mutex_unlock(&priv->buffer_mutex);
+
+	/* Set a timeout by which the reader must come claim the result */
+	mod_timer(&priv->user_read_timer, jiffies + (60 * HZ));
+
+	return in_size;
+}
+
+/*
+ * Called on file close
+ */
+void tpm_common_release(struct file *file, struct file_priv *priv)
+{
+	del_singleshot_timer_sync(&priv->user_read_timer);
+	flush_work(&priv->work);
+	file->private_data = NULL;
+	atomic_set(&priv->data_pending, 0);
+}
diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
index 249eeb0..92fc2c6 100644
--- a/drivers/char/tpm/tpm-dev.c
+++ b/drivers/char/tpm/tpm-dev.c
@@ -18,45 +18,15 @@
  *
  */
 #include <linux/slab.h>
-#include <linux/uaccess.h>
-#include "tpm.h"
-
-struct file_priv {
-	struct tpm_chip *chip;
-
-	/* Data passed to and from the tpm via the read/write calls */
-	atomic_t data_pending;
-	struct mutex buffer_mutex;
-
-	struct timer_list user_read_timer;      /* user needs to claim result */
-	struct work_struct work;
-
-	u8 data_buffer[TPM_BUFSIZE];
-};
-
-static void user_reader_timeout(unsigned long ptr)
-{
-	struct file_priv *priv = (struct file_priv *)ptr;
-
-	schedule_work(&priv->work);
-}
-
-static void timeout_work(struct work_struct *work)
-{
-	struct file_priv *priv = container_of(work, struct file_priv, work);
-
-	mutex_lock(&priv->buffer_mutex);
-	atomic_set(&priv->data_pending, 0);
-	memset(priv->data_buffer, 0, sizeof(priv->data_buffer));
-	mutex_unlock(&priv->buffer_mutex);
-}
+#include "tpm-dev.h"
 
 static int tpm_open(struct inode *inode, struct file *file)
 {
-	struct tpm_chip *chip =
-		container_of(inode->i_cdev, struct tpm_chip, cdev);
+	struct tpm_chip *chip;
 	struct file_priv *priv;
 
+	chip = container_of(inode->i_cdev, struct tpm_chip, cdev);
+
 	/* It's assured that the chip will be opened just once,
 	 * by the check of is_open variable, which is protected
 	 * by driver_lock. */
@@ -66,100 +36,22 @@ static int tpm_open(struct inode *inode, struct file *file)
 	}
 
 	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-	if (priv == NULL) {
-		clear_bit(0, &chip->is_open);
-		return -ENOMEM;
-	}
+	if (priv == NULL)
+		goto out;
 
-	priv->chip = chip;
-	atomic_set(&priv->data_pending, 0);
-	mutex_init(&priv->buffer_mutex);
-	setup_timer(&priv->user_read_timer, user_reader_timeout,
-			(unsigned long)priv);
-	INIT_WORK(&priv->work, timeout_work);
+	tpm_common_open(file, chip, priv);
 
-	file->private_data = priv;
 	return 0;
-}
-
-static ssize_t tpm_read(struct file *file, char __user *buf,
-			size_t size, loff_t *off)
-{
-	struct file_priv *priv = file->private_data;
-	ssize_t ret_size;
-	int rc;
-
-	del_singleshot_timer_sync(&priv->user_read_timer);
-	flush_work(&priv->work);
-	ret_size = atomic_read(&priv->data_pending);
-	if (ret_size > 0) {	/* relay data */
-		ssize_t orig_ret_size = ret_size;
-		if (size < ret_size)
-			ret_size = size;
-
-		mutex_lock(&priv->buffer_mutex);
-		rc = copy_to_user(buf, priv->data_buffer, ret_size);
-		memset(priv->data_buffer, 0, orig_ret_size);
-		if (rc)
-			ret_size = -EFAULT;
 
-		mutex_unlock(&priv->buffer_mutex);
-	}
-
-	atomic_set(&priv->data_pending, 0);
-
-	return ret_size;
+ out:
+	clear_bit(0, &chip->is_open);
+	return -ENOMEM;
 }
 
 static ssize_t tpm_write(struct file *file, const char __user *buf,
 			 size_t size, loff_t *off)
 {
-	struct file_priv *priv = file->private_data;
-	size_t in_size = size;
-	ssize_t out_size;
-
-	/* cannot perform a write until the read has cleared
-	   either via tpm_read or a user_read_timer timeout.
-	   This also prevents splitted buffered writes from blocking here.
-	*/
-	if (atomic_read(&priv->data_pending) != 0)
-		return -EBUSY;
-
-	if (in_size > TPM_BUFSIZE)
-		return -E2BIG;
-
-	mutex_lock(&priv->buffer_mutex);
-
-	if (copy_from_user
-	    (priv->data_buffer, (void __user *) buf, in_size)) {
-		mutex_unlock(&priv->buffer_mutex);
-		return -EFAULT;
-	}
-
-	/* atomic tpm command send and result receive. We only hold the ops
-	 * lock during this period so that the tpm can be unregistered even if
-	 * the char dev is held open.
-	 */
-	if (tpm_try_get_ops(priv->chip)) {
-		mutex_unlock(&priv->buffer_mutex);
-		return -EPIPE;
-	}
-	out_size = tpm_transmit(priv->chip, NULL, priv->data_buffer,
-				sizeof(priv->data_buffer), 0);
-
-	tpm_put_ops(priv->chip);
-	if (out_size < 0) {
-		mutex_unlock(&priv->buffer_mutex);
-		return out_size;
-	}
-
-	atomic_set(&priv->data_pending, out_size);
-	mutex_unlock(&priv->buffer_mutex);
-
-	/* Set a timeout by which the reader must come claim the result */
-	mod_timer(&priv->user_read_timer, jiffies + (60 * HZ));
-
-	return in_size;
+	return tpm_common_write(file, buf, size, off, NULL);
 }
 
 /*
@@ -169,12 +61,12 @@ static int tpm_release(struct inode *inode, struct file *file)
 {
 	struct file_priv *priv = file->private_data;
 
-	del_singleshot_timer_sync(&priv->user_read_timer);
-	flush_work(&priv->work);
-	file->private_data = NULL;
-	atomic_set(&priv->data_pending, 0);
+	tpm_common_release(file, priv);
+
 	clear_bit(0, &priv->chip->is_open);
+
 	kfree(priv);
+
 	return 0;
 }
 
@@ -182,9 +74,7 @@ const struct file_operations tpm_fops = {
 	.owner = THIS_MODULE,
 	.llseek = no_llseek,
 	.open = tpm_open,
-	.read = tpm_read,
+	.read = tpm_common_read,
 	.write = tpm_write,
 	.release = tpm_release,
 };
-
-
diff --git a/drivers/char/tpm/tpm-dev.h b/drivers/char/tpm/tpm-dev.h
new file mode 100644
index 0000000..ff15cf7
--- /dev/null
+++ b/drivers/char/tpm/tpm-dev.h
@@ -0,0 +1,27 @@
+#ifndef _TPM_DEV_H
+#define _TPM_DEV_H
+
+#include "tpm.h"
+
+struct file_priv {
+	struct tpm_chip *chip;
+
+	/* Data passed to and from the tpm via the read/write calls */
+	atomic_t data_pending;
+	struct mutex buffer_mutex;
+
+	struct timer_list user_read_timer;      /* user needs to claim result */
+	struct work_struct work;
+
+	u8 data_buffer[TPM_BUFSIZE];
+};
+
+void tpm_common_open(struct file *file, struct tpm_chip *chip,
+		     struct file_priv *priv);
+ssize_t tpm_common_read(struct file *file, char __user *buf,
+			size_t size, loff_t *off);
+ssize_t tpm_common_write(struct file *file, const char __user *buf,
+			 size_t size, loff_t *off, struct tpm_space *space);
+void tpm_common_release(struct file *file, struct file_priv *priv);
+
+#endif
-- 
2.6.6

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web