[PATCH 0/6] Have IMA find and use a tpm_chip until system shutdown

[PATCH 0/6] Have IMA find and use a tpm_chip until system shutdown

[Stefan Berger]

This series of patches converts IMA's usage of the tpm_chip to find a TPM
chip initially and use it until the machine is shut down. To do this we need
to introduce a kref for the tpm_chip that IMA and all other users of a
tpm_chip hold onto until they don't need the TPM chip anymore.

    Stefan

Stefan Berger (6):
  tpm: Introduce a kref for the tpm_chip
  tpm: Get additional kref with every call to tpm_try_get_ops()
  tpm: Implement tpm_chip_find() for other subsystems to call
  ima: Implement ima_shutdown and register it as a reboot_notifier
  ima: Use tpm_chip_find() and access TPM functions using it
  ima: Get rid of ima_used_chip and use ima_tpm_chip != NULL instead

 drivers/char/tpm/tpm-chip.c         | 54 +++++++++++++++++++++++++++++++++----
 drivers/char/tpm/tpm.h              |  1 +
 include/linux/tpm.h                 |  9 +++++++
 security/integrity/ima/ima.h        |  4 ++-
 security/integrity/ima/ima_crypto.c | 14 +++++++---
 security/integrity/ima/ima_init.c   | 36 ++++++++++++++++++-------
 security/integrity/ima/ima_queue.c  |  9 ++++---
 7 files changed, 105 insertions(+), 22 deletions(-)

-- 
2.13.6

[PATCH 1/6] tpm: Introduce a kref for the tpm_chip

[Stefan Berger]

Introduce a kref for the tpm_chip that we initialize when the tpm_chip has
been allocated and release before the tpm_chip is to be freed.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 drivers/char/tpm/tpm-chip.c | 24 +++++++++++++++++++-----
 drivers/char/tpm/tpm.h      |  1 +
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 0a62c19937b6..a933676194a4 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -119,8 +119,24 @@ struct tpm_chip *tpm_chip_find_get(struct tpm_chip *chip)
 	return res;
 }
 
+static void tpm_chip_free(struct kref *kref)
+{
+	struct tpm_chip *chip = container_of(kref, struct tpm_chip, kref);
+
+	kfree(chip->log.bios_event_log);
+	kfree(chip->work_space.context_buf);
+	kfree(chip->work_space.session_buf);
+	kfree(chip);
+}
+
+static void tpm_chip_put(struct tpm_chip *chip)
+{
+	if (chip)
+		kref_put(&chip->kref, tpm_chip_free);
+}
+
 /**
- * tpm_dev_release() - free chip memory and the device number
+ * tpm_dev_release() - free the device number and release reference to chip
  * @dev: the character device for the TPM chip
  *
  * This is used as the release function for the character device.
@@ -133,10 +149,7 @@ static void tpm_dev_release(struct device *dev)
 	idr_remove(&dev_nums_idr, chip->dev_num);
 	mutex_unlock(&idr_lock);
 
-	kfree(chip->log.bios_event_log);
-	kfree(chip->work_space.context_buf);
-	kfree(chip->work_space.session_buf);
-	kfree(chip);
+	tpm_chip_put(chip);
 }
 
 static void tpm_devs_release(struct device *dev)
@@ -195,6 +208,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
 
 	mutex_init(&chip->tpm_mutex);
 	init_rwsem(&chip->ops_sem);
+	kref_init(&chip->kref);
 
 	chip->ops = ops;
 
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 7f2d0f489e9c..098d7dcc04a4 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -211,6 +211,7 @@ struct tpm_chip {
 	struct cdev cdev;
 	struct cdev cdevs;
 
+	struct kref kref;
 	/* A driver callback under ops cannot be run unless ops_sem is held
 	 * (sometimes implicitly, eg for the sysfs code). ops becomes null
 	 * when the driver is unregistered, see tpm_try_get_ops.
-- 
2.13.6

[PATCH 2/6] tpm: Get additional kref with every call to tpm_try_get_ops()

[Stefan Berger]

Get a reference to the TPM chip on every call to tpm_try_get_ops()
and release the reference in tpm_put_ops().

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 drivers/char/tpm/tpm-chip.c | 5 ++++-
 include/linux/tpm.h         | 4 ++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index a933676194a4..23b667c730f6 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -58,6 +58,8 @@ int tpm_try_get_ops(struct tpm_chip *chip)
 	if (!chip->ops)
 		goto out_lock;
 
+	kref_get(&chip->kref);
+
 	return 0;
 out_lock:
 	up_read(&chip->ops_sem);
@@ -77,6 +79,7 @@ void tpm_put_ops(struct tpm_chip *chip)
 {
 	up_read(&chip->ops_sem);
 	put_device(&chip->dev);
+	tpm_chip_put(chip);
 }
 EXPORT_SYMBOL_GPL(tpm_put_ops);
 
@@ -129,7 +132,7 @@ static void tpm_chip_free(struct kref *kref)
 	kfree(chip);
 }
 
-static void tpm_chip_put(struct tpm_chip *chip)
+void tpm_chip_put(struct tpm_chip *chip)
 {
 	if (chip)
 		kref_put(&chip->kref, tpm_chip_free);
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 06639fb6ab85..cdb3ecdfc933 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -61,6 +61,7 @@ extern int tpm_seal_trusted(struct tpm_chip *chip,
 extern int tpm_unseal_trusted(struct tpm_chip *chip,
 			      struct trusted_key_payload *payload,
 			      struct trusted_key_options *options);
+extern void tpm_chip_put(struct tpm_chip *chip);
 #else
 static inline int tpm_is_tpm2(struct tpm_chip *chip)
 {
@@ -96,5 +97,8 @@ static inline int tpm_unseal_trusted(struct tpm_chip *chip,
 {
 	return -ENODEV;
 }
+static inline void tpm_chip_put(struct tpm_chip *chip)
+{
+}
 #endif
 #endif
-- 
2.13.6

[PATCH 3/6] tpm: Implement tpm_chip_find() for other subsystems to call

[Stefan Berger]

Implement tpm_chip_find() for other subsystems to find a TPM chip and
get a reference to that chip. Once done with using the chip, the reference
is released using tpm_chip_put().

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 drivers/char/tpm/tpm-chip.c | 27 +++++++++++++++++++++++++++
 include/linux/tpm.h         |  5 +++++
 2 files changed, 32 insertions(+)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 23b667c730f6..b04064291dab 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -84,6 +84,33 @@ void tpm_put_ops(struct tpm_chip *chip)
 EXPORT_SYMBOL_GPL(tpm_put_ops);
 
 /**
+ * tpm_chip_find() - find a TPM chip and get a reference to it
+ */
+struct tpm_chip *tpm_chip_find(void)
+{
+	struct tpm_chip *chip, *res = NULL;
+	int chip_num = 0;
+	int chip_prev;
+
+	mutex_lock(&idr_lock);
+
+	do {
+		chip_prev = chip_num;
+		chip = idr_get_next(&dev_nums_idr, &chip_num);
+		if (chip) {
+			kref_get(&chip->kref);
+			res = chip;
+			break;
+		}
+	} while (chip_prev != chip_num);
+
+	mutex_unlock(&idr_lock);
+
+	return res;
+}
+EXPORT_SYMBOL_GPL(tpm_chip_find);
+
+/**
  * tpm_chip_find_get() - find and reserve a TPM chip
  * @chip:	a &struct tpm_chip instance, %NULL for the default chip
  *
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index cdb3ecdfc933..f0d1abcaf4af 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -61,6 +61,7 @@ extern int tpm_seal_trusted(struct tpm_chip *chip,
 extern int tpm_unseal_trusted(struct tpm_chip *chip,
 			      struct trusted_key_payload *payload,
 			      struct trusted_key_options *options);
+extern struct tpm_chip *tpm_chip_find(void);
 extern void tpm_chip_put(struct tpm_chip *chip);
 #else
 static inline int tpm_is_tpm2(struct tpm_chip *chip)
@@ -97,6 +98,10 @@ static inline int tpm_unseal_trusted(struct tpm_chip *chip,
 {
 	return -ENODEV;
 }
+static inline struct tpm_chip *tpm_chip_find(void)
+{
+	return NULL;
+}
 static inline void tpm_chip_put(struct tpm_chip *chip)
 {
 }
-- 
2.13.6

[PATCH 4/6] ima: Implement ima_shutdown and register it as a reboot_notifier

[Stefan Berger]

Implement ima_shutdown so that we can release the tpm_chip before
devices are shut down. Register it as a low-priority reboot_notifier.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 security/integrity/ima/ima_init.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 29b72cd2502e..8a5258eb32b6 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -21,6 +21,7 @@
 #include <linux/scatterlist.h>
 #include <linux/slab.h>
 #include <linux/err.h>
+#include <linux/reboot.h>
 
 #include "ima.h"
 
@@ -104,11 +105,24 @@ void __init ima_load_x509(void)
 }
 #endif
 
+static int ima_shutdown(struct notifier_block *this, unsigned long action,
+			void *data)
+{
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block ima_reboot_notifier = {
+	.notifier_call = ima_shutdown,
+	.priority = 0,
+};
+
 int __init ima_init(void)
 {
 	u8 pcr_i[TPM_DIGEST_SIZE];
 	int rc;
 
+	register_reboot_notifier(&ima_reboot_notifier);
+
 	ima_used_chip = 0;
 	rc = tpm_pcr_read(NULL, 0, pcr_i);
 	if (rc == 0)
-- 
2.13.6

[PATCH 5/6] ima: Use tpm_chip_find() and access TPM functions using it

[Stefan Berger]

Rather than accessing the TPM functions using a NULL pointer, which
causes a lookup for a suitable chip every time, get a hold of a tpm_chip
and access the TPM functions using this chip. We call the tpm_chip
ima_tpm_chip and protect it, once initialization is done, using a
rw_semaphore ima_tpm_chip_lock.

Use ima_shutdown to release the tpm_chip.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 security/integrity/ima/ima.h        |  3 +++
 security/integrity/ima/ima_crypto.c | 12 ++++++++++--
 security/integrity/ima/ima_init.c   | 19 ++++++++++++-------
 security/integrity/ima/ima_queue.c  |  7 +++++--
 4 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 354bb5716ce3..53a88d578ca5 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -24,6 +24,7 @@
 #include <linux/hash.h>
 #include <linux/tpm.h>
 #include <linux/audit.h>
+#include <linux/rwsem.h>
 #include <crypto/hash_info.h>
 
 #include "../integrity.h"
@@ -56,6 +57,8 @@ extern int ima_policy_flag;
 extern int ima_used_chip;
 extern int ima_hash_algo;
 extern int ima_appraise;
+extern struct rw_semaphore ima_tpm_chip_lock;
+extern struct tpm_chip *ima_tpm_chip;
 
 /* IMA event related data */
 struct ima_event_data {
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 4e085a17124f..da7715240476 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -631,10 +631,18 @@ int ima_calc_buffer_hash(const void *buf, loff_t len,
 
 static void __init ima_pcrread(int idx, u8 *pcr)
 {
+	int result = 0;
+
+	down_read(&ima_tpm_chip_lock);
+
 	if (!ima_used_chip)
-		return;
+		goto out;
+
+	result = tpm_pcr_read(ima_tpm_chip, idx, pcr);
+out:
+	up_read(&ima_tpm_chip_lock);
 
-	if (tpm_pcr_read(NULL, idx, pcr) != 0)
+	if (result != 0)
 		pr_err("Error Communicating to TPM chip\n");
 }
 
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 8a5258eb32b6..24db06c4f463 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -28,6 +28,8 @@
 /* name for boot aggregate entry */
 static const char *boot_aggregate_name = "boot_aggregate";
 int ima_used_chip;
+struct rw_semaphore ima_tpm_chip_lock = __RWSEM_INITIALIZER(ima_tpm_chip_lock);
+struct tpm_chip *ima_tpm_chip;
 
 /* Add the boot aggregate to the IMA measurement list and extend
  * the PCR register.
@@ -108,6 +110,13 @@ void __init ima_load_x509(void)
 static int ima_shutdown(struct notifier_block *this, unsigned long action,
 			void *data)
 {
+	down_write(&ima_tpm_chip_lock);
+	if (ima_tpm_chip) {
+		tpm_chip_put(ima_tpm_chip);
+		ima_tpm_chip = NULL;
+		ima_used_chip = 0;
+	}
+	up_write(&ima_tpm_chip_lock);
 	return NOTIFY_DONE;
 }
 
@@ -118,19 +127,15 @@ static struct notifier_block ima_reboot_notifier = {
 
 int __init ima_init(void)
 {
-	u8 pcr_i[TPM_DIGEST_SIZE];
 	int rc;
 
 	register_reboot_notifier(&ima_reboot_notifier);
 
-	ima_used_chip = 0;
-	rc = tpm_pcr_read(NULL, 0, pcr_i);
-	if (rc == 0)
-		ima_used_chip = 1;
+	ima_tpm_chip = tpm_chip_find();
 
+	ima_used_chip = (ima_tpm_chip != NULL);
 	if (!ima_used_chip)
-		pr_info("No TPM chip found, activating TPM-bypass! (rc=%d)\n",
-			rc);
+		pr_info("No TPM chip found, activating TPM-bypass!\n");
 
 	rc = integrity_init_keyring(INTEGRITY_KEYRING_IMA);
 	if (rc)
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 418f35e38015..6c9427939a28 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -142,10 +142,13 @@ static int ima_pcr_extend(const u8 *hash, int pcr)
 {
 	int result = 0;
 
+	down_read(&ima_tpm_chip_lock);
 	if (!ima_used_chip)
-		return result;
+		goto out;
 
-	result = tpm_pcr_extend(NULL, pcr, hash);
+	result = tpm_pcr_extend(ima_tpm_chip, pcr, hash);
+out:
+	up_read(&ima_tpm_chip_lock);
 	if (result != 0)
 		pr_err("Error Communicating to TPM chip, result: %d\n", result);
 	return result;
-- 
2.13.6

[PATCH 6/6] ima: Get rid of ima_used_chip and use ima_tpm_chip != NULL instead

[Stefan Berger]

Get rid of ima_used_chip and use ima_tpm_chip variable instead for
determining whether to use the TPM chip.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 security/integrity/ima/ima.h        | 1 -
 security/integrity/ima/ima_crypto.c | 2 +-
 security/integrity/ima/ima_init.c   | 7 ++-----
 security/integrity/ima/ima_queue.c  | 2 +-
 4 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 53a88d578ca5..18491c0e9b97 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -54,7 +54,6 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 };
 extern int ima_policy_flag;
 
 /* set during initialization */
-extern int ima_used_chip;
 extern int ima_hash_algo;
 extern int ima_appraise;
 extern struct rw_semaphore ima_tpm_chip_lock;
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index da7715240476..bc6ad6b3243a 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -635,7 +635,7 @@ static void __init ima_pcrread(int idx, u8 *pcr)
 
 	down_read(&ima_tpm_chip_lock);
 
-	if (!ima_used_chip)
+	if (!ima_tpm_chip)
 		goto out;
 
 	result = tpm_pcr_read(ima_tpm_chip, idx, pcr);
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 24db06c4f463..84fba44c4415 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -27,7 +27,6 @@
 
 /* name for boot aggregate entry */
 static const char *boot_aggregate_name = "boot_aggregate";
-int ima_used_chip;
 struct rw_semaphore ima_tpm_chip_lock = __RWSEM_INITIALIZER(ima_tpm_chip_lock);
 struct tpm_chip *ima_tpm_chip;
 
@@ -67,7 +66,7 @@ static int __init ima_add_boot_aggregate(void)
 	iint->ima_hash->algo = HASH_ALGO_SHA1;
 	iint->ima_hash->length = SHA1_DIGEST_SIZE;
 
-	if (ima_used_chip) {
+	if (ima_tpm_chip) {
 		result = ima_calc_boot_aggregate(&hash.hdr);
 		if (result < 0) {
 			audit_cause = "hashing_error";
@@ -114,7 +113,6 @@ static int ima_shutdown(struct notifier_block *this, unsigned long action,
 	if (ima_tpm_chip) {
 		tpm_chip_put(ima_tpm_chip);
 		ima_tpm_chip = NULL;
-		ima_used_chip = 0;
 	}
 	up_write(&ima_tpm_chip_lock);
 	return NOTIFY_DONE;
@@ -133,8 +131,7 @@ int __init ima_init(void)
 
 	ima_tpm_chip = tpm_chip_find();
 
-	ima_used_chip = (ima_tpm_chip != NULL);
-	if (!ima_used_chip)
+	if (!ima_tpm_chip)
 		pr_info("No TPM chip found, activating TPM-bypass!\n");
 
 	rc = integrity_init_keyring(INTEGRITY_KEYRING_IMA);
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 6c9427939a28..591ff3d76fe6 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -143,7 +143,7 @@ static int ima_pcr_extend(const u8 *hash, int pcr)
 	int result = 0;
 
 	down_read(&ima_tpm_chip_lock);
-	if (!ima_used_chip)
+	if (!ima_tpm_chip)
 		goto out;
 
 	result = tpm_pcr_extend(ima_tpm_chip, pcr, hash);
-- 
2.13.6

Re: [PATCH 1/6] tpm: Introduce a kref for the tpm_chip

[Jason Gunthorpe]

On Wed, Jun 20, 2018 at 12:19:43PM -0400, Stefan Berger wrote:
> Introduce a kref for the tpm_chip that we initialize when the tpm_chip has
> been allocated and release before the tpm_chip is to be freed.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>  drivers/char/tpm/tpm-chip.c | 24 +++++++++++++++++++-----
>  drivers/char/tpm/tpm.h      |  1 +
>  2 files changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 0a62c19937b6..a933676194a4 100644
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -119,8 +119,24 @@ struct tpm_chip *tpm_chip_find_get(struct tpm_chip *chip)
>  	return res;
>  }
>  
> +static void tpm_chip_free(struct kref *kref)
> +{
> +	struct tpm_chip *chip = container_of(kref, struct tpm_chip, kref);
> +
> +	kfree(chip->log.bios_event_log);
> +	kfree(chip->work_space.context_buf);
> +	kfree(chip->work_space.session_buf);
> +	kfree(chip);
> +}
> +
> +static void tpm_chip_put(struct tpm_chip *chip)
> +{
> +	if (chip)
> +		kref_put(&chip->kref, tpm_chip_free);
> +}
> +
>  /**
> - * tpm_dev_release() - free chip memory and the device number
> + * tpm_dev_release() - free the device number and release reference to chip
>   * @dev: the character device for the TPM chip
>   *
>   * This is used as the release function for the character device.
> @@ -133,10 +149,7 @@ static void tpm_dev_release(struct device *dev)
>  	idr_remove(&dev_nums_idr, chip->dev_num);
>  	mutex_unlock(&idr_lock);
>  
> -	kfree(chip->log.bios_event_log);
> -	kfree(chip->work_space.context_buf);
> -	kfree(chip->work_space.session_buf);
> -	kfree(chip);
> +	tpm_chip_put(chip);
>  }
>  static void tpm_devs_release(struct device *dev)
> @@ -195,6 +208,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
>  
>  	mutex_init(&chip->tpm_mutex);
>  	init_rwsem(&chip->ops_sem);
> +	kref_init(&chip->kref);
>  
>  	chip->ops = ops;
>  
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 7f2d0f489e9c..098d7dcc04a4 100644
> +++ b/drivers/char/tpm/tpm.h
> @@ -211,6 +211,7 @@ struct tpm_chip {
>  	struct cdev cdev;
>  	struct cdev cdevs;
>  
> +	struct kref kref;

NAK, there is already a kref in struct device, that one must be used.

Jason

Re: [PATCH 1/6] tpm: Introduce a kref for the tpm_chip

[Stefan Berger]

On 06/20/2018 02:38 PM, Jason Gunthorpe wrote:
> On Wed, Jun 20, 2018 at 12:19:43PM -0400, Stefan Berger wrote:
>> Introduce a kref for the tpm_chip that we initialize when the tpm_chip has
>> been allocated and release before the tpm_chip is to be freed.
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>>   drivers/char/tpm/tpm-chip.c | 24 +++++++++++++++++++-----
>>   drivers/char/tpm/tpm.h      |  1 +
>>   2 files changed, 20 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
>> index 0a62c19937b6..a933676194a4 100644
>> +++ b/drivers/char/tpm/tpm-chip.c
>> @@ -119,8 +119,24 @@ struct tpm_chip *tpm_chip_find_get(struct tpm_chip *chip)
>>   	return res;
>>   }
>>   
>> +static void tpm_chip_free(struct kref *kref)
>> +{
>> +	struct tpm_chip *chip = container_of(kref, struct tpm_chip, kref);
>> +
>> +	kfree(chip->log.bios_event_log);
>> +	kfree(chip->work_space.context_buf);
>> +	kfree(chip->work_space.session_buf);
>> +	kfree(chip);
>> +}
>> +
>> +static void tpm_chip_put(struct tpm_chip *chip)
>> +{
>> +	if (chip)
>> +		kref_put(&chip->kref, tpm_chip_free);
>> +}
>> +
>>   /**
>> - * tpm_dev_release() - free chip memory and the device number
>> + * tpm_dev_release() - free the device number and release reference to chip
>>    * @dev: the character device for the TPM chip
>>    *
>>    * This is used as the release function for the character device.
>> @@ -133,10 +149,7 @@ static void tpm_dev_release(struct device *dev)
>>   	idr_remove(&dev_nums_idr, chip->dev_num);
>>   	mutex_unlock(&idr_lock);
>>   
>> -	kfree(chip->log.bios_event_log);
>> -	kfree(chip->work_space.context_buf);
>> -	kfree(chip->work_space.session_buf);
>> -	kfree(chip);
>> +	tpm_chip_put(chip);
>>   }
>>   static void tpm_devs_release(struct device *dev)
>> @@ -195,6 +208,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
>>   
>>   	mutex_init(&chip->tpm_mutex);
>>   	init_rwsem(&chip->ops_sem);
>> +	kref_init(&chip->kref);
>>   
>>   	chip->ops = ops;
>>   
>> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
>> index 7f2d0f489e9c..098d7dcc04a4 100644
>> +++ b/drivers/char/tpm/tpm.h
>> @@ -211,6 +211,7 @@ struct tpm_chip {
>>   	struct cdev cdev;
>>   	struct cdev cdevs;
>>   
>> +	struct kref kref;
> NAK, there is already a kref in struct device, that one must be used.

Right. Should make it simpler...


>
> Jason
>

Re: [PATCH 1/6] tpm: Introduce a kref for the tpm_chip

[Jarkko Sakkinen]

On Wed, Jun 20, 2018 at 03:34:36PM -0400, Stefan Berger wrote:
> On 06/20/2018 02:38 PM, Jason Gunthorpe wrote:
> > On Wed, Jun 20, 2018 at 12:19:43PM -0400, Stefan Berger wrote:
> > > Introduce a kref for the tpm_chip that we initialize when the tpm_chip has
> > > been allocated and release before the tpm_chip is to be freed.
> > > 
> > > Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> > >   drivers/char/tpm/tpm-chip.c | 24 +++++++++++++++++++-----
> > >   drivers/char/tpm/tpm.h      |  1 +
> > >   2 files changed, 20 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > > index 0a62c19937b6..a933676194a4 100644
> > > +++ b/drivers/char/tpm/tpm-chip.c
> > > @@ -119,8 +119,24 @@ struct tpm_chip *tpm_chip_find_get(struct tpm_chip *chip)
> > >   	return res;
> > >   }
> > > +static void tpm_chip_free(struct kref *kref)
> > > +{
> > > +	struct tpm_chip *chip = container_of(kref, struct tpm_chip, kref);
> > > +
> > > +	kfree(chip->log.bios_event_log);
> > > +	kfree(chip->work_space.context_buf);
> > > +	kfree(chip->work_space.session_buf);
> > > +	kfree(chip);
> > > +}
> > > +
> > > +static void tpm_chip_put(struct tpm_chip *chip)
> > > +{
> > > +	if (chip)
> > > +		kref_put(&chip->kref, tpm_chip_free);
> > > +}
> > > +
> > >   /**
> > > - * tpm_dev_release() - free chip memory and the device number
> > > + * tpm_dev_release() - free the device number and release reference to chip
> > >    * @dev: the character device for the TPM chip
> > >    *
> > >    * This is used as the release function for the character device.
> > > @@ -133,10 +149,7 @@ static void tpm_dev_release(struct device *dev)
> > >   	idr_remove(&dev_nums_idr, chip->dev_num);
> > >   	mutex_unlock(&idr_lock);
> > > -	kfree(chip->log.bios_event_log);
> > > -	kfree(chip->work_space.context_buf);
> > > -	kfree(chip->work_space.session_buf);
> > > -	kfree(chip);
> > > +	tpm_chip_put(chip);
> > >   }
> > >   static void tpm_devs_release(struct device *dev)
> > > @@ -195,6 +208,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
> > >   	mutex_init(&chip->tpm_mutex);
> > >   	init_rwsem(&chip->ops_sem);
> > > +	kref_init(&chip->kref);
> > >   	chip->ops = ops;
> > > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > > index 7f2d0f489e9c..098d7dcc04a4 100644
> > > +++ b/drivers/char/tpm/tpm.h
> > > @@ -211,6 +211,7 @@ struct tpm_chip {
> > >   	struct cdev cdev;
> > >   	struct cdev cdevs;
> > > +	struct kref kref;
> > NAK, there is already a kref in struct device, that one must be used.
> 
> Right. Should make it simpler...

I'll review the next version.

/Jarkko