| 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: <1223302886.6774.32.camel@dyn536739.br.ibm.com>
Date: Mon, 06 Oct 2008 11:21:26 -0300
From: Rajiv Andrade <srajiv@...ux.vnet.ibm.com>
To: "Serge E. Hallyn" <serue@...ibm.com>
Cc: linux-kernel@...r.kernel.org, zohar@...ux.vnet.ibm.com,
akpm@...ux-foundation.org, jmorris@...ei.org
Subject: Re: [PATCH 3/5][resubmit][BUG] TPM: rcu locking
Please, do not consider this patch, it's wrong...
On Fri, 2008-10-03 at 16:12 -0300, Rajiv Andrade wrote:
> This is a resubmission of rcu locking patch, posted some hours ago, addressing Serge Hallyn's comment.
> In the previous patch there was missing the spin_lock/unlock protecting list_add_rcu()
> in tpm_register_hardware(). There were also an unnecessary list_del() inside the same function,
> that is now being removed.
>
> Signed-off-by: Mimi Zohar <zohar@...ux.vnet.ibm.com>
> Signed-off-by: Rajiv Andrade <srajiv@...ux.vnet.ibm.com>
> ---
> drivers/char/tpm/tpm.c | 61 +++++++++++++++++++-----------------------------
> 1 files changed, 24 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
> index dfd4e7f..e96ca5a 100644
> --- a/drivers/char/tpm/tpm.c
> +++ b/drivers/char/tpm/tpm.c
> @@ -961,33 +961,28 @@ EXPORT_SYMBOL_GPL(tpm_store_cancel);
> */
> int tpm_open(struct inode *inode, struct file *file)
> {
> - int rc = 0, minor = iminor(inode);
> + int minor = iminor(inode);
> struct tpm_chip *chip = NULL, *pos;
>
> - spin_lock(&driver_lock);
> -
> - list_for_each_entry(pos, &tpm_chip_list, list) {
> + rcu_read_lock();
> + list_for_each_entry_rcu(pos, &tpm_chip_list, list) {
> if (pos->vendor.miscdev.minor == minor) {
> chip = pos;
> + get_device(chip->dev);
> break;
> }
> }
> + rcu_read_unlock();
>
> - if (chip == NULL) {
> - rc = -ENODEV;
> - goto err_out;
> - }
> + if (!chip)
> + return -ENODEV;
>
> if (test_and_set_bit(0, &chip->is_open)) {
> dev_dbg(chip->dev, "Another process owns this TPM\n");
> - rc = -EBUSY;
> - goto err_out;
> + put_device(chip->dev);
> + return -EBUSY;
> }
>
> - get_device(chip->dev);
> -
> - spin_unlock(&driver_lock);
> -
> chip->data_buffer = kmalloc(TPM_BUFSIZE * sizeof(u8), GFP_KERNEL);
> if (chip->data_buffer == NULL) {
> clear_bit(0, &chip->is_open);
> @@ -999,26 +994,23 @@ int tpm_open(struct inode *inode, struct file *file)
>
> file->private_data = chip;
> return 0;
> -
> -err_out:
> - spin_unlock(&driver_lock);
> - return rc;
> }
> EXPORT_SYMBOL_GPL(tpm_open);
>
> +/*
> + * Called on file close
> + */
> int tpm_release(struct inode *inode, struct file *file)
> {
> struct tpm_chip *chip = file->private_data;
>
> flush_scheduled_work();
> - spin_lock(&driver_lock);
> file->private_data = NULL;
> del_singleshot_timer_sync(&chip->user_read_timer);
> atomic_set(&chip->data_pending, 0);
> + kfree(chip->data_buffer);
> clear_bit(0, &chip->is_open);
> put_device(chip->dev);
> - kfree(chip->data_buffer);
> - spin_unlock(&driver_lock);
> return 0;
> }
> EXPORT_SYMBOL_GPL(tpm_release);
> @@ -1092,13 +1084,11 @@ void tpm_remove_hardware(struct device *dev)
> }
>
> spin_lock(&driver_lock);
> -
> - list_del(&chip->list);
> -
> + list_del_rcu(&chip->list);
> spin_unlock(&driver_lock);
> + synchronize_rcu();
>
> misc_deregister(&chip->vendor.miscdev);
> -
> sysfs_remove_group(&dev->kobj, chip->vendor.attr_group);
> tpm_bios_log_teardown(chip->bios_dir);
>
> @@ -1146,8 +1136,7 @@ EXPORT_SYMBOL_GPL(tpm_pm_resume);
> /*
> * Once all references to platform device are down to 0,
> * release all allocated structures.
> - * In case vendor provided release function,
> - * call it too.
> + * In case vendor provided release function, call it too.
> */
> static void tpm_dev_release(struct device *dev)
> {
> @@ -1155,7 +1144,6 @@ static void tpm_dev_release(struct device *dev)
>
> if (chip->vendor.release)
> chip->vendor.release(dev);
> -
> chip->release(dev);
>
> clear_bit(chip->dev_num, dev_mask);
> @@ -1170,8 +1158,8 @@ static void tpm_dev_release(struct device *dev)
> * upon errant exit from this function specific probe function should call
> * pci_disable_device
> */
> -struct tpm_chip *tpm_register_hardware(struct device *dev, const struct tpm_vendor_specific
> - *entry)
> +struct tpm_chip *tpm_register_hardware(struct device *dev,
> + const struct tpm_vendor_specific *entry)
> {
> #define DEVNAME_SIZE 7
>
> @@ -1230,21 +1218,20 @@ struct tpm_chip *tpm_register_hardware(struct device *dev, const struct tpm_vend
> return NULL;
> }
>
> - spin_lock(&driver_lock);
> -
> - list_add(&chip->list, &tpm_chip_list);
> -
> - spin_unlock(&driver_lock);
> -
> if (sysfs_create_group(&dev->kobj, chip->vendor.attr_group)) {
> - list_del(&chip->list);
> misc_deregister(&chip->vendor.miscdev);
> put_device(chip->dev);
> +
> return NULL;
> }
>
> chip->bios_dir = tpm_bios_log_setup(devname);
>
> + /* Make chip available */
> + spin_lock(&driver_lock);
> + list_add_rcu(&chip->list, &tpm_chip_list);
> + spin_lock(&driver_lock);
> +
> return chip;
> }
> EXPORT_SYMBOL_GPL(tpm_register_hardware);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists