* [PATCH] tpm: fix a race condition in tpm2_unseal_trusted()
@ 2016-08-28 6:36 ` Jarkko Sakkinen
0 siblings, 0 replies; 17+ messages in thread
From: Jarkko Sakkinen @ 2016-08-28 6:36 UTC (permalink / raw)
To: Peter Huewe
Cc: linux-security-module, Jarkko Sakkinen, stable, Marcel Selhorst,
Jason Gunthorpe, moderated list:TPM DEVICE DRIVER, open list
Unseal and load operations should be done as an atomic operation. This
commit introduces unlocked tpm_transmit() so that tpm2_unseal_trusted()
can do the locking by itself.
v2: Introduced an unlocked unseal operation instead of changing locking
strategy in order to make less intrusive bug fix and thus more
backportable.
v3: Have also separate __tpm_transmit() that takes 'flags' in order to
better localize the bug fix and make it easier to backport.
v4: Cleaned up the control flow in tpm2_unseal_trusted. Added the
missing 'Fixes' line.
v5: Previous versions did not set the enum to any value, so it was set
to zero. Changed enum to TPM_TRANSMIT_UNLOCKED because it is
guaranteed to catch such situation. With change the common case
adds so little clutter that the wrappers do not make sense anymore.
Fixes: 0fe5480303a1 ("keys, trusted: seal/unseal with TPM 2.0 chips")
CC: stable@vger.kernel.org
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
drivers/char/tpm/tpm-dev.c | 2 +-
drivers/char/tpm/tpm-interface.c | 51 ++++++++++++++++++++++------------------
drivers/char/tpm/tpm-sysfs.c | 2 +-
drivers/char/tpm/tpm.h | 12 ++++++----
drivers/char/tpm/tpm2-cmd.c | 34 ++++++++++++++++-----------
5 files changed, 58 insertions(+), 43 deletions(-)
diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
index f5d4521..912ad30 100644
--- a/drivers/char/tpm/tpm-dev.c
+++ b/drivers/char/tpm/tpm-dev.c
@@ -145,7 +145,7 @@ static ssize_t tpm_write(struct file *file, const char __user *buf,
return -EPIPE;
}
out_size = tpm_transmit(priv->chip, priv->data_buffer,
- sizeof(priv->data_buffer));
+ sizeof(priv->data_buffer), 0);
tpm_put_ops(priv->chip);
if (out_size < 0) {
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 1abe2d7..aef20ee 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -330,8 +330,8 @@ EXPORT_SYMBOL_GPL(tpm_calc_ordinal_duration);
/*
* Internal kernel interface to transmit TPM commands
*/
-ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
- size_t bufsiz)
+ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
+ unsigned int flags)
{
ssize_t rc;
u32 count, ordinal;
@@ -350,7 +350,8 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
return -E2BIG;
}
- mutex_lock(&chip->tpm_mutex);
+ if (!(flags & TPM_TRANSMIT_UNLOCKED))
+ mutex_lock(&chip->tpm_mutex);
rc = chip->ops->send(chip, (u8 *) buf, count);
if (rc < 0) {
@@ -393,20 +394,21 @@ out_recv:
dev_err(&chip->dev,
"tpm_transmit: tpm_recv: error %zd\n", rc);
out:
- mutex_unlock(&chip->tpm_mutex);
+ if (!(flags & TPM_TRANSMIT_UNLOCKED))
+ mutex_unlock(&chip->tpm_mutex);
return rc;
}
#define TPM_DIGEST_SIZE 20
#define TPM_RET_CODE_IDX 6
-ssize_t tpm_transmit_cmd(struct tpm_chip *chip, void *cmd,
- int len, const char *desc)
+ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void *cmd,
+ int len, unsigned int flags, const char *desc)
{
- struct tpm_output_header *header;
+ const struct tpm_output_header *header;
int err;
- len = tpm_transmit(chip, (u8 *) cmd, len);
+ len = tpm_transmit(chip, (const u8 *)cmd, len, flags);
if (len < 0)
return len;
else if (len < TPM_HEADER_SIZE)
@@ -453,7 +455,8 @@ ssize_t tpm_getcap(struct tpm_chip *chip, __be32 subcap_id, cap_t *cap,
tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
tpm_cmd.params.getcap_in.subcap = subcap_id;
}
- rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, desc);
+ rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, 0,
+ desc);
if (!rc)
*cap = tpm_cmd.params.getcap_out.cap;
return rc;
@@ -469,7 +472,7 @@ void tpm_gen_interrupt(struct tpm_chip *chip)
tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_TIMEOUT;
- rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
+ rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, 0,
"attempting to determine the timeouts");
}
EXPORT_SYMBOL_GPL(tpm_gen_interrupt);
@@ -490,7 +493,7 @@ static int tpm_startup(struct tpm_chip *chip, __be16 startup_type)
start_cmd.header.in = tpm_startup_header;
start_cmd.params.startup_in.startup_type = startup_type;
- return tpm_transmit_cmd(chip, &start_cmd, TPM_INTERNAL_RESULT_SIZE,
+ return tpm_transmit_cmd(chip, &start_cmd, TPM_INTERNAL_RESULT_SIZE, 0,
"attempting to start the TPM");
}
@@ -521,7 +524,8 @@ int tpm_get_timeouts(struct tpm_chip *chip)
tpm_cmd.params.getcap_in.cap = TPM_CAP_PROP;
tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_TIMEOUT;
- rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, NULL);
+ rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, 0,
+ NULL);
if (rc == TPM_ERR_INVALID_POSTINIT) {
/* The TPM is not started, we are the first to talk to it.
@@ -535,7 +539,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_TIMEOUT;
rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
- NULL);
+ 0, NULL);
}
if (rc) {
dev_err(&chip->dev,
@@ -596,7 +600,7 @@ duration:
tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_DURATION;
- rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
+ rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, 0,
"attempting to determine the durations");
if (rc)
return rc;
@@ -652,7 +656,7 @@ static int tpm_continue_selftest(struct tpm_chip *chip)
struct tpm_cmd_t cmd;
cmd.header.in = continue_selftest_header;
- rc = tpm_transmit_cmd(chip, &cmd, CONTINUE_SELFTEST_RESULT_SIZE,
+ rc = tpm_transmit_cmd(chip, &cmd, CONTINUE_SELFTEST_RESULT_SIZE, 0,
"continue selftest");
return rc;
}
@@ -672,7 +676,7 @@ int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
cmd.header.in = pcrread_header;
cmd.params.pcrread_in.pcr_idx = cpu_to_be32(pcr_idx);
- rc = tpm_transmit_cmd(chip, &cmd, READ_PCR_RESULT_SIZE,
+ rc = tpm_transmit_cmd(chip, &cmd, READ_PCR_RESULT_SIZE, 0,
"attempting to read a pcr value");
if (rc == 0)
@@ -770,7 +774,7 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
cmd.header.in = pcrextend_header;
cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(pcr_idx);
memcpy(cmd.params.pcrextend_in.hash, hash, TPM_DIGEST_SIZE);
- rc = tpm_transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE,
+ rc = tpm_transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE, 0,
"attempting extend a PCR value");
tpm_put_ops(chip);
@@ -809,7 +813,7 @@ int tpm_do_selftest(struct tpm_chip *chip)
/* Attempt to read a PCR value */
cmd.header.in = pcrread_header;
cmd.params.pcrread_in.pcr_idx = cpu_to_be32(0);
- rc = tpm_transmit(chip, (u8 *) &cmd, READ_PCR_RESULT_SIZE);
+ rc = tpm_transmit(chip, (u8 *) &cmd, READ_PCR_RESULT_SIZE, 0);
/* Some buggy TPMs will not respond to tpm_tis_ready() for
* around 300ms while the self test is ongoing, keep trying
* until the self test duration expires. */
@@ -879,7 +883,7 @@ int tpm_send(u32 chip_num, void *cmd, size_t buflen)
if (chip == NULL)
return -ENODEV;
- rc = tpm_transmit_cmd(chip, cmd, buflen, "attempting tpm_cmd");
+ rc = tpm_transmit_cmd(chip, cmd, buflen, 0, "attempting tpm_cmd");
tpm_put_ops(chip);
return rc;
@@ -981,14 +985,15 @@ int tpm_pm_suspend(struct device *dev)
cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(tpm_suspend_pcr);
memcpy(cmd.params.pcrextend_in.hash, dummy_hash,
TPM_DIGEST_SIZE);
- rc = tpm_transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE,
+ rc = tpm_transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE, 0,
"extending dummy pcr before suspend");
}
/* now do the actual savestate */
for (try = 0; try < TPM_RETRY; try++) {
cmd.header.in = savestate_header;
- rc = tpm_transmit_cmd(chip, &cmd, SAVESTATE_RESULT_SIZE, NULL);
+ rc = tpm_transmit_cmd(chip, &cmd, SAVESTATE_RESULT_SIZE, 0,
+ NULL);
/*
* If the TPM indicates that it is too busy to respond to
@@ -1072,8 +1077,8 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
tpm_cmd.params.getrandom_in.num_bytes = cpu_to_be32(num_bytes);
err = tpm_transmit_cmd(chip, &tpm_cmd,
- TPM_GETRANDOM_RESULT_SIZE + num_bytes,
- "attempting get random");
+ TPM_GETRANDOM_RESULT_SIZE + num_bytes,
+ 0, "attempting get random");
if (err)
break;
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index b46cf70..e1f7236 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -39,7 +39,7 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
struct tpm_chip *chip = to_tpm_chip(dev);
tpm_cmd.header.in = tpm_readpubek_header;
- err = tpm_transmit_cmd(chip, &tpm_cmd, READ_PUBEK_RESULT_SIZE,
+ err = tpm_transmit_cmd(chip, &tpm_cmd, READ_PUBEK_RESULT_SIZE, 0,
"attempting to read the PUBEK");
if (err)
goto out;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 3e32d5b..b0585e9 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -476,12 +476,16 @@ extern dev_t tpm_devt;
extern const struct file_operations tpm_fops;
extern struct idr dev_nums_idr;
+enum tpm_transmit_flags {
+ TPM_TRANSMIT_UNLOCKED = BIT(0),
+};
+
+ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
+ unsigned int flags);
+ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void *cmd, int len,
+ unsigned int flags, const char *desc);
ssize_t tpm_getcap(struct tpm_chip *chip, __be32 subcap_id, cap_t *cap,
const char *desc);
-ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
- size_t bufsiz);
-ssize_t tpm_transmit_cmd(struct tpm_chip *chip, void *cmd, int len,
- const char *desc);
extern int tpm_get_timeouts(struct tpm_chip *);
extern void tpm_gen_interrupt(struct tpm_chip *);
int tpm1_auto_startup(struct tpm_chip *chip);
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 08c7e23..5452ba7 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -282,7 +282,7 @@ int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
sizeof(cmd.params.pcrread_in.pcr_select));
cmd.params.pcrread_in.pcr_select[pcr_idx >> 3] = 1 << (pcr_idx & 0x7);
- rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd),
+ rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0,
"attempting to read a pcr value");
if (rc == 0) {
buf = cmd.params.pcrread_out.digest;
@@ -330,7 +330,7 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash)
cmd.params.pcrextend_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
memcpy(cmd.params.pcrextend_in.digest, hash, TPM_DIGEST_SIZE);
- rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd),
+ rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0,
"attempting extend a PCR value");
return rc;
@@ -376,7 +376,7 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
cmd.header.in = tpm2_getrandom_header;
cmd.params.getrandom_in.size = cpu_to_be16(num_bytes);
- err = tpm_transmit_cmd(chip, &cmd, sizeof(cmd),
+ err = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0,
"attempting get random");
if (err)
break;
@@ -512,7 +512,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
goto out;
}
- rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "sealing data");
+ rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 0, "sealing data");
if (rc)
goto out;
@@ -576,7 +576,8 @@ static int tpm2_load(struct tpm_chip *chip,
goto out;
}
- rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "loading blob");
+ rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, TPM_TRANSMIT_UNLOCKED,
+ "loading blob");
if (!rc)
*blob_handle = be32_to_cpup(
(__be32 *) &buf.data[TPM_HEADER_SIZE]);
@@ -604,7 +605,8 @@ static void tpm2_flush_context(struct tpm_chip *chip, u32 handle)
tpm_buf_append_u32(&buf, handle);
- rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "flushing context");
+ rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, TPM_TRANSMIT_UNLOCKED,
+ "flushing context");
if (rc)
dev_warn(&chip->dev, "0x%08x was not flushed, rc=%d\n", handle,
rc);
@@ -635,7 +637,8 @@ static int tpm2_unseal(struct tpm_chip *chip,
options->blobauth /* hmac */,
TPM_DIGEST_SIZE);
- rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "unsealing");
+ rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, TPM_TRANSMIT_UNLOCKED,
+ "unsealing");
if (rc > 0)
rc = -EPERM;
@@ -668,14 +671,17 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
u32 blob_handle;
int rc;
+ mutex_lock(&chip->tpm_mutex);
rc = tpm2_load(chip, payload, options, &blob_handle);
if (rc)
- return rc;
+ goto out;
rc = tpm2_unseal(chip, payload, options, blob_handle);
tpm2_flush_context(chip, blob_handle);
+out:
+ mutex_unlock(&chip->tpm_mutex);
return rc;
}
@@ -701,7 +707,7 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id, u32 *value,
cmd.params.get_tpm_pt_in.property_id = cpu_to_be32(property_id);
cmd.params.get_tpm_pt_in.property_cnt = cpu_to_be32(1);
- rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), desc);
+ rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0, desc);
if (!rc)
*value = be32_to_cpu(cmd.params.get_tpm_pt_out.value);
@@ -735,7 +741,7 @@ static int tpm2_startup(struct tpm_chip *chip, u16 startup_type)
cmd.header.in = tpm2_startup_header;
cmd.params.startup_in.startup_type = cpu_to_be16(startup_type);
- return tpm_transmit_cmd(chip, &cmd, sizeof(cmd),
+ return tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0,
"attempting to start the TPM");
}
@@ -763,7 +769,7 @@ void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
cmd.header.in = tpm2_shutdown_header;
cmd.params.startup_in.startup_type = cpu_to_be16(shutdown_type);
- rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), "stopping the TPM");
+ rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0, "stopping the TPM");
/* In places where shutdown command is sent there's no much we can do
* except print the error code on a system failure.
@@ -828,7 +834,7 @@ static int tpm2_start_selftest(struct tpm_chip *chip, bool full)
cmd.header.in = tpm2_selftest_header;
cmd.params.selftest_in.full_test = full;
- rc = tpm_transmit_cmd(chip, &cmd, TPM2_SELF_TEST_IN_SIZE,
+ rc = tpm_transmit_cmd(chip, &cmd, TPM2_SELF_TEST_IN_SIZE, 0,
"continue selftest");
/* At least some prototype chips seem to give RC_TESTING error
@@ -880,7 +886,7 @@ static int tpm2_do_selftest(struct tpm_chip *chip)
cmd.params.pcrread_in.pcr_select[1] = 0x00;
cmd.params.pcrread_in.pcr_select[2] = 0x00;
- rc = tpm_transmit_cmd(chip, (u8 *) &cmd, sizeof(cmd), NULL);
+ rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0, NULL);
if (rc < 0)
break;
@@ -928,7 +934,7 @@ int tpm2_probe(struct tpm_chip *chip)
cmd.params.get_tpm_pt_in.property_id = cpu_to_be32(0x100);
cmd.params.get_tpm_pt_in.property_cnt = cpu_to_be32(1);
- rc = tpm_transmit(chip, (const char *) &cmd, sizeof(cmd));
+ rc = tpm_transmit(chip, (const u8 *)&cmd, sizeof(cmd), 0);
if (rc < 0)
return rc;
else if (rc < TPM_HEADER_SIZE)
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH] tpm: fix a race condition in tpm2_unseal_trusted()
@ 2016-08-28 6:36 ` Jarkko Sakkinen
0 siblings, 0 replies; 17+ messages in thread
From: Jarkko Sakkinen @ 2016-08-28 6:36 UTC (permalink / raw)
To: Peter Huewe
Cc: linux-security-module, Jarkko Sakkinen, stable, Marcel Selhorst,
Jason Gunthorpe, moderated list:TPM DEVICE DRIVER, open list
Unseal and load operations should be done as an atomic operation. This
commit introduces unlocked tpm_transmit() so that tpm2_unseal_trusted()
can do the locking by itself.
v2: Introduced an unlocked unseal operation instead of changing locking
strategy in order to make less intrusive bug fix and thus more
backportable.
v3: Have also separate __tpm_transmit() that takes 'flags' in order to
better localize the bug fix and make it easier to backport.
v4: Cleaned up the control flow in tpm2_unseal_trusted. Added the
missing 'Fixes' line.
v5: Previous versions did not set the enum to any value, so it was set
to zero. Changed enum to TPM_TRANSMIT_UNLOCKED because it is
guaranteed to catch such situation. With change the common case
adds so little clutter that the wrappers do not make sense anymore.
Fixes: 0fe5480303a1 ("keys, trusted: seal/unseal with TPM 2.0 chips")
CC: stable@vger.kernel.org
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
drivers/char/tpm/tpm-dev.c | 2 +-
drivers/char/tpm/tpm-interface.c | 51 ++++++++++++++++++++++------------------
drivers/char/tpm/tpm-sysfs.c | 2 +-
drivers/char/tpm/tpm.h | 12 ++++++----
drivers/char/tpm/tpm2-cmd.c | 34 ++++++++++++++++-----------
5 files changed, 58 insertions(+), 43 deletions(-)
diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
index f5d4521..912ad30 100644
--- a/drivers/char/tpm/tpm-dev.c
+++ b/drivers/char/tpm/tpm-dev.c
@@ -145,7 +145,7 @@ static ssize_t tpm_write(struct file *file, const char __user *buf,
return -EPIPE;
}
out_size = tpm_transmit(priv->chip, priv->data_buffer,
- sizeof(priv->data_buffer));
+ sizeof(priv->data_buffer), 0);
tpm_put_ops(priv->chip);
if (out_size < 0) {
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 1abe2d7..aef20ee 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -330,8 +330,8 @@ EXPORT_SYMBOL_GPL(tpm_calc_ordinal_duration);
/*
* Internal kernel interface to transmit TPM commands
*/
-ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
- size_t bufsiz)
+ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
+ unsigned int flags)
{
ssize_t rc;
u32 count, ordinal;
@@ -350,7 +350,8 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
return -E2BIG;
}
- mutex_lock(&chip->tpm_mutex);
+ if (!(flags & TPM_TRANSMIT_UNLOCKED))
+ mutex_lock(&chip->tpm_mutex);
rc = chip->ops->send(chip, (u8 *) buf, count);
if (rc < 0) {
@@ -393,20 +394,21 @@ out_recv:
dev_err(&chip->dev,
"tpm_transmit: tpm_recv: error %zd\n", rc);
out:
- mutex_unlock(&chip->tpm_mutex);
+ if (!(flags & TPM_TRANSMIT_UNLOCKED))
+ mutex_unlock(&chip->tpm_mutex);
return rc;
}
#define TPM_DIGEST_SIZE 20
#define TPM_RET_CODE_IDX 6
-ssize_t tpm_transmit_cmd(struct tpm_chip *chip, void *cmd,
- int len, const char *desc)
+ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void *cmd,
+ int len, unsigned int flags, const char *desc)
{
- struct tpm_output_header *header;
+ const struct tpm_output_header *header;
int err;
- len = tpm_transmit(chip, (u8 *) cmd, len);
+ len = tpm_transmit(chip, (const u8 *)cmd, len, flags);
if (len < 0)
return len;
else if (len < TPM_HEADER_SIZE)
@@ -453,7 +455,8 @@ ssize_t tpm_getcap(struct tpm_chip *chip, __be32 subcap_id, cap_t *cap,
tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
tpm_cmd.params.getcap_in.subcap = subcap_id;
}
- rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, desc);
+ rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, 0,
+ desc);
if (!rc)
*cap = tpm_cmd.params.getcap_out.cap;
return rc;
@@ -469,7 +472,7 @@ void tpm_gen_interrupt(struct tpm_chip *chip)
tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_TIMEOUT;
- rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
+ rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, 0,
"attempting to determine the timeouts");
}
EXPORT_SYMBOL_GPL(tpm_gen_interrupt);
@@ -490,7 +493,7 @@ static int tpm_startup(struct tpm_chip *chip, __be16 startup_type)
start_cmd.header.in = tpm_startup_header;
start_cmd.params.startup_in.startup_type = startup_type;
- return tpm_transmit_cmd(chip, &start_cmd, TPM_INTERNAL_RESULT_SIZE,
+ return tpm_transmit_cmd(chip, &start_cmd, TPM_INTERNAL_RESULT_SIZE, 0,
"attempting to start the TPM");
}
@@ -521,7 +524,8 @@ int tpm_get_timeouts(struct tpm_chip *chip)
tpm_cmd.params.getcap_in.cap = TPM_CAP_PROP;
tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_TIMEOUT;
- rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, NULL);
+ rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, 0,
+ NULL);
if (rc == TPM_ERR_INVALID_POSTINIT) {
/* The TPM is not started, we are the first to talk to it.
@@ -535,7 +539,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_TIMEOUT;
rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
- NULL);
+ 0, NULL);
}
if (rc) {
dev_err(&chip->dev,
@@ -596,7 +600,7 @@ duration:
tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_DURATION;
- rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
+ rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, 0,
"attempting to determine the durations");
if (rc)
return rc;
@@ -652,7 +656,7 @@ static int tpm_continue_selftest(struct tpm_chip *chip)
struct tpm_cmd_t cmd;
cmd.header.in = continue_selftest_header;
- rc = tpm_transmit_cmd(chip, &cmd, CONTINUE_SELFTEST_RESULT_SIZE,
+ rc = tpm_transmit_cmd(chip, &cmd, CONTINUE_SELFTEST_RESULT_SIZE, 0,
"continue selftest");
return rc;
}
@@ -672,7 +676,7 @@ int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
cmd.header.in = pcrread_header;
cmd.params.pcrread_in.pcr_idx = cpu_to_be32(pcr_idx);
- rc = tpm_transmit_cmd(chip, &cmd, READ_PCR_RESULT_SIZE,
+ rc = tpm_transmit_cmd(chip, &cmd, READ_PCR_RESULT_SIZE, 0,
"attempting to read a pcr value");
if (rc == 0)
@@ -770,7 +774,7 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
cmd.header.in = pcrextend_header;
cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(pcr_idx);
memcpy(cmd.params.pcrextend_in.hash, hash, TPM_DIGEST_SIZE);
- rc = tpm_transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE,
+ rc = tpm_transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE, 0,
"attempting extend a PCR value");
tpm_put_ops(chip);
@@ -809,7 +813,7 @@ int tpm_do_selftest(struct tpm_chip *chip)
/* Attempt to read a PCR value */
cmd.header.in = pcrread_header;
cmd.params.pcrread_in.pcr_idx = cpu_to_be32(0);
- rc = tpm_transmit(chip, (u8 *) &cmd, READ_PCR_RESULT_SIZE);
+ rc = tpm_transmit(chip, (u8 *) &cmd, READ_PCR_RESULT_SIZE, 0);
/* Some buggy TPMs will not respond to tpm_tis_ready() for
* around 300ms while the self test is ongoing, keep trying
* until the self test duration expires. */
@@ -879,7 +883,7 @@ int tpm_send(u32 chip_num, void *cmd, size_t buflen)
if (chip == NULL)
return -ENODEV;
- rc = tpm_transmit_cmd(chip, cmd, buflen, "attempting tpm_cmd");
+ rc = tpm_transmit_cmd(chip, cmd, buflen, 0, "attempting tpm_cmd");
tpm_put_ops(chip);
return rc;
@@ -981,14 +985,15 @@ int tpm_pm_suspend(struct device *dev)
cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(tpm_suspend_pcr);
memcpy(cmd.params.pcrextend_in.hash, dummy_hash,
TPM_DIGEST_SIZE);
- rc = tpm_transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE,
+ rc = tpm_transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE, 0,
"extending dummy pcr before suspend");
}
/* now do the actual savestate */
for (try = 0; try < TPM_RETRY; try++) {
cmd.header.in = savestate_header;
- rc = tpm_transmit_cmd(chip, &cmd, SAVESTATE_RESULT_SIZE, NULL);
+ rc = tpm_transmit_cmd(chip, &cmd, SAVESTATE_RESULT_SIZE, 0,
+ NULL);
/*
* If the TPM indicates that it is too busy to respond to
@@ -1072,8 +1077,8 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
tpm_cmd.params.getrandom_in.num_bytes = cpu_to_be32(num_bytes);
err = tpm_transmit_cmd(chip, &tpm_cmd,
- TPM_GETRANDOM_RESULT_SIZE + num_bytes,
- "attempting get random");
+ TPM_GETRANDOM_RESULT_SIZE + num_bytes,
+ 0, "attempting get random");
if (err)
break;
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index b46cf70..e1f7236 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -39,7 +39,7 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
struct tpm_chip *chip = to_tpm_chip(dev);
tpm_cmd.header.in = tpm_readpubek_header;
- err = tpm_transmit_cmd(chip, &tpm_cmd, READ_PUBEK_RESULT_SIZE,
+ err = tpm_transmit_cmd(chip, &tpm_cmd, READ_PUBEK_RESULT_SIZE, 0,
"attempting to read the PUBEK");
if (err)
goto out;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 3e32d5b..b0585e9 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -476,12 +476,16 @@ extern dev_t tpm_devt;
extern const struct file_operations tpm_fops;
extern struct idr dev_nums_idr;
+enum tpm_transmit_flags {
+ TPM_TRANSMIT_UNLOCKED = BIT(0),
+};
+
+ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
+ unsigned int flags);
+ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void *cmd, int len,
+ unsigned int flags, const char *desc);
ssize_t tpm_getcap(struct tpm_chip *chip, __be32 subcap_id, cap_t *cap,
const char *desc);
-ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
- size_t bufsiz);
-ssize_t tpm_transmit_cmd(struct tpm_chip *chip, void *cmd, int len,
- const char *desc);
extern int tpm_get_timeouts(struct tpm_chip *);
extern void tpm_gen_interrupt(struct tpm_chip *);
int tpm1_auto_startup(struct tpm_chip *chip);
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 08c7e23..5452ba7 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -282,7 +282,7 @@ int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
sizeof(cmd.params.pcrread_in.pcr_select));
cmd.params.pcrread_in.pcr_select[pcr_idx >> 3] = 1 << (pcr_idx & 0x7);
- rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd),
+ rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0,
"attempting to read a pcr value");
if (rc == 0) {
buf = cmd.params.pcrread_out.digest;
@@ -330,7 +330,7 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash)
cmd.params.pcrextend_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
memcpy(cmd.params.pcrextend_in.digest, hash, TPM_DIGEST_SIZE);
- rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd),
+ rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0,
"attempting extend a PCR value");
return rc;
@@ -376,7 +376,7 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
cmd.header.in = tpm2_getrandom_header;
cmd.params.getrandom_in.size = cpu_to_be16(num_bytes);
- err = tpm_transmit_cmd(chip, &cmd, sizeof(cmd),
+ err = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0,
"attempting get random");
if (err)
break;
@@ -512,7 +512,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
goto out;
}
- rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "sealing data");
+ rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 0, "sealing data");
if (rc)
goto out;
@@ -576,7 +576,8 @@ static int tpm2_load(struct tpm_chip *chip,
goto out;
}
- rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "loading blob");
+ rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, TPM_TRANSMIT_UNLOCKED,
+ "loading blob");
if (!rc)
*blob_handle = be32_to_cpup(
(__be32 *) &buf.data[TPM_HEADER_SIZE]);
@@ -604,7 +605,8 @@ static void tpm2_flush_context(struct tpm_chip *chip, u32 handle)
tpm_buf_append_u32(&buf, handle);
- rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "flushing context");
+ rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, TPM_TRANSMIT_UNLOCKED,
+ "flushing context");
if (rc)
dev_warn(&chip->dev, "0x%08x was not flushed, rc=%d\n", handle,
rc);
@@ -635,7 +637,8 @@ static int tpm2_unseal(struct tpm_chip *chip,
options->blobauth /* hmac */,
TPM_DIGEST_SIZE);
- rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "unsealing");
+ rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, TPM_TRANSMIT_UNLOCKED,
+ "unsealing");
if (rc > 0)
rc = -EPERM;
@@ -668,14 +671,17 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
u32 blob_handle;
int rc;
+ mutex_lock(&chip->tpm_mutex);
rc = tpm2_load(chip, payload, options, &blob_handle);
if (rc)
- return rc;
+ goto out;
rc = tpm2_unseal(chip, payload, options, blob_handle);
tpm2_flush_context(chip, blob_handle);
+out:
+ mutex_unlock(&chip->tpm_mutex);
return rc;
}
@@ -701,7 +707,7 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id, u32 *value,
cmd.params.get_tpm_pt_in.property_id = cpu_to_be32(property_id);
cmd.params.get_tpm_pt_in.property_cnt = cpu_to_be32(1);
- rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), desc);
+ rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0, desc);
if (!rc)
*value = be32_to_cpu(cmd.params.get_tpm_pt_out.value);
@@ -735,7 +741,7 @@ static int tpm2_startup(struct tpm_chip *chip, u16 startup_type)
cmd.header.in = tpm2_startup_header;
cmd.params.startup_in.startup_type = cpu_to_be16(startup_type);
- return tpm_transmit_cmd(chip, &cmd, sizeof(cmd),
+ return tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0,
"attempting to start the TPM");
}
@@ -763,7 +769,7 @@ void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
cmd.header.in = tpm2_shutdown_header;
cmd.params.startup_in.startup_type = cpu_to_be16(shutdown_type);
- rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), "stopping the TPM");
+ rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0, "stopping the TPM");
/* In places where shutdown command is sent there's no much we can do
* except print the error code on a system failure.
@@ -828,7 +834,7 @@ static int tpm2_start_selftest(struct tpm_chip *chip, bool full)
cmd.header.in = tpm2_selftest_header;
cmd.params.selftest_in.full_test = full;
- rc = tpm_transmit_cmd(chip, &cmd, TPM2_SELF_TEST_IN_SIZE,
+ rc = tpm_transmit_cmd(chip, &cmd, TPM2_SELF_TEST_IN_SIZE, 0,
"continue selftest");
/* At least some prototype chips seem to give RC_TESTING error
@@ -880,7 +886,7 @@ static int tpm2_do_selftest(struct tpm_chip *chip)
cmd.params.pcrread_in.pcr_select[1] = 0x00;
cmd.params.pcrread_in.pcr_select[2] = 0x00;
- rc = tpm_transmit_cmd(chip, (u8 *) &cmd, sizeof(cmd), NULL);
+ rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0, NULL);
if (rc < 0)
break;
@@ -928,7 +934,7 @@ int tpm2_probe(struct tpm_chip *chip)
cmd.params.get_tpm_pt_in.property_id = cpu_to_be32(0x100);
cmd.params.get_tpm_pt_in.property_cnt = cpu_to_be32(1);
- rc = tpm_transmit(chip, (const char *) &cmd, sizeof(cmd));
+ rc = tpm_transmit(chip, (const u8 *)&cmd, sizeof(cmd), 0);
if (rc < 0)
return rc;
else if (rc < TPM_HEADER_SIZE)
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] tpm: fix a race condition in tpm2_unseal_trusted()
2016-08-28 6:36 ` Jarkko Sakkinen
(?)
@ 2016-08-28 18:51 ` Jason Gunthorpe
-1 siblings, 0 replies; 17+ messages in thread
From: Jason Gunthorpe @ 2016-08-28 18:51 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Peter Huewe, linux-security-module, stable, Marcel Selhorst,
moderated list:TPM DEVICE DRIVER, open list
On Sun, Aug 28, 2016 at 08:36:52AM +0200, Jarkko Sakkinen wrote:
>
> @@ -576,7 +576,8 @@ static int tpm2_load(struct tpm_chip *chip,
> goto out;
> }
>
> - rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "loading blob");
> + rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, TPM_TRANSMIT_UNLOCKED,
> + "loading blob");
I still don't like this, required mutex's should not be split outside the
function that needs them without more a more obvious indication:
> + mutex_lock(&chip->tpm_mutex);
> rc = tpm2_load(chip, payload, options, &blob_handle);
> if (rc)
> - return rc;
I recommend you stick with the idiom and do this:
mutex_lock(&chip->tpm_mutex);
rc = tpm2_load(chip, payload, options, &blob_handle, TPM_TRANSMIT_UNLOCKED);
Which makes it easy to see we are doing it right everywhere.
Jason
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] tpm: fix a race condition in tpm2_unseal_trusted()
@ 2016-08-28 18:51 ` Jason Gunthorpe
0 siblings, 0 replies; 17+ messages in thread
From: Jason Gunthorpe @ 2016-08-28 18:51 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Peter Huewe, linux-security-module, stable, Marcel Selhorst,
moderated list:TPM DEVICE DRIVER, open list
On Sun, Aug 28, 2016 at 08:36:52AM +0200, Jarkko Sakkinen wrote:
>
> @@ -576,7 +576,8 @@ static int tpm2_load(struct tpm_chip *chip,
> goto out;
> }
>
> - rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "loading blob");
> + rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, TPM_TRANSMIT_UNLOCKED,
> + "loading blob");
I still don't like this, required mutex's should not be split outside the
function that needs them without more a more obvious indication:
> + mutex_lock(&chip->tpm_mutex);
> rc = tpm2_load(chip, payload, options, &blob_handle);
> if (rc)
> - return rc;
I recommend you stick with the idiom and do this:
mutex_lock(&chip->tpm_mutex);
rc = tpm2_load(chip, payload, options, &blob_handle, TPM_TRANSMIT_UNLOCKED);
Which makes it easy to see we are doing it right everywhere.
Jason
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] tpm: fix a race condition in tpm2_unseal_trusted()
@ 2016-08-28 18:51 ` Jason Gunthorpe
0 siblings, 0 replies; 17+ messages in thread
From: Jason Gunthorpe @ 2016-08-28 18:51 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: open list, stable-u79uwXL29TY76Z2rM5mHXA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
moderated list:TPM DEVICE DRIVER
On Sun, Aug 28, 2016 at 08:36:52AM +0200, Jarkko Sakkinen wrote:
>
> @@ -576,7 +576,8 @@ static int tpm2_load(struct tpm_chip *chip,
> goto out;
> }
>
> - rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "loading blob");
> + rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, TPM_TRANSMIT_UNLOCKED,
> + "loading blob");
I still don't like this, required mutex's should not be split outside the
function that needs them without more a more obvious indication:
> + mutex_lock(&chip->tpm_mutex);
> rc = tpm2_load(chip, payload, options, &blob_handle);
> if (rc)
> - return rc;
I recommend you stick with the idiom and do this:
mutex_lock(&chip->tpm_mutex);
rc = tpm2_load(chip, payload, options, &blob_handle, TPM_TRANSMIT_UNLOCKED);
Which makes it easy to see we are doing it right everywhere.
Jason
------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] tpm: fix a race condition in tpm2_unseal_trusted()
2016-08-28 18:51 ` Jason Gunthorpe
@ 2016-08-29 15:25 ` Jarkko Sakkinen
-1 siblings, 0 replies; 17+ messages in thread
From: Jarkko Sakkinen @ 2016-08-29 15:25 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Peter Huewe, linux-security-module, stable, Marcel Selhorst,
moderated list:TPM DEVICE DRIVER, open list
On Sun, Aug 28, 2016 at 12:51:49PM -0600, Jason Gunthorpe wrote:
> On Sun, Aug 28, 2016 at 08:36:52AM +0200, Jarkko Sakkinen wrote:
> >
> > @@ -576,7 +576,8 @@ static int tpm2_load(struct tpm_chip *chip,
> > goto out;
> > }
> >
> > - rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "loading blob");
> > + rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, TPM_TRANSMIT_UNLOCKED,
> > + "loading blob");
>
> I still don't like this, required mutex's should not be split outside the
> function that needs them without more a more obvious indication:
>
> > + mutex_lock(&chip->tpm_mutex);
> > rc = tpm2_load(chip, payload, options, &blob_handle);
> > if (rc)
> > - return rc;
>
> I recommend you stick with the idiom and do this:
>
> mutex_lock(&chip->tpm_mutex);
> rc = tpm2_load(chip, payload, options, &blob_handle, TPM_TRANSMIT_UNLOCKED);
>
> Which makes it easy to see we are doing it right everywhere.
Why consume stack for unnecessary stuff? This is a static function. For
me this sounds like cutting hairs really.
One thing that would improve readability would be to rename internal
functions tpm2_load and tpm2_unseal to tpm2_load_cmd and tpm2_unseal_cmd
in order to underline that they are command wrappers and not to mix with
tpm2_unseal_trusted().
I've been thinking to move that kind of convetion at least with TPM2
specific stuff when a function is clearly a wrapper. It kind of
documents the call path. I.e. if function satifies a constraint that
it prepares a command blob, calls tpm_transmit_cmd once and then
returns results to the caller this would be the naming convention.
> Jason
/Jarkko
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] tpm: fix a race condition in tpm2_unseal_trusted()
@ 2016-08-29 15:25 ` Jarkko Sakkinen
0 siblings, 0 replies; 17+ messages in thread
From: Jarkko Sakkinen @ 2016-08-29 15:25 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Peter Huewe, linux-security-module, stable, Marcel Selhorst,
moderated list:TPM DEVICE DRIVER, open list
On Sun, Aug 28, 2016 at 12:51:49PM -0600, Jason Gunthorpe wrote:
> On Sun, Aug 28, 2016 at 08:36:52AM +0200, Jarkko Sakkinen wrote:
> >
> > @@ -576,7 +576,8 @@ static int tpm2_load(struct tpm_chip *chip,
> > goto out;
> > }
> >
> > - rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "loading blob");
> > + rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, TPM_TRANSMIT_UNLOCKED,
> > + "loading blob");
>
> I still don't like this, required mutex's should not be split outside the
> function that needs them without more a more obvious indication:
>
> > + mutex_lock(&chip->tpm_mutex);
> > rc = tpm2_load(chip, payload, options, &blob_handle);
> > if (rc)
> > - return rc;
>
> I recommend you stick with the idiom and do this:
>
> mutex_lock(&chip->tpm_mutex);
> rc = tpm2_load(chip, payload, options, &blob_handle, TPM_TRANSMIT_UNLOCKED);
>
> Which makes it easy to see we are doing it right everywhere.
Why consume stack for unnecessary stuff? This is a static function. For
me this sounds like cutting hairs really.
One thing that would improve readability would be to rename internal
functions tpm2_load and tpm2_unseal to tpm2_load_cmd and tpm2_unseal_cmd
in order to underline that they are command wrappers and not to mix with
tpm2_unseal_trusted().
I've been thinking to move that kind of convetion at least with TPM2
specific stuff when a function is clearly a wrapper. It kind of
documents the call path. I.e. if function satifies a constraint that
it prepares a command blob, calls tpm_transmit_cmd once and then
returns results to the caller this would be the naming convention.
> Jason
/Jarkko
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] tpm: fix a race condition in tpm2_unseal_trusted()
2016-08-29 15:25 ` Jarkko Sakkinen
(?)
@ 2016-08-29 19:21 ` Jason Gunthorpe
-1 siblings, 0 replies; 17+ messages in thread
From: Jason Gunthorpe @ 2016-08-29 19:21 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Peter Huewe, linux-security-module, stable, Marcel Selhorst,
moderated list:TPM DEVICE DRIVER, open list
On Mon, Aug 29, 2016 at 06:25:21PM +0300, Jarkko Sakkinen wrote:
> On Sun, Aug 28, 2016 at 12:51:49PM -0600, Jason Gunthorpe wrote:
> > On Sun, Aug 28, 2016 at 08:36:52AM +0200, Jarkko Sakkinen wrote:
> > >
> > > @@ -576,7 +576,8 @@ static int tpm2_load(struct tpm_chip *chip,
> > > goto out;
> > > }
> > >
> > > - rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "loading blob");
> > > + rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, TPM_TRANSMIT_UNLOCKED,
> > > + "loading blob");
> >
> > I still don't like this, required mutex's should not be split outside the
> > function that needs them without more a more obvious indication:
> >
> > > + mutex_lock(&chip->tpm_mutex);
> > > rc = tpm2_load(chip, payload, options, &blob_handle);
> > > if (rc)
> > > - return rc;
> >
> > I recommend you stick with the idiom and do this:
> >
> > mutex_lock(&chip->tpm_mutex);
> > rc = tpm2_load(chip, payload, options, &blob_handle, TPM_TRANSMIT_UNLOCKED);
> >
> > Which makes it easy to see we are doing it right everywhere.
>
> Why consume stack for unnecessary stuff? This is a static function. For
> me this sounds like cutting hairs really.
Well, tpm2_load looks like any other normal command that would grab
the mutex, so something has to be done to indicate to the reader it is
the unlocked version.
I wouldn't worry about the stack, the compiler will inline that away
anyhow.
> One thing that would improve readability would be to rename internal
> functions tpm2_load and tpm2_unseal to tpm2_load_cmd and tpm2_unseal_cmd
> in order to underline that they are command wrappers and not to mix with
> tpm2_unseal_trusted().
That seems reasonable as well, as long as all _cmd varients are unlocked.
Jason
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] tpm: fix a race condition in tpm2_unseal_trusted()
@ 2016-08-29 19:21 ` Jason Gunthorpe
0 siblings, 0 replies; 17+ messages in thread
From: Jason Gunthorpe @ 2016-08-29 19:21 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Peter Huewe, linux-security-module, stable, Marcel Selhorst,
moderated list:TPM DEVICE DRIVER, open list
On Mon, Aug 29, 2016 at 06:25:21PM +0300, Jarkko Sakkinen wrote:
> On Sun, Aug 28, 2016 at 12:51:49PM -0600, Jason Gunthorpe wrote:
> > On Sun, Aug 28, 2016 at 08:36:52AM +0200, Jarkko Sakkinen wrote:
> > >
> > > @@ -576,7 +576,8 @@ static int tpm2_load(struct tpm_chip *chip,
> > > goto out;
> > > }
> > >
> > > - rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "loading blob");
> > > + rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, TPM_TRANSMIT_UNLOCKED,
> > > + "loading blob");
> >
> > I still don't like this, required mutex's should not be split outside the
> > function that needs them without more a more obvious indication:
> >
> > > + mutex_lock(&chip->tpm_mutex);
> > > rc = tpm2_load(chip, payload, options, &blob_handle);
> > > if (rc)
> > > - return rc;
> >
> > I recommend you stick with the idiom and do this:
> >
> > mutex_lock(&chip->tpm_mutex);
> > rc = tpm2_load(chip, payload, options, &blob_handle, TPM_TRANSMIT_UNLOCKED);
> >
> > Which makes it easy to see we are doing it right everywhere.
>
> Why consume stack for unnecessary stuff? This is a static function. For
> me this sounds like cutting hairs really.
Well, tpm2_load looks like any other normal command that would grab
the mutex, so something has to be done to indicate to the reader it is
the unlocked version.
I wouldn't worry about the stack, the compiler will inline that away
anyhow.
> One thing that would improve readability would be to rename internal
> functions tpm2_load and tpm2_unseal to tpm2_load_cmd and tpm2_unseal_cmd
> in order to underline that they are command wrappers and not to mix with
> tpm2_unseal_trusted().
That seems reasonable as well, as long as all _cmd varients are unlocked.
Jason
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] tpm: fix a race condition in tpm2_unseal_trusted()
@ 2016-08-29 19:21 ` Jason Gunthorpe
0 siblings, 0 replies; 17+ messages in thread
From: Jason Gunthorpe @ 2016-08-29 19:21 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: open list, stable-u79uwXL29TY76Z2rM5mHXA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
moderated list:TPM DEVICE DRIVER
On Mon, Aug 29, 2016 at 06:25:21PM +0300, Jarkko Sakkinen wrote:
> On Sun, Aug 28, 2016 at 12:51:49PM -0600, Jason Gunthorpe wrote:
> > On Sun, Aug 28, 2016 at 08:36:52AM +0200, Jarkko Sakkinen wrote:
> > >
> > > @@ -576,7 +576,8 @@ static int tpm2_load(struct tpm_chip *chip,
> > > goto out;
> > > }
> > >
> > > - rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "loading blob");
> > > + rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, TPM_TRANSMIT_UNLOCKED,
> > > + "loading blob");
> >
> > I still don't like this, required mutex's should not be split outside the
> > function that needs them without more a more obvious indication:
> >
> > > + mutex_lock(&chip->tpm_mutex);
> > > rc = tpm2_load(chip, payload, options, &blob_handle);
> > > if (rc)
> > > - return rc;
> >
> > I recommend you stick with the idiom and do this:
> >
> > mutex_lock(&chip->tpm_mutex);
> > rc = tpm2_load(chip, payload, options, &blob_handle, TPM_TRANSMIT_UNLOCKED);
> >
> > Which makes it easy to see we are doing it right everywhere.
>
> Why consume stack for unnecessary stuff? This is a static function. For
> me this sounds like cutting hairs really.
Well, tpm2_load looks like any other normal command that would grab
the mutex, so something has to be done to indicate to the reader it is
the unlocked version.
I wouldn't worry about the stack, the compiler will inline that away
anyhow.
> One thing that would improve readability would be to rename internal
> functions tpm2_load and tpm2_unseal to tpm2_load_cmd and tpm2_unseal_cmd
> in order to underline that they are command wrappers and not to mix with
> tpm2_unseal_trusted().
That seems reasonable as well, as long as all _cmd varients are unlocked.
Jason
------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] tpm: fix a race condition in tpm2_unseal_trusted()
2016-08-29 19:21 ` Jason Gunthorpe
@ 2016-08-29 19:40 ` Jarkko Sakkinen
-1 siblings, 0 replies; 17+ messages in thread
From: Jarkko Sakkinen @ 2016-08-29 19:40 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Peter Huewe, linux-security-module, stable, Marcel Selhorst,
moderated list:TPM DEVICE DRIVER, open list
On Mon, Aug 29, 2016 at 01:21:10PM -0600, Jason Gunthorpe wrote:
> On Mon, Aug 29, 2016 at 06:25:21PM +0300, Jarkko Sakkinen wrote:
> > On Sun, Aug 28, 2016 at 12:51:49PM -0600, Jason Gunthorpe wrote:
> > > On Sun, Aug 28, 2016 at 08:36:52AM +0200, Jarkko Sakkinen wrote:
> > > >
> > > > @@ -576,7 +576,8 @@ static int tpm2_load(struct tpm_chip *chip,
> > > > goto out;
> > > > }
> > > >
> > > > - rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "loading blob");
> > > > + rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, TPM_TRANSMIT_UNLOCKED,
> > > > + "loading blob");
> > >
> > > I still don't like this, required mutex's should not be split outside the
> > > function that needs them without more a more obvious indication:
> > >
> > > > + mutex_lock(&chip->tpm_mutex);
> > > > rc = tpm2_load(chip, payload, options, &blob_handle);
> > > > if (rc)
> > > > - return rc;
> > >
> > > I recommend you stick with the idiom and do this:
> > >
> > > mutex_lock(&chip->tpm_mutex);
> > > rc = tpm2_load(chip, payload, options, &blob_handle, TPM_TRANSMIT_UNLOCKED);
> > >
> > > Which makes it easy to see we are doing it right everywhere.
> >
> > Why consume stack for unnecessary stuff? This is a static function. For
> > me this sounds like cutting hairs really.
>
> Well, tpm2_load looks like any other normal command that would grab
> the mutex, so something has to be done to indicate to the reader it is
> the unlocked version.
>
> I wouldn't worry about the stack, the compiler will inline that away
> anyhow.
>
> > One thing that would improve readability would be to rename internal
> > functions tpm2_load and tpm2_unseal to tpm2_load_cmd and tpm2_unseal_cmd
> > in order to underline that they are command wrappers and not to mix with
> > tpm2_unseal_trusted().
>
> That seems reasonable as well, as long as all _cmd varients are unlocked.
I think this more reasonable argument for your proposal than previous
ones for the flags parameter. Or maybe you had this argument in earlier
responses but I just failed to decipher it.
I can buy this.
> Jason
/Jarkko
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] tpm: fix a race condition in tpm2_unseal_trusted()
@ 2016-08-29 19:40 ` Jarkko Sakkinen
0 siblings, 0 replies; 17+ messages in thread
From: Jarkko Sakkinen @ 2016-08-29 19:40 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Peter Huewe, linux-security-module, stable, Marcel Selhorst,
moderated list:TPM DEVICE DRIVER, open list
On Mon, Aug 29, 2016 at 01:21:10PM -0600, Jason Gunthorpe wrote:
> On Mon, Aug 29, 2016 at 06:25:21PM +0300, Jarkko Sakkinen wrote:
> > On Sun, Aug 28, 2016 at 12:51:49PM -0600, Jason Gunthorpe wrote:
> > > On Sun, Aug 28, 2016 at 08:36:52AM +0200, Jarkko Sakkinen wrote:
> > > >
> > > > @@ -576,7 +576,8 @@ static int tpm2_load(struct tpm_chip *chip,
> > > > goto out;
> > > > }
> > > >
> > > > - rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "loading blob");
> > > > + rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, TPM_TRANSMIT_UNLOCKED,
> > > > + "loading blob");
> > >
> > > I still don't like this, required mutex's should not be split outside the
> > > function that needs them without more a more obvious indication:
> > >
> > > > + mutex_lock(&chip->tpm_mutex);
> > > > rc = tpm2_load(chip, payload, options, &blob_handle);
> > > > if (rc)
> > > > - return rc;
> > >
> > > I recommend you stick with the idiom and do this:
> > >
> > > mutex_lock(&chip->tpm_mutex);
> > > rc = tpm2_load(chip, payload, options, &blob_handle, TPM_TRANSMIT_UNLOCKED);
> > >
> > > Which makes it easy to see we are doing it right everywhere.
> >
> > Why consume stack for unnecessary stuff? This is a static function. For
> > me this sounds like cutting hairs really.
>
> Well, tpm2_load looks like any other normal command that would grab
> the mutex, so something has to be done to indicate to the reader it is
> the unlocked version.
>
> I wouldn't worry about the stack, the compiler will inline that away
> anyhow.
>
> > One thing that would improve readability would be to rename internal
> > functions tpm2_load and tpm2_unseal to tpm2_load_cmd and tpm2_unseal_cmd
> > in order to underline that they are command wrappers and not to mix with
> > tpm2_unseal_trusted().
>
> That seems reasonable as well, as long as all _cmd varients are unlocked.
I think this more reasonable argument for your proposal than previous
ones for the flags parameter. Or maybe you had this argument in earlier
responses but I just failed to decipher it.
I can buy this.
> Jason
/Jarkko
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] tpm: fix a race condition in tpm2_unseal_trusted()
2016-08-29 19:40 ` Jarkko Sakkinen
@ 2016-08-29 20:03 ` Jarkko Sakkinen
-1 siblings, 0 replies; 17+ messages in thread
From: Jarkko Sakkinen @ 2016-08-29 20:03 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Peter Huewe, linux-security-module, stable, Marcel Selhorst,
moderated list:TPM DEVICE DRIVER, open list
On Mon, Aug 29, 2016 at 10:40:27PM +0300, Jarkko Sakkinen wrote:
> On Mon, Aug 29, 2016 at 01:21:10PM -0600, Jason Gunthorpe wrote:
> > On Mon, Aug 29, 2016 at 06:25:21PM +0300, Jarkko Sakkinen wrote:
> > > On Sun, Aug 28, 2016 at 12:51:49PM -0600, Jason Gunthorpe wrote:
> > > > On Sun, Aug 28, 2016 at 08:36:52AM +0200, Jarkko Sakkinen wrote:
> > > > >
> > > > > @@ -576,7 +576,8 @@ static int tpm2_load(struct tpm_chip *chip,
> > > > > goto out;
> > > > > }
> > > > >
> > > > > - rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "loading blob");
> > > > > + rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, TPM_TRANSMIT_UNLOCKED,
> > > > > + "loading blob");
> > > >
> > > > I still don't like this, required mutex's should not be split outside the
> > > > function that needs them without more a more obvious indication:
> > > >
> > > > > + mutex_lock(&chip->tpm_mutex);
> > > > > rc = tpm2_load(chip, payload, options, &blob_handle);
> > > > > if (rc)
> > > > > - return rc;
> > > >
> > > > I recommend you stick with the idiom and do this:
> > > >
> > > > mutex_lock(&chip->tpm_mutex);
> > > > rc = tpm2_load(chip, payload, options, &blob_handle, TPM_TRANSMIT_UNLOCKED);
> > > >
> > > > Which makes it easy to see we are doing it right everywhere.
> > >
> > > Why consume stack for unnecessary stuff? This is a static function. For
> > > me this sounds like cutting hairs really.
> >
> > Well, tpm2_load looks like any other normal command that would grab
> > the mutex, so something has to be done to indicate to the reader it is
> > the unlocked version.
> >
> > I wouldn't worry about the stack, the compiler will inline that away
> > anyhow.
> >
> > > One thing that would improve readability would be to rename internal
> > > functions tpm2_load and tpm2_unseal to tpm2_load_cmd and tpm2_unseal_cmd
> > > in order to underline that they are command wrappers and not to mix with
> > > tpm2_unseal_trusted().
> >
> > That seems reasonable as well, as long as all _cmd varients are unlocked.
>
> I think this more reasonable argument for your proposal than previous
> ones for the flags parameter. Or maybe you had this argument in earlier
> responses but I just failed to decipher it.
>
> I can buy this.
I fix the comments as well because they have some errors concerning
return value and also do not match guidelines of [1] now that I'm at it.
I will also add documentation comments for tpm2_unseal_cmd,
tpm2_load_cmd and tpm2_flush_context_cmd. Do you think it is sufficient
to say "Return: same as with tpm_transmit_cmd"?
[1] https://www.kernel.org/doc/Documentation/kernel-doc-nano-HOWTO.txt
/Jarkko
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] tpm: fix a race condition in tpm2_unseal_trusted()
@ 2016-08-29 20:03 ` Jarkko Sakkinen
0 siblings, 0 replies; 17+ messages in thread
From: Jarkko Sakkinen @ 2016-08-29 20:03 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Peter Huewe, linux-security-module, stable, Marcel Selhorst,
moderated list:TPM DEVICE DRIVER, open list
On Mon, Aug 29, 2016 at 10:40:27PM +0300, Jarkko Sakkinen wrote:
> On Mon, Aug 29, 2016 at 01:21:10PM -0600, Jason Gunthorpe wrote:
> > On Mon, Aug 29, 2016 at 06:25:21PM +0300, Jarkko Sakkinen wrote:
> > > On Sun, Aug 28, 2016 at 12:51:49PM -0600, Jason Gunthorpe wrote:
> > > > On Sun, Aug 28, 2016 at 08:36:52AM +0200, Jarkko Sakkinen wrote:
> > > > >
> > > > > @@ -576,7 +576,8 @@ static int tpm2_load(struct tpm_chip *chip,
> > > > > goto out;
> > > > > }
> > > > >
> > > > > - rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "loading blob");
> > > > > + rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, TPM_TRANSMIT_UNLOCKED,
> > > > > + "loading blob");
> > > >
> > > > I still don't like this, required mutex's should not be split outside the
> > > > function that needs them without more a more obvious indication:
> > > >
> > > > > + mutex_lock(&chip->tpm_mutex);
> > > > > rc = tpm2_load(chip, payload, options, &blob_handle);
> > > > > if (rc)
> > > > > - return rc;
> > > >
> > > > I recommend you stick with the idiom and do this:
> > > >
> > > > mutex_lock(&chip->tpm_mutex);
> > > > rc = tpm2_load(chip, payload, options, &blob_handle, TPM_TRANSMIT_UNLOCKED);
> > > >
> > > > Which makes it easy to see we are doing it right everywhere.
> > >
> > > Why consume stack for unnecessary stuff? This is a static function. For
> > > me this sounds like cutting hairs really.
> >
> > Well, tpm2_load looks like any other normal command that would grab
> > the mutex, so something has to be done to indicate to the reader it is
> > the unlocked version.
> >
> > I wouldn't worry about the stack, the compiler will inline that away
> > anyhow.
> >
> > > One thing that would improve readability would be to rename internal
> > > functions tpm2_load and tpm2_unseal to tpm2_load_cmd and tpm2_unseal_cmd
> > > in order to underline that they are command wrappers and not to mix with
> > > tpm2_unseal_trusted().
> >
> > That seems reasonable as well, as long as all _cmd varients are unlocked.
>
> I think this more reasonable argument for your proposal than previous
> ones for the flags parameter. Or maybe you had this argument in earlier
> responses but I just failed to decipher it.
>
> I can buy this.
I fix the comments as well because they have some errors concerning
return value and also do not match guidelines of [1] now that I'm at it.
I will also add documentation comments for tpm2_unseal_cmd,
tpm2_load_cmd and tpm2_flush_context_cmd. Do you think it is sufficient
to say "Return: same as with tpm_transmit_cmd"?
[1] https://www.kernel.org/doc/Documentation/kernel-doc-nano-HOWTO.txt
/Jarkko
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] tpm: fix a race condition in tpm2_unseal_trusted()
2016-08-29 20:03 ` Jarkko Sakkinen
(?)
@ 2016-08-29 20:07 ` Jason Gunthorpe
-1 siblings, 0 replies; 17+ messages in thread
From: Jason Gunthorpe @ 2016-08-29 20:07 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Peter Huewe, linux-security-module, stable, Marcel Selhorst,
moderated list:TPM DEVICE DRIVER, open list
On Mon, Aug 29, 2016 at 11:03:26PM +0300, Jarkko Sakkinen wrote:
> I will also add documentation comments for tpm2_unseal_cmd,
> tpm2_load_cmd and tpm2_flush_context_cmd. Do you think it is sufficient
> to say "Return: same as with tpm_transmit_cmd"?
yes
Jason
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] tpm: fix a race condition in tpm2_unseal_trusted()
@ 2016-08-29 20:07 ` Jason Gunthorpe
0 siblings, 0 replies; 17+ messages in thread
From: Jason Gunthorpe @ 2016-08-29 20:07 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Peter Huewe, linux-security-module, stable, Marcel Selhorst,
moderated list:TPM DEVICE DRIVER, open list
On Mon, Aug 29, 2016 at 11:03:26PM +0300, Jarkko Sakkinen wrote:
> I will also add documentation comments for tpm2_unseal_cmd,
> tpm2_load_cmd and tpm2_flush_context_cmd. Do you think it is sufficient
> to say "Return: same as with tpm_transmit_cmd"?
yes
Jason
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] tpm: fix a race condition in tpm2_unseal_trusted()
@ 2016-08-29 20:07 ` Jason Gunthorpe
0 siblings, 0 replies; 17+ messages in thread
From: Jason Gunthorpe @ 2016-08-29 20:07 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: open list, stable-u79uwXL29TY76Z2rM5mHXA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
moderated list:TPM DEVICE DRIVER
On Mon, Aug 29, 2016 at 11:03:26PM +0300, Jarkko Sakkinen wrote:
> I will also add documentation comments for tpm2_unseal_cmd,
> tpm2_load_cmd and tpm2_flush_context_cmd. Do you think it is sufficient
> to say "Return: same as with tpm_transmit_cmd"?
yes
Jason
------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2016-08-29 20:07 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-28 6:36 [PATCH] tpm: fix a race condition in tpm2_unseal_trusted() Jarkko Sakkinen
2016-08-28 6:36 ` Jarkko Sakkinen
2016-08-28 18:51 ` Jason Gunthorpe
2016-08-28 18:51 ` Jason Gunthorpe
2016-08-28 18:51 ` Jason Gunthorpe
2016-08-29 15:25 ` Jarkko Sakkinen
2016-08-29 15:25 ` Jarkko Sakkinen
2016-08-29 19:21 ` Jason Gunthorpe
2016-08-29 19:21 ` Jason Gunthorpe
2016-08-29 19:21 ` Jason Gunthorpe
2016-08-29 19:40 ` Jarkko Sakkinen
2016-08-29 19:40 ` Jarkko Sakkinen
2016-08-29 20:03 ` Jarkko Sakkinen
2016-08-29 20:03 ` Jarkko Sakkinen
2016-08-29 20:07 ` Jason Gunthorpe
2016-08-29 20:07 ` Jason Gunthorpe
2016-08-29 20:07 ` Jason Gunthorpe
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.