[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <81a638ff-3eab-47da-90ff-a758aa7b7b1c@linux.vnet.ibm.com>
Date: Mon, 16 Jan 2017 09:46:21 -0500
From: Stefan Berger <stefanb@...ux.vnet.ibm.com>
To: Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
Cc: tpmdd-devel@...ts.sourceforge.net,
linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5] tpm: Check size of response before accessing data
On 01/16/2017 08:24 AM, Jarkko Sakkinen wrote:
> On Fri, Jan 13, 2017 at 09:36:34PM -0500, Stefan Berger wrote:
>> Make sure that we have not received less bytes than what is indicated
>> in the header of the TPM response. Also, check the number of bytes in
>> the response before accessing its data.
>>
>> Signed-off-by: Stefan Berger <stefanb@...ux.vnet.ibm.com>
> There are some things that I want to comment after all but I can give
> now
>
> Tested-by: Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
>
> I also noticed that this patch is not CC'd to linux-kernel.
>
>> ---
>>
>> v5:
>> - Fixed a bug related to tpm_getcap() having to allow to return
>> only a header rather than having to receive a full response
>> ---
>> drivers/char/tpm/tpm-interface.c | 64 ++++++++++++++++++++++++---------
>> drivers/char/tpm/tpm-sysfs.c | 29 +++++++++------
>> drivers/char/tpm/tpm.h | 7 ++--
>> drivers/char/tpm/tpm2-cmd.c | 76 +++++++++++++++++++++++++++++-----------
>> drivers/char/tpm/tpm_tis_core.c | 3 +-
>> 5 files changed, 128 insertions(+), 51 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
>> index fecdd3f..4da6ef8 100644
>> --- a/drivers/char/tpm/tpm-interface.c
>> +++ b/drivers/char/tpm/tpm-interface.c
>> @@ -434,7 +434,8 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
>> * A positive number for a TPM error.
>> */
>> ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void *cmd,
>> - int len, unsigned int flags, const char *desc)
>> + size_t len, size_t min_rx_length,
> You haven't documented min_rx_length.
>
>> + unsigned int flags, const char *desc)
>> {
>> const struct tpm_output_header *header;
>> int err;
>> @@ -446,6 +447,9 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void *cmd,
>> return -EFAULT;
>>
>> header = cmd;
>> + if (len < be32_to_cpu(header->length) ||
>> + be32_to_cpu(header->length) < min_rx_length)
>> + return -EFAULT;
>>
>> err = be32_to_cpu(header->return_code);
>> if (err != 0 && desc)
>> @@ -468,7 +472,8 @@ static const struct tpm_input_header tpm_getcap_header = {
>> };
>>
>> ssize_t tpm_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
>> - const char *desc)
>> + const char *desc, size_t min_cap_length,
>> + u32 *rlength)
>> {
>> struct tpm_cmd_t tpm_cmd;
>> int rc;
>> @@ -491,10 +496,12 @@ ssize_t tpm_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
>> tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
>> tpm_cmd.params.getcap_in.subcap = cpu_to_be32(subcap_id);
>> }
>> - rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, 0,
>> - desc);
>> + rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
>> + TPM_HEADER_SIZE + min_cap_length, 0, desc);
>> if (!rc)
>> *cap = tpm_cmd.params.getcap_out.cap;
>> + if (rlength)
>> + *rlength = be32_to_cpu(tpm_cmd.header.out.length);
>> return rc;
>> }
>> EXPORT_SYMBOL_GPL(tpm_getcap);
>> @@ -515,7 +522,8 @@ 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, 0,
>> + return tpm_transmit_cmd(chip, &start_cmd, TPM_INTERNAL_RESULT_SIZE,
>> + TPM_HEADER_SIZE, 0,
>> "attempting to start the TPM");
>> }
>>
>> @@ -525,6 +533,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
>> unsigned long new_timeout[4];
>> unsigned long old_timeout[4];
>> ssize_t rc;
>> + u32 rlength;
>>
>> if (chip->flags & TPM_CHIP_FLAG_HAVE_TIMEOUTS)
>> return 0;
>> @@ -546,7 +555,8 @@ int tpm_get_timeouts(struct tpm_chip *chip)
>> return 0;
>> }
>>
>> - rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, NULL);
>> + rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, NULL,
>> + 0, &rlength);
>> if (rc == TPM_ERR_INVALID_POSTINIT) {
>> /* The TPM is not started, we are the first to talk to it.
>> Execute a startup command. */
>> @@ -555,8 +565,12 @@ int tpm_get_timeouts(struct tpm_chip *chip)
>> return rc;
>>
>> rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap,
>> - "attempting to determine the timeouts");
>> + "attempting to determine the timeouts",
>> + 0, &rlength);
>> }
>> + if (rlength < TPM_HEADER_SIZE + sizeof(cap.timeout))
>> + rc = -EFAULT;
>> +
>> if (rc) {
>> dev_err(&chip->dev,
>> "A TPM error (%zd) occurred attempting to determine the timeouts\n",
>> @@ -606,7 +620,8 @@ int tpm_get_timeouts(struct tpm_chip *chip)
>> chip->timeout_d = usecs_to_jiffies(new_timeout[3]);
>>
>> rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_DURATION, &cap,
>> - "attempting to determine the durations");
>> + "attempting to determine the durations",
>> + sizeof(cap.duration), NULL);
>> if (rc)
>> return rc;
>>
>> @@ -657,8 +672,9 @@ 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, 0,
>> - "continue selftest");
>> + rc = tpm_transmit_cmd(chip, &cmd, CONTINUE_SELFTEST_RESULT_SIZE,
>> + CONTINUE_SELFTEST_RESULT_SIZE,
>> + 0, "continue selftest");
>> return rc;
>> }
>>
>> @@ -677,7 +693,9 @@ 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, 0,
>> + rc = tpm_transmit_cmd(chip, &cmd,
>> + READ_PCR_RESULT_SIZE,
>> + READ_PCR_RESULT_SIZE, 0,
>> "attempting to read a pcr value");
>>
>> if (rc == 0)
>> @@ -775,7 +793,8 @@ 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, 0,
>> + rc = tpm_transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE,
>> + EXTEND_PCR_RESULT_SIZE, 0,
>> "attempting extend a PCR value");
>>
>> tpm_put_ops(chip);
>> @@ -879,7 +898,8 @@ int tpm_send(u32 chip_num, void *cmd, size_t buflen)
>> if (chip == NULL)
>> return -ENODEV;
>>
>> - rc = tpm_transmit_cmd(chip, cmd, buflen, 0, "attempting tpm_cmd");
>> + rc = tpm_transmit_cmd(chip, cmd, buflen, TPM_HEADER_SIZE,
>> + 0, "attempting tpm_cmd");
>>
>> tpm_put_ops(chip);
>> return rc;
>> @@ -981,15 +1001,16 @@ 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, 0,
>> + rc = tpm_transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE,
>> + 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, 0,
>> - NULL);
>> + rc = tpm_transmit_cmd(chip, &cmd, SAVESTATE_RESULT_SIZE,
>> + SAVESTATE_RESULT_SIZE, 0, NULL);
>>
>> /*
>> * If the TPM indicates that it is too busy to respond to
>> @@ -1051,7 +1072,7 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
>> {
>> struct tpm_chip *chip;
>> struct tpm_cmd_t tpm_cmd;
>> - u32 recd, num_bytes = min_t(u32, max, TPM_MAX_RNG_DATA);
>> + u32 recd, num_bytes = min_t(u32, max, TPM_MAX_RNG_DATA), rlength;
>> int err, total = 0, retries = 5;
>> u8 *dest = out;
>>
>> @@ -1074,11 +1095,20 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
>>
>> err = tpm_transmit_cmd(chip, &tpm_cmd,
>> TPM_GETRANDOM_RESULT_SIZE + num_bytes,
>> + offsetof(struct tpm_cmd_t,
>> + params.getrandom_out.rng_data),
>> 0, "attempting get random");
>> if (err)
>> break;
>>
>> recd = be32_to_cpu(tpm_cmd.params.getrandom_out.rng_data_len);
>> +
>> + rlength = be32_to_cpu(tpm_cmd.header.out.length);
>> + if (rlength < offsetof(struct tpm_cmd_t,
>> + params.getrandom_out.rng_data) + recd) {
>> + total = -EFAULT;
>> + break;
>> + }
>> memcpy(dest, tpm_cmd.params.getrandom_out.rng_data, recd);
>>
>> dest += recd;
>> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
>> index 848ad65..5e8100c 100644
>> --- a/drivers/char/tpm/tpm-sysfs.c
>> +++ b/drivers/char/tpm/tpm-sysfs.c
>> @@ -39,8 +39,9 @@ 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, 0,
>> - "attempting to read the PUBEK");
>> + err = tpm_transmit_cmd(chip, &tpm_cmd, READ_PUBEK_RESULT_SIZE,
>> + TPM_HEADER_SIZE + 28 + 256,
>> + 0, "attempting to read the PUBEK");
> There are bunch of these but this is worse than having just
> a named constant containing a number. That arithmetic looks
> horrible.
76 for (i = 0; i < 256; i++) {
77 str += sprintf(str, "%02X ", data[i + 28]);
78 if ((i + 1) % 16 == 0)
79 str += sprintf(str, "\n");
80 }
Look at the following. It ends up with 28 + 256 matching exactly the
stuff above.
>
> There are many of these. BTW, what is difference between the
> value stored in READ_PUBEK_RESULT_SIZE and the value you are
> calculating?
>
>
>> if (err)
>> goto out;
>>
>> @@ -95,7 +96,8 @@ static ssize_t pcrs_show(struct device *dev, struct device_attribute *attr,
>> struct tpm_chip *chip = to_tpm_chip(dev);
>>
>> rc = tpm_getcap(chip, TPM_CAP_PROP_PCR, &cap,
>> - "attempting to determine the number of PCRS");
>> + "attempting to determine the number of PCRS",
>> + sizeof(cap.num_pcrs), NULL);
>> if (rc)
>> return 0;
>>
>> @@ -120,7 +122,8 @@ static ssize_t enabled_show(struct device *dev, struct device_attribute *attr,
>> ssize_t rc;
>>
>> rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap,
>> - "attempting to determine the permanent enabled state");
>> + "attempting to determine the permanent enabled state",
>> + sizeof(cap.perm_flags), NULL);
>> if (rc)
>> return 0;
>>
>> @@ -136,7 +139,8 @@ static ssize_t active_show(struct device *dev, struct device_attribute *attr,
>> ssize_t rc;
>>
>> rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap,
>> - "attempting to determine the permanent active state");
>> + "attempting to determine the permanent active state",
>> + sizeof(cap.perm_flags), NULL);
>> if (rc)
>> return 0;
>>
>> @@ -152,7 +156,8 @@ static ssize_t owned_show(struct device *dev, struct device_attribute *attr,
>> ssize_t rc;
>>
>> rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_PROP_OWNER, &cap,
>> - "attempting to determine the owner state");
>> + "attempting to determine the owner state",
>> + sizeof(cap.owned), NULL);
>> if (rc)
>> return 0;
>>
>> @@ -168,7 +173,8 @@ static ssize_t temp_deactivated_show(struct device *dev,
>> ssize_t rc;
>>
>> rc = tpm_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_VOL, &cap,
>> - "attempting to determine the temporary state");
>> + "attempting to determine the temporary state",
>> + sizeof(cap.stclear_flags), NULL);
>> if (rc)
>> return 0;
>>
>> @@ -186,7 +192,8 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
>> char *str = buf;
>>
>> rc = tpm_getcap(chip, TPM_CAP_PROP_MANUFACTURER, &cap,
>> - "attempting to determine the manufacturer");
>> + "attempting to determine the manufacturer",
>> + sizeof(cap.manufacturer_id), NULL);
>> if (rc)
>> return 0;
>> str += sprintf(str, "Manufacturer: 0x%x\n",
>> @@ -194,7 +201,8 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
>>
>> /* Try to get a TPM version 1.2 TPM_CAP_VERSION_INFO */
>> rc = tpm_getcap(chip, TPM_CAP_VERSION_1_2, &cap,
>> - "attempting to determine the 1.2 version");
>> + "attempting to determine the 1.2 version",
>> + sizeof(cap.tpm_version_1_2), NULL);
>> if (!rc) {
>> str += sprintf(str,
>> "TCG version: %d.%d\nFirmware version: %d.%d\n",
>> @@ -205,7 +213,8 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
>> } else {
>> /* Otherwise just use TPM_STRUCT_VER */
>> rc = tpm_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
>> - "attempting to determine the 1.1 version");
>> + "attempting to determine the 1.1 version",
>> + sizeof(cap.tpm_version), NULL);
>> if (rc)
>> return 0;
>> str += sprintf(str,
>> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
>> index 1ae9768..b7b12498 100644
>> --- a/drivers/char/tpm/tpm.h
>> +++ b/drivers/char/tpm/tpm.h
>> @@ -493,10 +493,11 @@ enum tpm_transmit_flags {
>>
>> 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_transmit_cmd(struct tpm_chip *chip, const void *cmd, size_t len,
>> + size_t min_tx_length, unsigned int flags,
>> + const char *desc);
>> ssize_t tpm_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
>> - const char *desc);
>> + const char *desc, size_t min_cap_length, u32 *rlength);
>> int tpm_get_timeouts(struct tpm_chip *);
>> int tpm1_auto_startup(struct tpm_chip *chip);
>> int tpm_do_selftest(struct tpm_chip *chip);
>> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
>> index 6eda239..9e8256b 100644
>> --- a/drivers/char/tpm/tpm2-cmd.c
>> +++ b/drivers/char/tpm/tpm2-cmd.c
>> @@ -248,6 +248,10 @@ static const u8 tpm2_ordinal_duration[TPM2_CC_LAST - TPM2_CC_FIRST + 1] = {
>> (sizeof(struct tpm_input_header) + \
>> sizeof(struct tpm2_pcr_read_in))
>>
>> +#define TPM2_PCR_READ_OUT_SIZE \
>> + (sizeof(struct tpm_output_header) + \
>> + sizeof(struct tpm2_pcr_read_out))
> Another example where this commit goes wrong. Would be better
> just to have
>
> #define TPM2_PCR_READ_OUT_SIZE <number>. We anyway want to
> drop those special structures eventually (eg tpm2_pcr_read_out)
> so lets not bind more stuff to them.
Look at the code right above it. It follows a pattern in this case.
>
>> +
>> static const struct tpm_input_header tpm2_pcrread_header = {
>> .tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
>> .length = cpu_to_be32(TPM2_PCR_READ_IN_SIZE),
>> @@ -280,8 +284,8 @@ 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), 0,
>> - "attempting to read a pcr value");
>> + rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), TPM2_PCR_READ_OUT_SIZE,
>> + 0, "attempting to read a pcr value");
>> if (rc == 0) {
>> buf = cmd.params.pcrread_out.digest;
>> memcpy(res_buf, buf, TPM_DIGEST_SIZE);
>> @@ -327,7 +331,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), 0,
>> + rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), TPM_HEADER_SIZE, 0,
>> "attempting extend a PCR value");
>>
>> return rc;
>> @@ -356,7 +360,7 @@ static const struct tpm_input_header tpm2_getrandom_header = {
>> int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
>> {
>> struct tpm2_cmd cmd;
>> - u32 recd;
>> + u32 recd, rlength;
>> u32 num_bytes;
>> int err;
>> int total = 0;
>> @@ -373,13 +377,19 @@ 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), 0,
>> - "attempting get random");
>> + err = tpm_transmit_cmd(chip, &cmd, sizeof(cmd),
>> + offsetof(struct tpm2_cmd,
>> + params.getrandom_out.buffer),
>> + 0, "attempting get random");
>> if (err)
>> break;
>>
>> recd = min_t(u32, be16_to_cpu(cmd.params.getrandom_out.size),
>> num_bytes);
>> + rlength = be32_to_cpu(cmd.header.out.length);
>> + if (rlength < offsetof(struct tpm2_cmd,
>> + params.getrandom_out.buffer) + recd)
>> + return -EFAULT;
>> memcpy(dest, cmd.params.getrandom_out.buffer, recd);
>>
>> dest += recd;
>> @@ -394,6 +404,10 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
>> (sizeof(struct tpm_input_header) + \
>> sizeof(struct tpm2_get_tpm_pt_in))
>>
>> +#define TPM2_GET_TPM_PT_OUT_SIZE \
>> + (sizeof(struct tpm_output_header) + \
>> + sizeof(struct tpm2_get_tpm_pt_out))
>> +
>> static const struct tpm_input_header tpm2_get_tpm_pt_header = {
>> .tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
>> .length = cpu_to_be32(TPM2_GET_TPM_PT_IN_SIZE),
>> @@ -445,7 +459,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>> {
>> unsigned int blob_len;
>> struct tpm_buf buf;
>> - u32 hash;
>> + u32 hash, rlength;
>> int i;
>> int rc;
>>
>> @@ -510,7 +524,9 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>> goto out;
>> }
>>
>> - rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 0, "sealing data");
>> + rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE,
>> + TPM_HEADER_SIZE + 4,
>> + 0, "sealing data");
>> if (rc)
>> goto out;
>>
>> @@ -519,6 +535,11 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>> rc = -E2BIG;
>> goto out;
>> }
>> + rlength = be32_to_cpu(((struct tpm2_cmd *)&buf)->header.out.length);
>> + if (rlength < TPM_HEADER_SIZE + 4 + blob_len) {
>> + rc = -EFAULT;
>> + goto out;
>> + }
>>
>> memcpy(payload->blob, &buf.data[TPM_HEADER_SIZE + 4], blob_len);
>> payload->blob_len = blob_len;
>> @@ -588,7 +609,8 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
>> goto out;
>> }
>>
>> - rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, flags, "loading blob");
>> + rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE,
>> + TPM_HEADER_SIZE + 4, flags, "loading blob");
>> if (!rc)
>> *blob_handle = be32_to_cpup(
>> (__be32 *) &buf.data[TPM_HEADER_SIZE]);
>> @@ -626,8 +648,8 @@ static void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
>>
>> tpm_buf_append_u32(&buf, handle);
>>
>> - rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, flags,
>> - "flushing context");
>> + rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, TPM_HEADER_SIZE,
>> + flags, "flushing context");
>> if (rc)
>> dev_warn(&chip->dev, "0x%08x was not flushed, rc=%d\n", handle,
>> rc);
>> @@ -657,6 +679,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
>> u16 data_len;
>> u8 *data;
>> int rc;
>> + u32 rlength;
>>
>> rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
>> if (rc)
>> @@ -671,13 +694,21 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
>> options->blobauth /* hmac */,
>> TPM_DIGEST_SIZE);
>>
>> - rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, flags, "unsealing");
>> + rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE,
>> + TPM_HEADER_SIZE + 4 + 2, flags, "unsealing");
>> if (rc > 0)
>> rc = -EPERM;
>>
>> if (!rc) {
>> data_len = be16_to_cpup(
>> (__be16 *) &buf.data[TPM_HEADER_SIZE + 4]);
>> +
>> + rlength = be32_to_cpu(((struct tpm2_cmd *)&buf)
>> + ->header.out.length);
>> + if (rlength < TPM_HEADER_SIZE + 4 + 2 + data_len) {
>> + rc = -EFAULT;
>> + goto out;
>> + }
>> data = &buf.data[TPM_HEADER_SIZE + 6];
>>
>> memcpy(payload->key, data, data_len - 1);
>> @@ -685,6 +716,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
>> payload->migratable = data[data_len - 1];
>> }
>>
>> +out:
>> tpm_buf_destroy(&buf);
>> return rc;
>> }
>> @@ -739,7 +771,8 @@ 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), 0, desc);
>> + rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd),
>> + TPM2_GET_TPM_PT_OUT_SIZE, 0, desc);
>> if (!rc)
>> *value = be32_to_cpu(cmd.params.get_tpm_pt_out.value);
>>
>> @@ -773,8 +806,8 @@ 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), 0,
>> - "attempting to start the TPM");
>> + return tpm_transmit_cmd(chip, &cmd, sizeof(cmd), TPM_HEADER_SIZE,
>> + 0, "attempting to start the TPM");
>> }
>>
>> #define TPM2_SHUTDOWN_IN_SIZE \
>> @@ -802,7 +835,8 @@ 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), 0, "stopping the TPM");
>> + rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), TPM_HEADER_SIZE,
>> + 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.
>> @@ -865,8 +899,8 @@ 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, 0,
>> - "continue selftest");
>> + rc = tpm_transmit_cmd(chip, &cmd, TPM2_SELF_TEST_IN_SIZE,
>> + TPM_HEADER_SIZE, 0, "continue selftest");
>>
>> /* At least some prototype chips seem to give RC_TESTING error
>> * immediately. This is a workaround for that.
>> @@ -916,7 +950,8 @@ 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, &cmd, sizeof(cmd), 0, NULL);
>> + rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), TPM_HEADER_SIZE,
>> + 0, NULL);
>> if (rc < 0)
>> break;
>>
>> @@ -949,7 +984,8 @@ 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_cmd(chip, &cmd, sizeof(cmd), 0, NULL);
>> + rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), TPM_HEADER_SIZE,
>> + 0, NULL);
>> if (rc < 0)
>> return rc;
>>
>> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
>> index 7993678..025ddb5 100644
>> --- a/drivers/char/tpm/tpm_tis_core.c
>> +++ b/drivers/char/tpm/tpm_tis_core.c
>> @@ -552,7 +552,8 @@ static int tpm_tis_gen_interrupt(struct tpm_chip *chip)
>> if (chip->flags & TPM_CHIP_FLAG_TPM2)
>> return tpm2_get_tpm_pt(chip, 0x100, &cap2, desc);
>> else
>> - return tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, desc);
>> + return tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, desc,
>> + 0, NULL);
>> }
>>
>> /* Register the IRQ and issue a command that will cause an interrupt. If an
>> --
>> 2.4.3
>>
> /Jarkko
>
Powered by blists - more mailing lists