linux-kernel - [PATCH v2] tpm: fix: sanitized code paths in tpm_chip

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-next>] [day] [month] [year] [list]

Message-Id: <1426659434-4003-1-git-send-email-jarkko.sakkinen@linux.intel.com>
Date:	Wed, 18 Mar 2015 08:17:14 +0200
From:	Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
To:	Peter Huewe <peterhuewe@....de>, Ashley Lai <ashley@...leylai.com>,
	Marcel Selhorst <tpmdd@...horst.net>
Cc:	tpmdd-devel@...ts.sourceforge.net, linux-kernel@...r.kernel.org,
	christophe.ricard@...il.com, jason.gunthorpe@...idianresearch.com,
	stefanb@...ux.vnet.ibm.com,
	Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
Subject: [PATCH v2] tpm: fix: sanitized code paths in tpm_chip_register()

I started to work with PPI interface so that it would be available
under character device sysfs directory and realized that chip
registeration was still too messy.

In TPM 1.x in some rare scenarios (errors that almost never occur)
wrong order in deinitialization steps was taken in teardown. I
reproduced these scenarios by manually inserting error codes in the
place of the corresponding function calls.

The key problem is that the teardown is messy with two separate code
paths (this was inherited when moving code from tpm-interface.c).

Moved TPM 1.x specific register/unregister functionality to own helper
functions and added single code path for teardown in tpm_chip_register().
Now the code paths have been fixed and it should be easier to review
later on this part of the code.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
---
 drivers/char/tpm/tpm-chip.c | 66 ++++++++++++++++++++++++++++-----------------
 1 file changed, 42 insertions(+), 24 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index e096e9c..283f00a 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -170,6 +170,41 @@ static void tpm_dev_del_device(struct tpm_chip *chip)
 	device_unregister(&chip->dev);
 }
 
+static int tpm1_chip_register(struct tpm_chip *chip)
+{
+	int rc;
+
+	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		return 0;
+
+	rc = tpm_sysfs_add_device(chip);
+	if (rc)
+		return rc;
+
+	rc = tpm_add_ppi(chip);
+	if (rc) {
+		tpm_sysfs_del_device(chip);
+		return rc;
+	}
+
+	chip->bios_dir = tpm_bios_log_setup(chip->devname);
+
+	return 0;
+}
+
+static void tpm1_chip_unregister(struct tpm_chip *chip)
+{
+	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		return;
+
+	if (chip->bios_dir)
+		tpm_bios_log_teardown(chip->bios_dir);
+
+	tpm_remove_ppi(chip);
+
+	tpm_sysfs_del_device(chip);
+}
+
 /*
  * tpm_chip_register() - create a character device for the TPM chip
  * @chip: TPM chip to use.
@@ -185,22 +220,13 @@ int tpm_chip_register(struct tpm_chip *chip)
 {
 	int rc;
 
-	/* Populate sysfs for TPM1 devices. */
-	if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
-		rc = tpm_sysfs_add_device(chip);
-		if (rc)
-			goto del_misc;
-
-		rc = tpm_add_ppi(chip);
-		if (rc)
-			goto del_sysfs;
-
-		chip->bios_dir = tpm_bios_log_setup(chip->devname);
-	}
+	rc = tpm1_chip_register(chip);
+	if (rc)
+		return rc;
 
 	rc = tpm_dev_add_device(chip);
 	if (rc)
-		return rc;
+		goto out_err;
 
 	/* Make the chip available. */
 	spin_lock(&driver_lock);
@@ -210,10 +236,8 @@ int tpm_chip_register(struct tpm_chip *chip)
 	chip->flags |= TPM_CHIP_FLAG_REGISTERED;
 
 	return 0;
-del_sysfs:
-	tpm_sysfs_del_device(chip);
-del_misc:
-	tpm_dev_del_device(chip);
+out_err:
+	tpm1_chip_unregister(chip);
 	return rc;
 }
 EXPORT_SYMBOL_GPL(tpm_chip_register);
@@ -238,13 +262,7 @@ void tpm_chip_unregister(struct tpm_chip *chip)
 	spin_unlock(&driver_lock);
 	synchronize_rcu();
 
-	if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
-		if (chip->bios_dir)
-			tpm_bios_log_teardown(chip->bios_dir);
-		tpm_remove_ppi(chip);
-		tpm_sysfs_del_device(chip);
-	}
-
+	tpm1_chip_unregister(chip);
 	tpm_dev_del_device(chip);
 }
 EXPORT_SYMBOL_GPL(tpm_chip_unregister);
-- 
2.1.4

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