2018-06-20 16:22:38

by Stefan Berger

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

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



2018-06-20 16:21:37

by Stefan Berger

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

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 <[email protected]>
---
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


2018-06-20 16:21:53

by Stefan Berger

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

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 <[email protected]>
---
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


2018-06-20 16:22:35

by Stefan Berger

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

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 <[email protected]>
---
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


2018-06-20 16:22:43

by Stefan Berger

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

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 <[email protected]>
---
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


2018-06-20 16:23:08

by Stefan Berger

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

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 <[email protected]>
---
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


2018-06-20 16:23:36

by Stefan Berger

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

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 <[email protected]>
---
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


2018-06-20 18:39:58

by Jason Gunthorpe

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

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 <[email protected]>
> 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

2018-06-20 19:36:59

by Stefan Berger

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

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 <[email protected]>
>> 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
>


2018-06-21 16:52:39

by Jarkko Sakkinen

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

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 <[email protected]>
> > > 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