[PATCH 4.4 20/21] tpm: fix a race condition in tpm2_unseal_trusted()

From: Greg Kroah-Hartman
Date: Fri Oct 14 2016 - 07:44:05 EST


4.4-stable review patch. If anyone has any objections, please let me know.

------------------

From: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxxxxxxxx>

commit d4816edfe706497a8525480c1685ceb9871bc118 upstream.

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.

Fixes: 0fe5480303a1 ("keys, trusted: seal/unseal with TPM 2.0 chips")
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxxxxxxxx>
Reviewed-by: Jason Gunthorpe <jgunthorpe@xxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>

---
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 | 101 +++++++++++++++++++++++++--------------
5 files changed, 103 insertions(+), 65 deletions(-)

--- a/drivers/char/tpm/tpm-dev.c
+++ b/drivers/char/tpm/tpm-dev.c
@@ -139,7 +139,7 @@ static ssize_t tpm_write(struct file *fi

/* atomic tpm command send and result receive */
out_size = tpm_transmit(priv->chip, priv->data_buffer,
- sizeof(priv->data_buffer));
+ sizeof(priv->data_buffer), 0);
if (out_size < 0) {
mutex_unlock(&priv->buffer_mutex);
return out_size;
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -328,8 +328,8 @@ EXPORT_SYMBOL_GPL(tpm_calc_ordinal_durat
/*
* 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;
@@ -348,7 +348,8 @@ ssize_t tpm_transmit(struct tpm_chip *ch
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) {
@@ -391,20 +392,21 @@ out_recv:
dev_err(chip->pdev,
"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)
@@ -452,7 +454,8 @@ ssize_t tpm_getcap(struct device *dev, _
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;
@@ -468,7 +471,7 @@ void tpm_gen_interrupt(struct tpm_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);
@@ -489,7 +492,7 @@ static int tpm_startup(struct tpm_chip *
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");
}

@@ -505,7 +508,8 @@ int tpm_get_timeouts(struct tpm_chip *ch
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.
@@ -519,7 +523,7 @@ int tpm_get_timeouts(struct tpm_chip *ch
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->pdev,
@@ -580,7 +584,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;
@@ -636,7 +640,7 @@ static int tpm_continue_selftest(struct
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;
}
@@ -656,7 +660,7 @@ int tpm_pcr_read_dev(struct tpm_chip *ch

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)
@@ -754,7 +758,7 @@ int tpm_pcr_extend(u32 chip_num, int pcr
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_chip_put(chip);
@@ -793,7 +797,7 @@ int tpm_do_selftest(struct tpm_chip *chi
/* 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. */
@@ -834,7 +838,7 @@ int tpm_send(u32 chip_num, void *cmd, si
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_chip_put(chip);
return rc;
@@ -936,14 +940,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
@@ -1027,8 +1032,8 @@ int tpm_get_random(u32 chip_num, u8 *out
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;

--- 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
struct tpm_chip *chip = dev_get_drvdata(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;
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -498,11 +498,15 @@ extern struct class *tpm_class;
extern dev_t tpm_devt;
extern const struct file_operations tpm_fops;

+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 device *, __be32, cap_t *, const char *);
-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 *);
extern int tpm_do_selftest(struct tpm_chip *);
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -264,7 +264,7 @@ int tpm2_pcr_read(struct tpm_chip *chip,
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;
@@ -312,7 +312,7 @@ int tpm2_pcr_extend(struct tpm_chip *chi
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;
@@ -358,7 +358,7 @@ int tpm2_get_random(struct tpm_chip *chi
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;
@@ -416,12 +416,12 @@ static void tpm2_buf_append_auth(struct
}

/**
- * tpm2_seal_trusted() - seal a trusted key
- * @chip_num: A specific chip number for the request or TPM_ANY_NUM
- * @options: authentication values and other options
+ * tpm2_seal_trusted() - seal the payload of a trusted key
+ * @chip_num: TPM chip to use
* @payload: the key data in clear and encrypted form
+ * @options: authentication values and other options
*
- * Returns < 0 on error and 0 on success.
+ * Return: < 0 on error and 0 on success.
*/
int tpm2_seal_trusted(struct tpm_chip *chip,
struct trusted_key_payload *payload,
@@ -472,7 +472,7 @@ int tpm2_seal_trusted(struct tpm_chip *c
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;

@@ -494,10 +494,18 @@ out:
return rc;
}

-static int tpm2_load(struct tpm_chip *chip,
- struct trusted_key_payload *payload,
- struct trusted_key_options *options,
- u32 *blob_handle)
+/**
+ * tpm2_load_cmd() - execute a TPM2_Load command
+ * @chip_num: TPM chip to use
+ * @payload: the key data in clear and encrypted form
+ * @options: authentication values and other options
+ *
+ * Return: same as with tpm_transmit_cmd
+ */
+static int tpm2_load_cmd(struct tpm_chip *chip,
+ struct trusted_key_payload *payload,
+ struct trusted_key_options *options,
+ u32 *blob_handle, unsigned int flags)
{
struct tpm_buf buf;
unsigned int private_len;
@@ -532,7 +540,7 @@ static int tpm2_load(struct tpm_chip *ch
goto out;
}

- rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "loading blob");
+ rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, flags, "loading blob");
if (!rc)
*blob_handle = be32_to_cpup(
(__be32 *) &buf.data[TPM_HEADER_SIZE]);
@@ -546,7 +554,16 @@ out:
return rc;
}

-static void tpm2_flush_context(struct tpm_chip *chip, u32 handle)
+/**
+ * tpm2_flush_context_cmd() - execute a TPM2_FlushContext command
+ * @chip_num: TPM chip to use
+ * @payload: the key data in clear and encrypted form
+ * @options: authentication values and other options
+ *
+ * Return: same as with tpm_transmit_cmd
+ */
+static void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
+ unsigned int flags)
{
struct tpm_buf buf;
int rc;
@@ -560,7 +577,8 @@ static void tpm2_flush_context(struct tp

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, flags,
+ "flushing context");
if (rc)
dev_warn(chip->pdev, "0x%08x was not flushed, rc=%d\n", handle,
rc);
@@ -568,10 +586,18 @@ static void tpm2_flush_context(struct tp
tpm_buf_destroy(&buf);
}

-static int tpm2_unseal(struct tpm_chip *chip,
- struct trusted_key_payload *payload,
- struct trusted_key_options *options,
- u32 blob_handle)
+/**
+ * tpm2_unseal_cmd() - execute a TPM2_Unload command
+ * @chip_num: TPM chip to use
+ * @payload: the key data in clear and encrypted form
+ * @options: authentication values and other options
+ *
+ * Return: same as with tpm_transmit_cmd
+ */
+static int tpm2_unseal_cmd(struct tpm_chip *chip,
+ struct trusted_key_payload *payload,
+ struct trusted_key_options *options,
+ u32 blob_handle, unsigned int flags)
{
struct tpm_buf buf;
u16 data_len;
@@ -589,7 +615,7 @@ static int tpm2_unseal(struct tpm_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, flags, "unsealing");
if (rc > 0)
rc = -EPERM;

@@ -608,12 +634,12 @@ static int tpm2_unseal(struct tpm_chip *
}

/**
- * tpm_unseal_trusted() - unseal a trusted key
- * @chip_num: A specific chip number for the request or TPM_ANY_NUM
- * @options: authentication values and other options
+ * tpm_unseal_trusted() - unseal the payload of a trusted key
+ * @chip_num: TPM chip to use
* @payload: the key data in clear and encrypted form
+ * @options: authentication values and other options
*
- * Returns < 0 on error and 0 on success.
+ * Return: < 0 on error and 0 on success.
*/
int tpm2_unseal_trusted(struct tpm_chip *chip,
struct trusted_key_payload *payload,
@@ -622,14 +648,17 @@ int tpm2_unseal_trusted(struct tpm_chip
u32 blob_handle;
int rc;

- rc = tpm2_load(chip, payload, options, &blob_handle);
+ mutex_lock(&chip->tpm_mutex);
+ rc = tpm2_load_cmd(chip, payload, options, &blob_handle,
+ TPM_TRANSMIT_UNLOCKED);
if (rc)
- return rc;
-
- rc = tpm2_unseal(chip, payload, options, blob_handle);
-
- tpm2_flush_context(chip, blob_handle);
+ goto out;

+ rc = tpm2_unseal_cmd(chip, payload, options, blob_handle,
+ TPM_TRANSMIT_UNLOCKED);
+ tpm2_flush_context_cmd(chip, blob_handle, TPM_TRANSMIT_UNLOCKED);
+out:
+ mutex_unlock(&chip->tpm_mutex);
return rc;
}

@@ -655,7 +684,7 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip
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);

@@ -689,7 +718,7 @@ int tpm2_startup(struct tpm_chip *chip,
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");
}
EXPORT_SYMBOL_GPL(tpm2_startup);
@@ -718,7 +747,7 @@ void tpm2_shutdown(struct tpm_chip *chip
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.
@@ -784,7 +813,7 @@ static int tpm2_start_selftest(struct tp
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
@@ -836,7 +865,7 @@ int tpm2_do_selftest(struct tpm_chip *ch
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;

@@ -885,7 +914,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)