tpm: fix call order in tpm-chip.c

Commit Message

Jarkko Sakkinen March 1, 2015, 9:55 p.m. UTC

- tpm_dev_add_device(): cdev_add() must be done before uevent is
  propagated in order to avoid races.
- tpm_chip_register(): tpm_dev_add_device() must be done as the
  last step before exposing device to the user space in order to
  avoid races.

In addition clarified description in tpm_chip_register().

Signed-off-by: Jarkko Sakkinen <[e-mail]>
---
 drivers/char/tpm/tpm-chip.c | 34 ++++++++++++++--------------------
 1 file changed, 14 insertions(+), 20 deletions(-)

Comments

Peter Huewe March 1, 2015, 11:06 p.m. UTC | #1

Am Sonntag, 1. März 2015, 22:55:47 schrieb Jarkko Sakkinen:
> - tpm_dev_add_device(): cdev_add() must be done before uevent is
>   propagated in order to avoid races.
> - tpm_chip_register(): tpm_dev_add_device() must be done as the
>   last step before exposing device to the user space in order to
>   avoid races.
> 
> In addition clarified description in tpm_chip_register().
> 
> Signed-off-by: Jarkko Sakkinen <[e-mail]>
Looks good to me.
Staged it and I'll send out the pull request in the next few days.

(have to catch up mails from traveling)

Thanks,
Peter

------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the 
conversation now. http://goparallel.sourceforge.net/

Patch

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 1d278cc..e096e9c 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -140,24 +140,24 @@  static int tpm_dev_add_device(struct tpm_chip *chip)
 {
 	int rc;
 
-	rc = device_add(&chip->dev);
+	rc = cdev_add(&chip->cdev, chip->dev.devt, 1);
 	if (rc) {
 		dev_err(&chip->dev,
-			"unable to device_register() %s, major %d, minor %d, err=%d\n",
+			"unable to cdev_add() %s, major %d, minor %d, err=%d\n",
 			chip->devname, MAJOR(chip->dev.devt),
 			MINOR(chip->dev.devt), rc);
 
+		device_unregister(&chip->dev);
 		return rc;
 	}
 
-	rc = cdev_add(&chip->cdev, chip->dev.devt, 1);
+	rc = device_add(&chip->dev);
 	if (rc) {
 		dev_err(&chip->dev,
-			"unable to cdev_add() %s, major %d, minor %d, err=%d\n",
+			"unable to device_register() %s, major %d, minor %d, err=%d\n",
 			chip->devname, MAJOR(chip->dev.devt),
 			MINOR(chip->dev.devt), rc);
 
-		device_unregister(&chip->dev);
 		return rc;
 	}
 
@@ -174,27 +174,17 @@  static void tpm_dev_del_device(struct tpm_chip *chip)
  * tpm_chip_register() - create a character device for the TPM chip
  * @chip: TPM chip to use.
  *
- * Creates a character device for the TPM chip and adds sysfs interfaces for
- * the device, PPI and TCPA. As the last step this function adds the
- * chip to the list of TPM chips available for use.
+ * Creates a character device for the TPM chip and adds sysfs attributes for
+ * the device. As the last step this function adds the chip to the list of TPM
+ * chips available for in-kernel use.
  *
- * NOTE: This function should be only called after the chip initialization
- * is complete.
- *
- * Called from tpm_<specific>.c probe function only for devices
- * the driver has determined it should claim.  Prior to calling
- * this function the specific probe function has called pci_enable_device
- * upon errant exit from this function specific probe function should call
- * pci_disable_device
+ * This function should be only called after the chip initialization is
+ * complete.
  */
 int tpm_chip_register(struct tpm_chip *chip)
 {
 	int rc;
 
-	rc = tpm_dev_add_device(chip);
-	if (rc)
-		return rc;
-
 	/* Populate sysfs for TPM1 devices. */
 	if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
 		rc = tpm_sysfs_add_device(chip);
@@ -208,6 +198,10 @@  int tpm_chip_register(struct tpm_chip *chip)
 		chip->bios_dir = tpm_bios_log_setup(chip->devname);
 	}
 
+	rc = tpm_dev_add_device(chip);
+	if (rc)
+		return rc;
+
 	/* Make the chip available. */
 	spin_lock(&driver_lock);
 	list_add_rcu(&chip->list, &tpm_chip_list);