| 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: <20160315055814.GA4947@intel.com>
Date: Tue, 15 Mar 2016 07:58:14 +0200
From: Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
To: Stefan Berger <stefanb@...ux.vnet.ibm.com>
Cc: tpmdd-devel@...ts.sourceforge.net, jgunthorpe@...idianresearch.com,
linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v8 06/10] tpm: Replace device number bitmap with IDR
On Sun, Mar 13, 2016 at 06:54:36PM -0400, Stefan Berger wrote:
> Replace the device number bitmap with IDR. Extend the number of devices we
> can create to 64k.
> Since an IDR allows us to associate a pointer with an ID, we use this now
> to rewrite tpm_chip_find_get() to simply look up the chip pointer by the
> given device ID.
>
> Protect the IDR calls with a mutex.
I've merged patches up to this to my next branch i.e. you don't have
to carry them in the seres anymore.
/Jarkko
> Signed-off-by: Stefan Berger <stefanb@...ux.vnet.ibm.com>
> Reviewed-by: Jason Gunthorpe <jgunthorpe@...idianresearch.com>
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
> Tested-by: Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
> ---
> drivers/char/tpm/tpm-chip.c | 84 +++++++++++++++++++++-------------------
> drivers/char/tpm/tpm-interface.c | 1 +
> drivers/char/tpm/tpm.h | 5 +--
> 3 files changed, 48 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 5880377..f62c851 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -29,9 +29,8 @@
> #include "tpm.h"
> #include "tpm_eventlog.h"
>
> -static DECLARE_BITMAP(dev_mask, TPM_NUM_DEVICES);
> -static LIST_HEAD(tpm_chip_list);
> -static DEFINE_SPINLOCK(driver_lock);
> +DEFINE_IDR(dev_nums_idr);
> +static DEFINE_MUTEX(idr_lock);
>
> struct class *tpm_class;
> dev_t tpm_devt;
> @@ -88,20 +87,30 @@ EXPORT_SYMBOL_GPL(tpm_put_ops);
> */
> struct tpm_chip *tpm_chip_find_get(int chip_num)
> {
> - struct tpm_chip *pos, *chip = NULL;
> + struct tpm_chip *chip, *res = NULL;
> + int chip_prev;
> +
> + mutex_lock(&idr_lock);
> +
> + if (chip_num == TPM_ANY_NUM) {
> + chip_num = 0;
> + do {
> + chip_prev = chip_num;
> + chip = idr_get_next(&dev_nums_idr, &chip_num);
> + if (chip && !tpm_try_get_ops(chip)) {
> + res = chip;
> + break;
> + }
> + } while (chip_prev != chip_num);
> + } else {
> + chip = idr_find_slowpath(&dev_nums_idr, chip_num);
> + if (chip && !tpm_try_get_ops(chip))
> + res = chip;
> + }
>
> - rcu_read_lock();
> - list_for_each_entry_rcu(pos, &tpm_chip_list, list) {
> - if (chip_num != TPM_ANY_NUM && chip_num != pos->dev_num)
> - continue;
> + mutex_unlock(&idr_lock);
>
> - /* rcu prevents chip from being free'd */
> - if (!tpm_try_get_ops(pos))
> - chip = pos;
> - break;
> - }
> - rcu_read_unlock();
> - return chip;
> + return res;
> }
>
> /**
> @@ -114,9 +123,10 @@ static void tpm_dev_release(struct device *dev)
> {
> struct tpm_chip *chip = container_of(dev, struct tpm_chip, dev);
>
> - spin_lock(&driver_lock);
> - clear_bit(chip->dev_num, dev_mask);
> - spin_unlock(&driver_lock);
> + mutex_lock(&idr_lock);
> + idr_remove(&dev_nums_idr, chip->dev_num);
> + mutex_unlock(&idr_lock);
> +
> kfree(chip);
> }
>
> @@ -142,21 +152,18 @@ struct tpm_chip *tpm_chip_alloc(struct device *dev,
>
> mutex_init(&chip->tpm_mutex);
> init_rwsem(&chip->ops_sem);
> - INIT_LIST_HEAD(&chip->list);
>
> chip->ops = ops;
>
> - spin_lock(&driver_lock);
> - chip->dev_num = find_first_zero_bit(dev_mask, TPM_NUM_DEVICES);
> - spin_unlock(&driver_lock);
> -
> - if (chip->dev_num >= TPM_NUM_DEVICES) {
> + mutex_lock(&idr_lock);
> + rc = idr_alloc(&dev_nums_idr, NULL, 0, TPM_NUM_DEVICES, GFP_KERNEL);
> + mutex_unlock(&idr_lock);
> + if (rc < 0) {
> dev_err(dev, "No available tpm device numbers\n");
> kfree(chip);
> - return ERR_PTR(-ENOMEM);
> + return ERR_PTR(rc);
> }
> -
> - set_bit(chip->dev_num, dev_mask);
> + chip->dev_num = rc;
>
> device_initialize(&chip->dev);
>
> @@ -242,19 +249,28 @@ static int tpm_add_char_device(struct tpm_chip *chip)
> return rc;
> }
>
> + /* Make the chip available. */
> + mutex_lock(&idr_lock);
> + idr_replace(&dev_nums_idr, chip, chip->dev_num);
> + mutex_unlock(&idr_lock);
> +
> return rc;
> }
>
> static void tpm_del_char_device(struct tpm_chip *chip)
> {
> cdev_del(&chip->cdev);
> + device_del(&chip->dev);
> +
> + /* Make the chip unavailable. */
> + mutex_lock(&idr_lock);
> + idr_replace(&dev_nums_idr, NULL, chip->dev_num);
> + mutex_unlock(&idr_lock);
>
> /* Make the driver uncallable. */
> down_write(&chip->ops_sem);
> chip->ops = NULL;
> up_write(&chip->ops_sem);
> -
> - device_del(&chip->dev);
> }
>
> static int tpm1_chip_register(struct tpm_chip *chip)
> @@ -309,11 +325,6 @@ int tpm_chip_register(struct tpm_chip *chip)
> if (rc)
> goto out_err;
>
> - /* Make the chip available. */
> - spin_lock(&driver_lock);
> - list_add_tail_rcu(&chip->list, &tpm_chip_list);
> - spin_unlock(&driver_lock);
> -
> chip->flags |= TPM_CHIP_FLAG_REGISTERED;
>
> if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> @@ -350,11 +361,6 @@ void tpm_chip_unregister(struct tpm_chip *chip)
> if (!(chip->flags & TPM_CHIP_FLAG_REGISTERED))
> return;
>
> - spin_lock(&driver_lock);
> - list_del_rcu(&chip->list);
> - spin_unlock(&driver_lock);
> - synchronize_rcu();
> -
> if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
>
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 5caf154..5397b64 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -1139,6 +1139,7 @@ static int __init tpm_init(void)
>
> static void __exit tpm_exit(void)
> {
> + idr_destroy(&dev_nums_idr);
> class_destroy(tpm_class);
> unregister_chrdev_region(tpm_devt, TPM_NUM_DEVICES);
> }
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 5fcf788..928b47f 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -34,7 +34,7 @@
> enum tpm_const {
> TPM_MINOR = 224, /* officially assigned */
> TPM_BUFSIZE = 4096,
> - TPM_NUM_DEVICES = 256,
> + TPM_NUM_DEVICES = 65536,
> TPM_RETRY = 50, /* 5 seconds */
> };
>
> @@ -195,8 +195,6 @@ struct tpm_chip {
> acpi_handle acpi_dev_handle;
> char ppi_version[TPM_PPI_VERSION_LEN + 1];
> #endif /* CONFIG_ACPI */
> -
> - struct list_head list;
> };
>
> #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
> @@ -492,6 +490,7 @@ static inline void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
> extern struct class *tpm_class;
> extern dev_t tpm_devt;
> extern const struct file_operations tpm_fops;
> +extern struct idr dev_nums_idr;
>
> ssize_t tpm_getcap(struct device *, __be32, cap_t *, const char *);
> ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
> --
> 2.4.3
>
Powered by blists - more mailing lists