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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Sat, 30 Aug 2008 00:05:04 -0300
From:	Rajiv Andrade <srajiv@...ux.vnet.ibm.com>
To:	linux-kernel@...r.kernel.org
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	James Morris <jmoriss@...ei.org>,
	Paul McKenney <paulmck@...ux.vnet.ibm.com>,
	Peter Dolding <oiaohm@...il.com>,
	Kenneth Goldman <kgoldman@...ibm.com>,
	Debora Velarde <debora@...ux.vnet.ibm.com>,
	David Safford <safford@...son.ibm.com>,
	Serge Hallyn <serue@...ux.vnet.ibm.com>,
	Mimi Zohar <zohar@...ux.vnet.ibm.com>,
	Pavel Machek <pavel@...e.cz>, Greg KH <greg@...ah.com>,
	Christoph Hellwig <hch@...radead.org>,
	Alan Cox <alan@...rguk.ukuu.org.uk>,
	Rajiv Andrade <srajiv@...ux.vnet.ibm.com>
Subject: [PATCH 2/2] TPM: rcu locking

Continue to protect the tpm_chip_list using the driver_lock
as before, and add an rcu to protect readers.

Signed-off-by: Mimi Zohar <zohar@...ux.vnet.ibm.com>
Acked-by: Rajiv Andrade <srajiv@...ux.vnet.ibm.com>
---
 drivers/char/tpm/tpm.c |  268 ++++++++++++++++++++++++++++++++----------------
 1 files changed, 178 insertions(+), 90 deletions(-)

diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
index 59b31e1..704ab01 100644
--- a/drivers/char/tpm/tpm.c
+++ b/drivers/char/tpm/tpm.c
@@ -587,14 +587,22 @@ ssize_t tpm_show_enabled(struct device * dev, struct device_attribute * attr,
 {
 	u8 *data;
 	ssize_t rc;
+	struct tpm_chip *chip;
+
+	rcu_read_lock();
+	chip = dev_get_drvdata(dev);
+	if (chip)
+		get_device(chip->dev);	/* protect from disappearing */
+	rcu_read_unlock();
 
-	struct tpm_chip *chip = dev_get_drvdata(dev);
 	if (chip == NULL)
 		return -ENODEV;
 
 	data = kzalloc(TPM_INTERNAL_RESULT_SIZE, GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
+	if (!data) {
+		rc = -ENOMEM;
+		goto out_alloc;
+	}
 
 	memcpy(data, tpm_cap, sizeof(tpm_cap));
 	data[TPM_CAP_IDX] = TPM_CAP_FLAG;
@@ -603,13 +611,15 @@ ssize_t tpm_show_enabled(struct device * dev, struct device_attribute * attr,
 	rc = transmit_cmd(chip, data, TPM_INTERNAL_RESULT_SIZE,
 			"attemtping to determine the permanent enabled state");
 	if (rc) {
-		kfree(data);
-		return 0;
+		rc = 0;
+		goto out;
 	}
 
 	rc = sprintf(buf, "%d\n", !data[TPM_GET_CAP_PERM_DISABLE_IDX]);
-
+out:
 	kfree(data);
+out_alloc:
+	put_device(chip->dev);
 	return rc;
 }
 EXPORT_SYMBOL_GPL(tpm_show_enabled);
@@ -619,14 +629,24 @@ ssize_t tpm_show_active(struct device * dev, struct device_attribute * attr,
 {
 	u8 *data;
 	ssize_t rc;
+	struct tpm_chip *chip;
 
-	struct tpm_chip *chip = dev_get_drvdata(dev);
-	if (chip == NULL)
-		return -ENODEV;
+	rcu_read_lock();
+	chip = dev_get_drvdata(dev);
+	if (chip)
+		get_device(chip->dev);	/* protect from disappearing */
+	rcu_read_unlock();
+
+	if (chip == NULL) {
+		rc = -ENODEV;
+		goto out_alloc;
+	}
 
 	data = kzalloc(TPM_INTERNAL_RESULT_SIZE, GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
+	if (!data) {
+		rc = -ENOMEM;
+		goto out;
+	}
 
 	memcpy(data, tpm_cap, sizeof(tpm_cap));
 	data[TPM_CAP_IDX] = TPM_CAP_FLAG;
@@ -635,13 +655,15 @@ ssize_t tpm_show_active(struct device * dev, struct device_attribute * attr,
 	rc = transmit_cmd(chip, data, TPM_INTERNAL_RESULT_SIZE,
 			"attemtping to determine the permanent active state");
 	if (rc) {
-		kfree(data);
-		return 0;
+		rc = 0;
+		goto out;
 	}
 
 	rc = sprintf(buf, "%d\n", !data[TPM_GET_CAP_PERM_INACTIVE_IDX]);
-
+out:
 	kfree(data);
+out_alloc:
+	put_device(chip->dev);
 	return rc;
 }
 EXPORT_SYMBOL_GPL(tpm_show_active);
@@ -651,14 +673,22 @@ ssize_t tpm_show_owned(struct device * dev, struct device_attribute * attr,
 {
 	u8 *data;
 	ssize_t rc;
+	struct tpm_chip *chip;
+
+	rcu_read_lock();
+	chip = dev_get_drvdata(dev);
+	if (chip)
+		get_device(chip->dev);	/* protect from disappearing */
+	rcu_read_unlock();
 
-	struct tpm_chip *chip = dev_get_drvdata(dev);
 	if (chip == NULL)
 		return -ENODEV;
 
 	data = kzalloc(TPM_INTERNAL_RESULT_SIZE, GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
+	if (!data) {
+		rc = -ENOMEM;
+		goto out_alloc;
+	}
 
 	memcpy(data, tpm_cap, sizeof(tpm_cap));
 	data[TPM_CAP_IDX] = TPM_CAP_PROP;
@@ -667,13 +697,15 @@ ssize_t tpm_show_owned(struct device * dev, struct device_attribute * attr,
 	rc = transmit_cmd(chip, data, TPM_INTERNAL_RESULT_SIZE,
 			"attempting to determine the owner state");
 	if (rc) {
-		kfree(data);
-		return 0;
+		rc = 0;
+		goto out;
 	}
 
 	rc = sprintf(buf, "%d\n", data[TPM_GET_CAP_RET_BOOL_1_IDX]);
-
+out:
 	kfree(data);
+out_alloc:
+	put_device(chip->dev);
 	return rc;
 }
 EXPORT_SYMBOL_GPL(tpm_show_owned);
@@ -683,14 +715,22 @@ ssize_t tpm_show_temp_deactivated(struct device * dev,
 {
 	u8 *data;
 	ssize_t rc;
+	struct tpm_chip *chip;
+
+	rcu_read_lock();
+	chip = dev_get_drvdata(dev);
+	if (chip)
+		get_device(chip->dev);	/* protect from disappearing */
+	rcu_read_unlock();
 
-	struct tpm_chip *chip = dev_get_drvdata(dev);
 	if (chip == NULL)
 		return -ENODEV;
 
 	data = kzalloc(TPM_INTERNAL_RESULT_SIZE, GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
+	if (!data) {
+		rc = -ENOMEM;
+		goto out_alloc;
+	}
 
 	memcpy(data, tpm_cap, sizeof(tpm_cap));
 	data[TPM_CAP_IDX] = TPM_CAP_FLAG;
@@ -699,13 +739,15 @@ ssize_t tpm_show_temp_deactivated(struct device * dev,
 	rc = transmit_cmd(chip, data, TPM_INTERNAL_RESULT_SIZE,
 			"attempting to determine the temporary state");
 	if (rc) {
-		kfree(data);
-		return 0;
+		rc = 0;
+		goto out;
 	}
 
 	rc = sprintf(buf, "%d\n", data[TPM_GET_CAP_TEMP_INACTIVE_IDX]);
-
+out:
 	kfree(data);
+out_alloc:
+	put_device(chip->dev);
 	return rc;
 }
 EXPORT_SYMBOL_GPL(tpm_show_temp_deactivated);
@@ -725,14 +767,22 @@ ssize_t tpm_show_pcrs(struct device *dev, struct device_attribute *attr,
 	int i, j, num_pcrs;
 	__be32 index;
 	char *str = buf;
+	struct tpm_chip *chip;
+
+	rcu_read_lock();
+	chip = dev_get_drvdata(dev);
+	if (chip)
+		get_device(chip->dev);	/* protect from disappearing */
+	rcu_read_unlock();
 
-	struct tpm_chip *chip = dev_get_drvdata(dev);
 	if (chip == NULL)
 		return -ENODEV;
 
 	data = kzalloc(TPM_INTERNAL_RESULT_SIZE, GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
+	if (!data) {
+		rc = -ENOMEM;
+		goto out_alloc;
+	}
 
 	memcpy(data, tpm_cap, sizeof(tpm_cap));
 	data[TPM_CAP_IDX] = TPM_CAP_PROP;
@@ -741,8 +791,8 @@ ssize_t tpm_show_pcrs(struct device *dev, struct device_attribute *attr,
 	rc = transmit_cmd(chip, data, TPM_INTERNAL_RESULT_SIZE,
 			"attempting to determine the number of PCRS");
 	if (rc) {
-		kfree(data);
-		return 0;
+		rc = 0;
+		goto out;
 	}
 
 	num_pcrs = be32_to_cpu(*((__be32 *) (data + 14)));
@@ -753,15 +803,18 @@ ssize_t tpm_show_pcrs(struct device *dev, struct device_attribute *attr,
 		rc = transmit_cmd(chip, data, TPM_INTERNAL_RESULT_SIZE,
 				"attempting to read a PCR");
 		if (rc)
-			goto out;
+			break;
 		str += sprintf(str, "PCR-%02d: ", i);
 		for (j = 0; j < TPM_DIGEST_SIZE; j++)
 			str += sprintf(str, "%02X ", *(data + 10 + j));
 		str += sprintf(str, "\n");
 	}
+	rc = str - buf;
 out:
 	kfree(data);
-	return str - buf;
+out_alloc:
+	put_device(chip->dev);
+	return rc;
 }
 EXPORT_SYMBOL_GPL(tpm_show_pcrs);
 
@@ -779,21 +832,31 @@ ssize_t tpm_show_pubek(struct device *dev, struct device_attribute *attr,
 	ssize_t err;
 	int i, rc;
 	char *str = buf;
+	struct tpm_chip *chip;
+
+	rcu_read_lock();
+	chip = dev_get_drvdata(dev);
+	if (chip)
+		get_device(chip->dev);	/* protect from disappearing */
+	rcu_read_unlock();
 
-	struct tpm_chip *chip = dev_get_drvdata(dev);
 	if (chip == NULL)
 		return -ENODEV;
 
 	data = kzalloc(READ_PUBEK_RESULT_SIZE, GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
+	if (!data) {
+		rc = -ENOMEM;
+		goto out_alloc;
+	}
 
 	memcpy(data, readpubek, sizeof(readpubek));
 
 	err = transmit_cmd(chip, data, READ_PUBEK_RESULT_SIZE,
 			"attempting to read the PUBEK");
-	if (err)
+	if (err) {
+		rc = str - buf;
 		goto out;
+	}
 
 	/* 
 	   ignore header 10 bytes
@@ -823,9 +886,11 @@ ssize_t tpm_show_pubek(struct device *dev, struct device_attribute *attr,
 		if ((i + 1) % 16 == 0)
 			str += sprintf(str, "\n");
 	}
-out:
 	rc = str - buf;
+out:
 	kfree(data);
+out_alloc:
+	put_device(chip->dev);
 	return rc;
 }
 EXPORT_SYMBOL_GPL(tpm_show_pubek);
@@ -847,14 +912,22 @@ ssize_t tpm_show_caps(struct device *dev, struct device_attribute *attr,
 	u8 *data;
 	ssize_t rc;
 	char *str = buf;
+	struct tpm_chip *chip;
+
+	rcu_read_lock();
+	chip = dev_get_drvdata(dev);
+	if (chip)
+		get_device(chip->dev);	/* protect from disappearing */
+	rcu_read_unlock();
 
-	struct tpm_chip *chip = dev_get_drvdata(dev);
 	if (chip == NULL)
 		return -ENODEV;
 
 	data = kzalloc(TPM_INTERNAL_RESULT_SIZE, GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
+	if (!data) {
+		rc = -ENOMEM;
+		goto out_alloc;
+	}
 
 	memcpy(data, tpm_cap, sizeof(tpm_cap));
 	data[TPM_CAP_IDX] = TPM_CAP_PROP;
@@ -863,8 +936,8 @@ ssize_t tpm_show_caps(struct device *dev, struct device_attribute *attr,
 	rc = transmit_cmd(chip, data, TPM_INTERNAL_RESULT_SIZE,
 			"attempting to determine the manufacturer");
 	if (rc) {
-		kfree(data);
-		return 0;
+		rc = 0;
+		goto out;
 	}
 
 	str += sprintf(str, "Manufacturer: 0x%x\n",
@@ -874,17 +947,22 @@ ssize_t tpm_show_caps(struct device *dev, struct device_attribute *attr,
 	data[CAP_VERSION_IDX] = CAP_VERSION_1_1;
 	rc = transmit_cmd(chip, data, TPM_INTERNAL_RESULT_SIZE,
 			"attempting to determine the 1.1 version");
-	if (rc)
+	if (rc) {
+		rc = str - buf;
 		goto out;
+	}
 
 	str += sprintf(str,
 		       "TCG version: %d.%d\nFirmware version: %d.%d\n",
 		       (int) data[14], (int) data[15], (int) data[16],
 		       (int) data[17]);
 
+	rc = str - buf;
 out:
 	kfree(data);
-	return str - buf;
+out_alloc:
+	put_device(chip->dev);
+	return rc;
 }
 EXPORT_SYMBOL_GPL(tpm_show_caps);
 
@@ -892,16 +970,25 @@ ssize_t tpm_show_caps_1_2(struct device * dev,
 			  struct device_attribute * attr, char *buf)
 {
 	u8 *data;
+	ssize_t rc;
 	ssize_t len;
 	char *str = buf;
+	struct tpm_chip *chip;
+
+	rcu_read_lock();
+	chip = dev_get_drvdata(dev);
+	if (chip)
+		get_device(chip->dev);	/* protect from disappearing */
+	rcu_read_unlock();
 
-	struct tpm_chip *chip = dev_get_drvdata(dev);
 	if (chip == NULL)
 		return -ENODEV;
 
 	data = kzalloc(TPM_INTERNAL_RESULT_SIZE, GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
+	if (!data) {
+		rc = -ENOMEM;
+		goto out_alloc;
+	}
 
 	memcpy(data, tpm_cap, sizeof(tpm_cap));
 	data[TPM_CAP_IDX] = TPM_CAP_PROP;
@@ -913,7 +1000,8 @@ ssize_t tpm_show_caps_1_2(struct device * dev,
 			"attempting to determine the manufacturer\n",
 			be32_to_cpu(*((__be32 *) (data + TPM_RET_CODE_IDX))));
 		kfree(data);
-		return 0;
+		rc = 0;
+		goto out;
 	}
 
 	str += sprintf(str, "Manufacturer: 0x%x\n",
@@ -927,6 +1015,7 @@ ssize_t tpm_show_caps_1_2(struct device * dev,
 		dev_err(chip->dev, "A TPM error (%d) occurred "
 			"attempting to determine the 1.2 version\n",
 			be32_to_cpu(*((__be32 *) (data + TPM_RET_CODE_IDX))));
+		rc = str - buf;
 		goto out;
 	}
 	str += sprintf(str,
@@ -934,20 +1023,29 @@ ssize_t tpm_show_caps_1_2(struct device * dev,
 		       (int) data[16], (int) data[17], (int) data[18],
 		       (int) data[19]);
 
+	rc = str - buf;
 out:
 	kfree(data);
-	return str - buf;
+out_alloc:
+	put_device(chip->dev);
+	return rc;
 }
 EXPORT_SYMBOL_GPL(tpm_show_caps_1_2);
 
 ssize_t tpm_store_cancel(struct device *dev, struct device_attribute *attr,
 			const char *buf, size_t count)
 {
-	struct tpm_chip *chip = dev_get_drvdata(dev);
+	struct tpm_chip *chip;
+
+	rcu_read_lock();
+	chip = dev_get_drvdata(dev);
+	if (chip)
+		get_device(chip->dev);	/* protect from disappearing */
+	rcu_read_unlock();
 	if (chip == NULL)
 		return 0;
-
 	chip->vendor.cancel(chip);
+	put_device(chip->dev);
 	return count;
 }
 EXPORT_SYMBOL_GPL(tpm_store_cancel);
@@ -956,70 +1054,59 @@ EXPORT_SYMBOL_GPL(tpm_store_cancel);
  * Device file system interface to the TPM
  *
  * It's assured that the chip will be opened just once,
- * by the check of is_open variable, which is protected
- * by driver_lock.
+ * by the check of is_open variable.
  */
 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); /* protect from disappearing */
 			break;
 		}
 	}
+	rcu_read_unlock();
+	if (!chip)
+		return -ENODEV;
 
-	if (chip == NULL) {
-		rc = -ENODEV;
-		goto err_out;
-	}
-
-	if (chip->is_open) {
+	if (!test_and_set_bit(0, &chip->is_open)) {
 		dev_dbg(chip->dev, "Another process owns this TPM\n");
-		rc = -EBUSY;
-		goto err_out;
+		return -EBUSY;
 	}
 
-	chip->is_open = 1;
-	get_device(chip->dev); /* protect from chip disappearing */
-
-	spin_unlock(&driver_lock);
-
 	chip->data_buffer = kmalloc(TPM_BUFSIZE * sizeof(u8), GFP_KERNEL);
 	if (chip->data_buffer == NULL) {
-		chip->is_open = 0;
+		clear_bit(0, &chip->is_open);
 		put_device(chip->dev);
 		return -ENOMEM;
 	}
 
 	atomic_set(&chip->data_pending, 0);
-
 	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);
-	chip->is_open = 0;
-	put_device(chip->dev);
 	kfree(chip->data_buffer);
-	spin_unlock(&driver_lock);
+
+	clear_bit(0, &chip->is_open);
+
+	put_device(chip->dev);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(tpm_release);
@@ -1082,6 +1169,7 @@ ssize_t tpm_read(struct file *file, char __user *buf,
 	return ret_size;
 }
 EXPORT_SYMBOL_GPL(tpm_read);
+
 /*
  *   Called on unloading the driver.
  *
@@ -1098,13 +1186,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);
 
@@ -1152,8 +1238,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)
 {
@@ -1176,8 +1261,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
 
@@ -1243,9 +1328,12 @@ struct tpm_chip *tpm_register_hardware(struct device *dev, const struct tpm_vend
 	}
 
 	chip->bios_dir = tpm_bios_log_setup(devname);
+
+	/* Make chip available */
 	spin_lock(&driver_lock);
-	list_add(&chip->list, &tpm_chip_list);
+	list_add_rcu(&chip->list, &tpm_chip_list);
 	spin_unlock(&driver_lock);
+	synchronize_rcu();
 
 	return chip;
 }
-- 
1.5.6.3

--
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ