Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1549178 > unrolled thread
| Started by | Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> |
|---|---|
| First post | 2017-01-02 14:30 +0100 |
| Last post | 2017-01-11 11:10 +0100 |
| Articles | 20 on this page of 67 — 8 participants |
Back to article view | Back to linux.kernel
[PATCH RFC 0/4] RFC: in-kernel resource manager Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-02 14:30 +0100
[PATCH RFC 2/4] tpm: validate TPM 2.0 commands Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-02 14:30 +0100
Re: [tpmdd-devel] [PATCH RFC 2/4] tpm: validate TPM 2.0 commands James Bottomley <jejb@linux.vnet.ibm.com> - 2017-01-04 19:30 +0100
Re: [tpmdd-devel] [PATCH RFC 2/4] tpm: validate TPM 2.0 commands Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-04 19:50 +0100
[PATCH RFC 3/4] tpm: export tpm2_flush_context_cmd Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-02 14:30 +0100
[PATCH RFC 4/4] tpm: add the infrastructure for TPM space for TPM 2.0 Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-02 14:30 +0100
Re: [PATCH RFC 4/4] tpm: add the infrastructure for TPM space for TPM 2.0 Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-02 22:20 +0100
Re: [PATCH RFC 4/4] tpm: add the infrastructure for TPM space for TPM 2.0 Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-03 05:10 +0100
Re: [PATCH RFC 4/4] tpm: add the infrastructure for TPM space for TPM 2.0 Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-03 19:50 +0100
Re: [PATCH RFC 4/4] tpm: add the infrastructure for TPM space for TPM 2.0 Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-04 13:50 +0100
Re: [PATCH RFC 4/4] tpm: add the infrastructure for TPM space for TPM 2.0 Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-03 20:20 +0100
Re: [PATCH RFC 4/4] tpm: add the infrastructure for TPM space for TPM 2.0 Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-04 13:50 +0100
Re: [tpmdd-devel] [PATCH RFC 4/4] tpm: add the infrastructure for TPM space for TPM 2.0 Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-09 23:20 +0100
[PATCH RFC 1/4] tpm: migrate struct tpm_buf to struct tpm_chip Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-02 14:30 +0100
Re: [PATCH RFC 1/4] tpm: migrate struct tpm_buf to struct tpm_chip Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-02 22:10 +0100
Re: [PATCH RFC 1/4] tpm: migrate struct tpm_buf to struct tpm_chip Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-03 05:10 +0100
Re: [PATCH RFC 1/4] tpm: migrate struct tpm_buf to struct tpm_chip Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-03 20:20 +0100
Re: [PATCH RFC 1/4] tpm: migrate struct tpm_buf to struct tpm_chip Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-04 13:40 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-01-02 17:40 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-01-02 22:50 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager James Bottomley <jejb@linux.vnet.ibm.com> - 2017-01-03 06:30 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-03 14:50 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager James Bottomley <jejb@linux.vnet.ibm.com> - 2017-01-03 17:20 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-03 19:40 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-03 20:20 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager James Bottomley <jejb@linux.vnet.ibm.com> - 2017-01-04 02:30 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-03 23:00 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-04 14:10 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-04 18:00 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Andy Lutomirski <luto@kernel.org> - 2017-01-04 06:50 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-04 14:10 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-03 15:00 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-01-03 17:40 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-03 19:50 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-03 22:50 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-01-03 23:50 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-04 01:20 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-01-04 01:40 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-04 02:00 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-04 14:00 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-01-04 16:00 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-01-04 20:10 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-04 20:30 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-04 20:30 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-04 00:30 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-04 13:50 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Ken Goldman <kgoldman@us.ibm.com> - 2017-01-04 15:00 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-03 05:10 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-03 22:40 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-01-03 23:10 +0100
RE: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager "Fuchs, Andreas" <andreas.fuchs@sit.fraunhofer.de> - 2017-01-05 17:10 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-05 18:30 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager James Bottomley <jejb@linux.vnet.ibm.com> - 2017-01-05 19:10 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Andreas Fuchs <andreas.fuchs@sit.fraunhofer.de> - 2017-01-06 09:50 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager James Bottomley <jejb@linux.vnet.ibm.com> - 2017-01-05 19:40 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-05 21:00 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager James Bottomley <jejb@linux.vnet.ibm.com> - 2017-01-05 21:00 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-05 23:30 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-06 01:00 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager James Bottomley <jejb@linux.vnet.ibm.com> - 2017-01-06 01:40 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Andreas Fuchs <andreas.fuchs@sit.fraunhofer.de> - 2017-01-06 10:10 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-06 20:20 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-06 20:10 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager James Bottomley <jejb@linux.vnet.ibm.com> - 2017-01-06 02:40 +0100
Re: [PATCH RFC 0/4] RFC: in-kernel resource manager Ken Goldman <kgoldman@us.ibm.com> - 2017-01-10 20:20 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-09 23:40 +0100
Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Andreas Fuchs <andreas.fuchs@sit.fraunhofer.de> - 2017-01-11 11:10 +0100
Page 1 of 4 [1] 2 3 4 Next page →
| From | Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> |
|---|---|
| Date | 2017-01-02 14:30 +0100 |
| Subject | [PATCH RFC 0/4] RFC: in-kernel resource manager |
| Message-ID | <sVaQx-7mn-5@gated-at.bofh.it> |
This patch set adds support for TPM spaces that provide a context for isolating and swapping transient objects. This patch set does not yet include support for isolating policy and HMAC sessions but it is trivial to add once the basic approach is settled (and that's why I created an RFC patch set). There's a test script for trying out TPM spaces in git://git.infradead.org/users/jjs/tpm2-scripts.git A simple smoke test can be run by sudo python -m unittest -v tpm2_smoke.SpaceTest Jarkko Sakkinen (4): tpm: migrate struct tpm_buf to struct tpm_chip tpm: validate TPM 2.0 commands tpm: export tpm2_flush_context_cmd tpm: add the infrastructure for TPM space for TPM 2.0 drivers/char/tpm/Makefile | 2 +- drivers/char/tpm/tpm-chip.c | 15 ++ drivers/char/tpm/tpm-dev.c | 80 ++++++++++- drivers/char/tpm/tpm-interface.c | 93 +++++++++---- drivers/char/tpm/tpm-sysfs.c | 2 +- drivers/char/tpm/tpm.h | 106 ++++++++------ drivers/char/tpm/tpm2-cmd.c | 232 ++++++++++++++++--------------- drivers/char/tpm/tpm2-space.c | 288 +++++++++++++++++++++++++++++++++++++++ include/uapi/linux/tpm.h | 23 ++++ 9 files changed, 662 insertions(+), 179 deletions(-) create mode 100644 drivers/char/tpm/tpm2-space.c create mode 100644 include/uapi/linux/tpm.h -- 2.9.3
[toc] | [next] | [standalone]
| From | Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> |
|---|---|
| Date | 2017-01-02 14:30 +0100 |
| Subject | [PATCH RFC 2/4] tpm: validate TPM 2.0 commands |
| Message-ID | <sVaQx-7mn-3@gated-at.bofh.it> |
| In reply to | #1549178 |
Check for every TPM 2.0 command that the command code is supported and
the command buffer has at least the length that can contain the header
and the handle area.
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
drivers/char/tpm/tpm-interface.c | 32 +++++++++++++++++++++++++++++-
drivers/char/tpm/tpm.h | 15 +++++++++++++-
drivers/char/tpm/tpm2-cmd.c | 43 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 88 insertions(+), 2 deletions(-)
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 769d8b0..0794a5d3 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -328,6 +328,36 @@ unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip,
}
EXPORT_SYMBOL_GPL(tpm_calc_ordinal_duration);
+static bool tpm_validate_command(struct tpm_chip *chip, const u8 *cmd,
+ size_t len)
+{
+ const struct tpm_input_header *header = (const void *)cmd;
+ u32 cc;
+ size_t len_min = TPM_HEADER_SIZE;
+ u32 attrs;
+
+ if ((len >= len_min) && (chip->flags & TPM_CHIP_FLAG_TPM2) &&
+ chip->nr_commands) {
+ cc = be32_to_cpu(header->ordinal);
+ if (!tpm2_find_cc_attrs(chip, cc, &attrs)) {
+ dev_dbg(&chip->dev, "0x%04x is an invalid command\n",
+ cc);
+ return false;
+ }
+ len_min +=
+ 4 * ((attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0));
+ }
+
+ if (len < len_min) {
+ dev_dbg(&chip->dev,
+ "%s: insufficient command length %zu < %zu\n",
+ __func__, len, len_min);
+ return false;
+ }
+
+ return true;
+}
+
/**
* tmp_transmit - Internal kernel interface to transmit TPM commands.
*
@@ -347,7 +377,7 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
u32 count, ordinal;
unsigned long stop;
- if (bufsiz < TPM_HEADER_SIZE)
+ if (!tpm_validate_command(chip, buf, bufsiz))
return -EINVAL;
if (bufsiz > TPM_BUFSIZE)
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index c74a663..ed21c2c 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -127,7 +127,12 @@ enum tpm2_permanent_handles {
};
enum tpm2_capabilities {
- TPM2_CAP_TPM_PROPERTIES = 6,
+ TPM2_CAP_COMMANDS = 2,
+ TPM2_CAP_TPM_PROPERTIES = 6,
+};
+
+enum tpm2_properties {
+ TPM_PT_TOTAL_COMMANDS = 0x0129,
};
enum tpm2_startup_types {
@@ -135,6 +140,11 @@ enum tpm2_startup_types {
TPM2_SU_STATE = 0x0001,
};
+enum tpm2_cc_attrs {
+ TPM2_CC_ATTR_CHANDLES = 25,
+ TPM2_CC_ATTR_RHANDLE = 28,
+};
+
#define TPM_VID_INTEL 0x8086
#define TPM_VID_WINBOND 0x1050
#define TPM_VID_STM 0x104A
@@ -207,6 +217,8 @@ struct tpm_chip {
#endif /* CONFIG_ACPI */
struct tpm_buf tr_buf;
+ u32 nr_commands;
+ u32 *cc_attrs_tbl;
};
#define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
@@ -534,4 +546,5 @@ int tpm2_auto_startup(struct tpm_chip *chip);
void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
int tpm2_probe(struct tpm_chip *chip);
+bool tpm2_find_cc_attrs(struct tpm_chip *chip, u32 cc, u32 *attrs);
#endif
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index f0e0807..fa928c7 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -943,7 +943,9 @@ EXPORT_SYMBOL_GPL(tpm2_probe);
*/
int tpm2_auto_startup(struct tpm_chip *chip)
{
+ u32 nr_commands;
int rc;
+ int i;
rc = tpm_get_timeouts(chip);
if (rc)
@@ -967,8 +969,49 @@ int tpm2_auto_startup(struct tpm_chip *chip)
}
}
+ rc = tpm2_get_tpm_pt(chip, TPM_PT_TOTAL_COMMANDS, &nr_commands, NULL);
+ if (rc)
+ return rc;
+
+ chip->cc_attrs_tbl = devm_kzalloc(&chip->dev, 4 * nr_commands,
+ GFP_KERNEL);
+
+ tpm_buf_init(&chip->tr_buf, TPM2_ST_NO_SESSIONS,
+ TPM2_CC_GET_CAPABILITY);
+ tpm_buf_append_u32(&chip->tr_buf, TPM2_CAP_COMMANDS);
+ tpm_buf_append_u32(&chip->tr_buf, TPM2_CC_FIRST);
+ tpm_buf_append_u32(&chip->tr_buf, nr_commands);
+
+ rc = tpm_transmit_cmd(chip, chip->tr_buf.data, TPM_BUFSIZE, 0, NULL);
+ if (rc < 0)
+ goto out;
+
+ if (nr_commands !=
+ be32_to_cpup((__be32 *)&chip->tr_buf.data[TPM_HEADER_SIZE + 5]))
+ return -EINVAL;
+
+ for (i = 0; i < nr_commands; i++)
+ chip->cc_attrs_tbl[i] = be32_to_cpup(
+ (u32 *)&chip->tr_buf.data[TPM_HEADER_SIZE + 9 + 4 * i]);
+
+ chip->nr_commands = nr_commands;
+
out:
if (rc > 0)
rc = -ENODEV;
return rc;
}
+
+bool tpm2_find_cc_attrs(struct tpm_chip *chip, u32 cc, u32 *attrs)
+{
+ int i;
+
+ for (i = 0; i < chip->nr_commands; i++) {
+ if (cc == (chip->cc_attrs_tbl[i] & GENMASK(15, 0))) {
+ *attrs = chip->cc_attrs_tbl[i];
+ return true;
+ }
+ }
+
+ return false;
+}
--
2.9.3
[toc] | [prev] | [next] | [standalone]
| From | James Bottomley <jejb@linux.vnet.ibm.com> |
|---|---|
| Date | 2017-01-04 19:30 +0100 |
| Subject | Re: [tpmdd-devel] [PATCH RFC 2/4] tpm: validate TPM 2.0 commands |
| Message-ID | <sVYtY-8k8-31@gated-at.bofh.it> |
| In reply to | #1549179 |
On Wed, 2017-01-04 at 13:04 -0500, Stefan Berger wrote:
> Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote on 01/02/2017
> 08:22:08 AM:
>
> > --- a/drivers/char/tpm/tpm2-cmd.c
> > +++ b/drivers/char/tpm/tpm2-cmd.c
> > @@ -943,7 +943,9 @@ EXPORT_SYMBOL_GPL(tpm2_probe);
> > */
> > int tpm2_auto_startup(struct tpm_chip *chip)
> > {
> > + u32 nr_commands;
> > int rc;
> > + int i;
> >
> > rc = tpm_get_timeouts(chip);
> > if (rc)
> > @@ -967,8 +969,49 @@ int tpm2_auto_startup(struct tpm_chip *chip)
> > }
> > }
> >
> > + rc = tpm2_get_tpm_pt(chip, TPM_PT_TOTAL_COMMANDS, &nr_commands,
> NULL);
> > + if (rc)
> > + return rc;
> > +
> > + chip->cc_attrs_tbl = devm_kzalloc(&chip->dev, 4 * nr_commands,
> > + GFP_KERNEL);
>
> For some reason this devm_kzalloc bombs for the vtpm proxy driver.
> The only reason I could come up with is that it's being called before
> tpm_add_char_device() has been called.
No, it should be sufficient that chip->dev be initialized (which it is
in tpm_chip_alloc()). What's the error you're getting?
It does look like the intention was to have non-devm with
tpm_chip_alloc() and devm with tpmm_chip_alloc(), but devm_kzalloc
should just work regardless because it's tied to the device model.
James
[toc] | [prev] | [next] | [standalone]
| From | Jason Gunthorpe <jgunthorpe@obsidianresearch.com> |
|---|---|
| Date | 2017-01-04 19:50 +0100 |
| Subject | Re: [tpmdd-devel] [PATCH RFC 2/4] tpm: validate TPM 2.0 commands |
| Message-ID | <sVYNk-8qL-39@gated-at.bofh.it> |
| In reply to | #1549179 |
On Wed, Jan 04, 2017 at 01:04:59PM -0500, Stefan Berger wrote:
> > @@ -943,7 +943,9 @@ EXPORT_SYMBOL_GPL(tpm2_probe);
> > */
> > int tpm2_auto_startup(struct tpm_chip *chip)
> > {
> > + u32 nr_commands;
> > int rc;
> > + int i;
> >
> > rc = tpm_get_timeouts(chip);
> > if (rc)
> > @@ -967,8 +969,49 @@ int tpm2_auto_startup(struct tpm_chip *chip)
> > }
> > }
> >
> > + rc = tpm2_get_tpm_pt(chip, TPM_PT_TOTAL_COMMANDS, &nr_commands,
> NULL);
> > + if (rc)
> > + return rc;
> > +
> > + chip->cc_attrs_tbl = devm_kzalloc(&chip->dev, 4 * nr_commands,
> > + GFP_KERNEL);
> For some reason this devm_kzalloc bombs for the vtpm proxy driver. The
> only reason I could come up with is that it's being called before
> tpm_add_char_device() has been called.
It would also fail if nr_commands is wrong, and this should be one of
the array safe allocation functions since nr_command is data from the
TPM...
Jason
[toc] | [prev] | [next] | [standalone]
| From | Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> |
|---|---|
| Date | 2017-01-02 14:30 +0100 |
| Subject | [PATCH RFC 3/4] tpm: export tpm2_flush_context_cmd |
| Message-ID | <sVaQy-7mn-21@gated-at.bofh.it> |
| In reply to | #1549178 |
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
drivers/char/tpm/tpm.h | 2 ++
drivers/char/tpm/tpm2-cmd.c | 47 ++++++++++++++++++++++-----------------------
2 files changed, 25 insertions(+), 24 deletions(-)
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index ed21c2c..fb02b57 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -533,6 +533,8 @@ static inline void tpm_add_ppi(struct tpm_chip *chip)
int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash);
int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max);
+void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
+ unsigned int flags);
int tpm2_seal_trusted(struct tpm_chip *chip,
struct trusted_key_payload *payload,
struct trusted_key_options *options);
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index fa928c7..311dc8e 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -401,6 +401,29 @@ static const struct tpm_input_header tpm2_get_tpm_pt_header = {
};
/**
+ * tpm2_flush_context_cmd() - execute a TPM2_FlushContext command
+ * @chip: 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
+ */
+void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
+ unsigned int flags)
+{
+ int rc;
+
+ tpm_buf_init(&chip->tr_buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
+ tpm_buf_append_u32(&chip->tr_buf, handle);
+
+ rc = tpm_transmit_cmd(chip, chip->tr_buf.data, TPM_BUFSIZE, flags,
+ "flushing context");
+ if (rc)
+ dev_warn(&chip->dev, "0x%08x was not flushed, rc=%d\n", handle,
+ rc);
+}
+
+/**
* tpm_buf_append_auth() - append TPMS_AUTH_COMMAND to the buffer.
*
* @buf: an allocated tpm_buf instance
@@ -589,30 +612,6 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
}
/**
- * tpm2_flush_context_cmd() - execute a TPM2_FlushContext command
- *
- * @chip: TPM chip to use
- * @handle: the key data in clear and encrypted form
- * @flags: tpm transmit flags
- *
- * Return: Same as with tpm_transmit_cmd.
- */
-static void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
- unsigned int flags)
-{
- int rc;
-
- tpm_buf_init(&chip->tr_buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
- tpm_buf_append_u32(&chip->tr_buf, handle);
-
- rc = tpm_transmit_cmd(chip, &chip->tr_buf, TPM_BUFSIZE, flags,
- "flushing context");
- if (rc)
- dev_warn(&chip->dev, "0x%08x was not flushed, rc=%d\n", handle,
- rc);
-}
-
-/**
* tpm2_unseal_cmd() - execute a TPM2_Unload command
*
* @chip: TPM chip to use
--
2.9.3
[toc] | [prev] | [next] | [standalone]
| From | Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> |
|---|---|
| Date | 2017-01-02 14:30 +0100 |
| Subject | [PATCH RFC 4/4] tpm: add the infrastructure for TPM space for TPM 2.0 |
| Message-ID | <sVaQy-7mn-25@gated-at.bofh.it> |
| In reply to | #1549178 |
Added a ioctl for creating a TPM space. The space is isolated from the
other users of the TPM. Only a process holding the file with the handle
can access the objects and only objects that are created through that
file handle can be accessed.
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
drivers/char/tpm/Makefile | 2 +-
drivers/char/tpm/tpm-chip.c | 7 +
drivers/char/tpm/tpm-dev.c | 80 ++++++++++-
drivers/char/tpm/tpm-interface.c | 61 +++++----
drivers/char/tpm/tpm-sysfs.c | 2 +-
drivers/char/tpm/tpm.h | 22 ++-
drivers/char/tpm/tpm2-cmd.c | 30 ++--
drivers/char/tpm/tpm2-space.c | 288 +++++++++++++++++++++++++++++++++++++++
include/uapi/linux/tpm.h | 23 ++++
9 files changed, 470 insertions(+), 45 deletions(-)
create mode 100644 drivers/char/tpm/tpm2-space.c
create mode 100644 include/uapi/linux/tpm.h
diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index a05b1eb..251d0ed 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -3,7 +3,7 @@
#
obj-$(CONFIG_TCG_TPM) += tpm.o
tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
- tpm_eventlog.o
+ tpm_eventlog.o tpm2-space.o
tpm-$(CONFIG_ACPI) += tpm_ppi.o tpm_acpi.o
tpm-$(CONFIG_OF) += tpm_of.o
obj-$(CONFIG_TCG_TIS_CORE) += tpm_tis_core.o
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 41e518e..468af50 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -129,6 +129,7 @@ static void tpm_dev_release(struct device *dev)
kfree(chip->log.bios_event_log);
kfree(chip->tr_buf.data);
+ kfree(chip->work_space.context_buf);
kfree(chip);
}
@@ -197,6 +198,12 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
}
chip->tr_buf.size = TPM_BUFSIZE;
+ chip->work_space.context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
+ if (!chip->work_space.context_buf) {
+ rc = -ENOMEM;
+ goto out;
+ }
+
return chip;
out:
diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
index 912ad30..139638b 100644
--- a/drivers/char/tpm/tpm-dev.c
+++ b/drivers/char/tpm/tpm-dev.c
@@ -19,6 +19,7 @@
*/
#include <linux/slab.h>
#include <linux/uaccess.h>
+#include <uapi/linux/tpm.h>
#include "tpm.h"
struct file_priv {
@@ -32,6 +33,8 @@ struct file_priv {
struct work_struct work;
u8 data_buffer[TPM_BUFSIZE];
+ struct tpm_space space;
+ bool has_space;
};
static void user_reader_timeout(unsigned long ptr)
@@ -115,6 +118,7 @@ static ssize_t tpm_write(struct file *file, const char __user *buf,
size_t size, loff_t *off)
{
struct file_priv *priv = file->private_data;
+ struct tpm_space *space = NULL;
size_t in_size = size;
ssize_t out_size;
@@ -130,6 +134,9 @@ static ssize_t tpm_write(struct file *file, const char __user *buf,
mutex_lock(&priv->buffer_mutex);
+ if (priv->has_space)
+ space = &priv->space;
+
if (copy_from_user
(priv->data_buffer, (void __user *) buf, in_size)) {
mutex_unlock(&priv->buffer_mutex);
@@ -144,7 +151,7 @@ static ssize_t tpm_write(struct file *file, const char __user *buf,
mutex_unlock(&priv->buffer_mutex);
return -EPIPE;
}
- out_size = tpm_transmit(priv->chip, priv->data_buffer,
+ out_size = tpm_transmit(priv->chip, space, priv->data_buffer,
sizeof(priv->data_buffer), 0);
tpm_put_ops(priv->chip);
@@ -162,6 +169,65 @@ static ssize_t tpm_write(struct file *file, const char __user *buf,
return in_size;
}
+/**
+ * tpm_ioc_new_space - handler for %SGX_IOC_NEW_SPACE ioctl
+ *
+ * Creates a new TPM space that can hold a set of transient objects. The space
+ * is isolated with virtual handles that are mapped into physical handles by the
+ * driver.
+ */
+static long tpm_ioc_new_space(struct file *file, unsigned int ioctl,
+ unsigned long arg)
+{
+ struct file_priv *priv = file->private_data;
+ struct tpm_chip *chip = priv->chip;
+ int rc = 0;
+
+ if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
+ return -EOPNOTSUPP;
+
+ mutex_lock(&priv->buffer_mutex);
+
+ if (priv->has_space) {
+ rc = -EBUSY;
+ goto out;
+ }
+
+ priv->space.context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
+ if (!priv->space.context_buf) {
+ rc = -ENOMEM;
+ goto out;
+ }
+
+ /* The TPM device can be opened again as this file has been moved to a
+ * TPM handle space.
+ */
+ priv->has_space = true;
+ clear_bit(0, &chip->is_open);
+out:
+ mutex_unlock(&priv->buffer_mutex);
+ return rc;
+}
+
+static long tpm_ioctl(struct file *file, unsigned int ioctl,
+ unsigned long arg)
+{
+ switch (ioctl) {
+ case TPM_IOC_NEW_SPACE:
+ return tpm_ioc_new_space(file, ioctl, arg);
+ default:
+ return -ENOIOCTLCMD;
+ }
+}
+
+#ifdef CONFIG_COMPAT
+static long tpm_compat_ioctl(struct file *file, unsigned int ioctl,
+ unsigned long arg)
+{
+ return tpm_ioctl(file, ioctl, arg);
+}
+#endif
+
/*
* Called on file close
*/
@@ -169,6 +235,14 @@ static int tpm_release(struct inode *inode, struct file *file)
{
struct file_priv *priv = file->private_data;
+ if (tpm_try_get_ops(priv->chip)) {
+ mutex_unlock(&priv->buffer_mutex);
+ return -EPIPE;
+ }
+ if (priv->has_space)
+ kfree(priv->space.context_buf);
+ tpm_put_ops(priv->chip);
+
del_singleshot_timer_sync(&priv->user_read_timer);
flush_work(&priv->work);
file->private_data = NULL;
@@ -184,6 +258,10 @@ const struct file_operations tpm_fops = {
.open = tpm_open,
.read = tpm_read,
.write = tpm_write,
+ .unlocked_ioctl = tpm_ioctl,
+#ifdef CONFIG_COMPAT
+ .compat_ioctl = tpm_compat_ioctl,
+#endif
.release = tpm_release,
};
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 0794a5d3..a1ae57e 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -370,10 +370,11 @@ static bool tpm_validate_command(struct tpm_chip *chip, const u8 *cmd,
* 0 when the operation is successful.
* A negative number for system errors (errno).
*/
-ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
- unsigned int flags)
+ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
+ u8 *buf, size_t bufsiz, unsigned int flags)
{
- ssize_t rc;
+ int rc;
+ ssize_t len = 0;
u32 count, ordinal;
unsigned long stop;
@@ -399,10 +400,14 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
if (chip->dev.parent)
pm_runtime_get_sync(chip->dev.parent);
+ rc = tpm2_prepare_space(chip, space, ordinal, buf, bufsiz);
+ if (rc)
+ goto out;
+
rc = chip->ops->send(chip, (u8 *) buf, count);
if (rc < 0) {
dev_err(&chip->dev,
- "tpm_transmit: tpm_send: error %zd\n", rc);
+ "tpm_transmit: tpm_send: error %d\n", rc);
goto out;
}
@@ -435,17 +440,23 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
goto out;
out_recv:
- rc = chip->ops->recv(chip, (u8 *) buf, bufsiz);
- if (rc < 0)
+ len = chip->ops->recv(chip, (u8 *) buf, bufsiz);
+ if (len < 0) {
dev_err(&chip->dev,
- "tpm_transmit: tpm_recv: error %zd\n", rc);
+ "tpm_transmit: tpm_recv: error %d\n", rc);
+ rc = len;
+ goto out;
+ }
+
+ rc = tpm2_commit_space(chip, space, ordinal, buf, bufsiz);
+
out:
if (chip->dev.parent)
pm_runtime_put_sync(chip->dev.parent);
if (!(flags & TPM_TRANSMIT_UNLOCKED))
mutex_unlock(&chip->tpm_mutex);
- return rc;
+ return rc ? rc : len;
}
/**
@@ -463,13 +474,14 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
* A negative number for system errors (errno).
* 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)
+ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
+ void *cmd, int len, unsigned int flags,
+ const char *desc)
{
const struct tpm_output_header *header;
int err;
- len = tpm_transmit(chip, (const u8 *)cmd, len, flags);
+ len = tpm_transmit(chip, space, cmd, len, flags);
if (len < 0)
return len;
else if (len < TPM_HEADER_SIZE)
@@ -521,7 +533,7 @@ 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,
+ rc = tpm_transmit_cmd(chip, NULL, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, 0,
desc);
if (!rc)
*cap = tpm_cmd.params.getcap_out.cap;
@@ -545,8 +557,9 @@ 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,
- "attempting to start the TPM");
+ return tpm_transmit_cmd(chip, NULL, &start_cmd,
+ TPM_INTERNAL_RESULT_SIZE,
+ 0, "attempting to start the TPM");
}
int tpm_get_timeouts(struct tpm_chip *chip)
@@ -684,8 +697,8 @@ 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, NULL, &cmd, CONTINUE_SELFTEST_RESULT_SIZE,
+ 0, "continue selftest");
return rc;
}
@@ -704,7 +717,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, 0,
+ rc = tpm_transmit_cmd(chip, NULL, &cmd, READ_PCR_RESULT_SIZE, 0,
"attempting to read a pcr value");
if (rc == 0)
@@ -802,7 +815,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, 0,
+ rc = tpm_transmit_cmd(chip, NULL, &cmd, EXTEND_PCR_RESULT_SIZE, 0,
"attempting extend a PCR value");
tpm_put_ops(chip);
@@ -906,7 +919,7 @@ 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, NULL, cmd, buflen, 0, "attempting tpm_cmd");
tpm_put_ops(chip);
return rc;
@@ -1008,15 +1021,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, 0,
- "extending dummy pcr before suspend");
+ rc = tpm_transmit_cmd(chip, NULL, &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, 0,
- NULL);
+ rc = tpm_transmit_cmd(chip, NULL, &cmd, SAVESTATE_RESULT_SIZE,
+ 0, NULL);
/*
* If the TPM indicates that it is too busy to respond to
@@ -1099,7 +1112,7 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
tpm_cmd.header.in = tpm_getrandom_header;
tpm_cmd.params.getrandom_in.num_bytes = cpu_to_be32(num_bytes);
- err = tpm_transmit_cmd(chip, &tpm_cmd,
+ err = tpm_transmit_cmd(chip, NULL, &tpm_cmd,
TPM_GETRANDOM_RESULT_SIZE + num_bytes,
0, "attempting get random");
if (err)
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 848ad65..dd31a00 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, 0,
+ err = tpm_transmit_cmd(chip, NULL, &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 fb02b57..c6171e5 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -91,6 +91,7 @@ enum tpm2_structures {
enum tpm2_return_codes {
TPM2_RC_HASH = 0x0083, /* RC_FMT1 */
+ TPM2_RC_HANDLE = 0x008B,
TPM2_RC_INITIALIZE = 0x0100, /* RC_VER1 */
TPM2_RC_DISABLED = 0x0120,
TPM2_RC_TESTING = 0x090A, /* RC_WARN */
@@ -114,6 +115,8 @@ enum tpm2_command_codes {
TPM2_CC_CREATE = 0x0153,
TPM2_CC_LOAD = 0x0157,
TPM2_CC_UNSEAL = 0x015E,
+ TPM2_CC_CONTEXT_LOAD = 0x0161,
+ TPM2_CC_CONTEXT_SAVE = 0x0162,
TPM2_CC_FLUSH_CONTEXT = 0x0165,
TPM2_CC_GET_CAPABILITY = 0x017A,
TPM2_CC_GET_RANDOM = 0x017B,
@@ -151,6 +154,11 @@ enum tpm2_cc_attrs {
#define TPM_PPI_VERSION_LEN 3
+struct tpm_space {
+ u32 context_tbl[14];
+ u8 *context_buf;
+};
+
enum tpm_chip_flags {
TPM_CHIP_FLAG_TPM2 = BIT(1),
TPM_CHIP_FLAG_IRQ = BIT(2),
@@ -217,6 +225,7 @@ struct tpm_chip {
#endif /* CONFIG_ACPI */
struct tpm_buf tr_buf;
+ struct tpm_space work_space;
u32 nr_commands;
u32 *cc_attrs_tbl;
};
@@ -492,10 +501,11 @@ 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_transmit(struct tpm_chip *chip, struct tpm_space *space,
+ u8 *buf, size_t bufsiz, unsigned int flags);
+ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
+ void *cmd, int len, unsigned int flags,
+ const char *desc);
ssize_t tpm_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
const char *desc);
int tpm_get_timeouts(struct tpm_chip *);
@@ -549,4 +559,8 @@ void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
int tpm2_probe(struct tpm_chip *chip);
bool tpm2_find_cc_attrs(struct tpm_chip *chip, u32 cc, u32 *attrs);
+int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space,
+ u32 cc, u8 *buf, size_t bufsiz);
+int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
+ u32 cc, u8 *buf, size_t bufsiz);
#endif
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 311dc8e..abaa355 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -280,7 +280,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), 0,
+ rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0,
"attempting to read a pcr value");
if (rc == 0) {
buf = cmd.params.pcrread_out.digest;
@@ -327,7 +327,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, NULL, &cmd, sizeof(cmd), 0,
"attempting extend a PCR value");
return rc;
@@ -373,7 +373,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), 0,
+ err = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0,
"attempting get random");
if (err)
break;
@@ -416,7 +416,7 @@ void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
tpm_buf_init(&chip->tr_buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
tpm_buf_append_u32(&chip->tr_buf, handle);
- rc = tpm_transmit_cmd(chip, chip->tr_buf.data, TPM_BUFSIZE, flags,
+ rc = tpm_transmit_cmd(chip, NULL, chip->tr_buf.data, TPM_BUFSIZE, flags,
"flushing context");
if (rc)
dev_warn(&chip->dev, "0x%08x was not flushed, rc=%d\n", handle,
@@ -529,7 +529,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
if (chip->tr_buf.flags & TPM_BUF_OVERFLOW)
return -E2BIG;
- rc = tpm_transmit_cmd(chip, &chip->tr_buf, TPM_BUFSIZE, 0,
+ rc = tpm_transmit_cmd(chip, NULL, &chip->tr_buf, TPM_BUFSIZE, 0,
"sealing data");
if (rc)
return rc;
@@ -599,7 +599,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
if (chip->tr_buf.flags & TPM_BUF_OVERFLOW)
return -E2BIG;
- rc = tpm_transmit_cmd(chip, &chip->tr_buf, TPM_BUFSIZE, flags,
+ rc = tpm_transmit_cmd(chip, NULL, &chip->tr_buf, TPM_BUFSIZE, flags,
"loading blob");
if (!rc)
*blob_handle = be32_to_cpup(
@@ -644,7 +644,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
options->blobauth /* hmac */,
TPM_DIGEST_SIZE);
- rc = tpm_transmit_cmd(chip, chip->tr_buf.data, TPM_BUFSIZE, flags,
+ rc = tpm_transmit_cmd(chip, NULL, chip->tr_buf.data, TPM_BUFSIZE, flags,
"unsealing");
if (rc > 0)
rc = -EPERM;
@@ -712,7 +712,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), 0, desc);
+ rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, desc);
if (!rc)
*value = be32_to_cpu(cmd.params.get_tpm_pt_out.value);
@@ -746,7 +746,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), 0,
+ return tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0,
"attempting to start the TPM");
}
@@ -775,7 +775,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, NULL, &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.
@@ -838,7 +839,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, 0,
+ rc = tpm_transmit_cmd(chip, NULL, &cmd, TPM2_SELF_TEST_IN_SIZE, 0,
"continue selftest");
/* At least some prototype chips seem to give RC_TESTING error
@@ -889,7 +890,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, &cmd, sizeof(cmd), 0, NULL);
+ rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, NULL);
if (rc < 0)
break;
@@ -922,7 +923,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_cmd(chip, &cmd, sizeof(cmd), 0, NULL);
+ rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, NULL);
if (rc < 0)
return rc;
@@ -981,7 +982,8 @@ int tpm2_auto_startup(struct tpm_chip *chip)
tpm_buf_append_u32(&chip->tr_buf, TPM2_CC_FIRST);
tpm_buf_append_u32(&chip->tr_buf, nr_commands);
- rc = tpm_transmit_cmd(chip, chip->tr_buf.data, TPM_BUFSIZE, 0, NULL);
+ rc = tpm_transmit_cmd(chip, NULL, chip->tr_buf.data, TPM_BUFSIZE, 0,
+ NULL);
if (rc < 0)
goto out;
diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
new file mode 100644
index 0000000..12a84e6
--- /dev/null
+++ b/drivers/char/tpm/tpm2-space.c
@@ -0,0 +1,288 @@
+/*
+ * Copyright (C) 2016 Intel Corporation
+ *
+ * Authors:
+ * Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
+ *
+ * Maintained by: <tpmdd-devel@lists.sourceforge.net>
+ *
+ * This file contains TPM2 protocol implementations of the commands
+ * used by the kernel internally.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ */
+
+#include <linux/gfp.h>
+#include "tpm.h"
+
+enum tpm2_handle_types {
+ TPM2_HT_HMAC_SESSION = 0x02000000,
+ TPM2_HT_POLICY_SESSION = 0x03000000,
+ TPM2_HT_TRANSIENT = 0x80000000,
+};
+
+static void tpm2_flush_space(struct tpm_chip *chip)
+{
+ struct tpm_space *space = &chip->work_space;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(space->context_tbl); i++)
+ if (space->context_tbl[i] && ~space->context_tbl[i])
+ tpm2_flush_context_cmd(chip, space->context_tbl[i],
+ TPM_TRANSMIT_UNLOCKED);
+}
+
+struct tpm2_context {
+ __be64 sequence;
+ __be32 saved_handle;
+ __be32 hierarchy;
+ __be16 blob_size;
+} __packed;
+
+static int tpm2_load_space(struct tpm_chip *chip)
+{
+ struct tpm_space *space = &chip->work_space;
+ struct tpm2_context *ctx;
+ int i;
+ int j;
+ int rc;
+ u32 s;
+
+ for (i = 0, j = 0; i < ARRAY_SIZE(space->context_tbl); i++) {
+ if (!space->context_tbl[i])
+ continue;
+
+ /* sanity check, should never happen */
+ if (~space->context_tbl[i]) {
+ dev_err(&chip->dev, "context table is inconsistent");
+ return -EFAULT;
+ }
+
+ tpm_buf_init(&chip->tr_buf, TPM2_ST_NO_SESSIONS,
+ TPM2_CC_CONTEXT_LOAD);
+
+ ctx = (struct tpm2_context *)&space->context_buf[j];
+ s = sizeof(*ctx) + be16_to_cpu(ctx->blob_size);
+ tpm_buf_append(&chip->tr_buf, &space->context_buf[j], s);
+
+ rc = tpm_transmit_cmd(chip, NULL, chip->tr_buf.data, PAGE_SIZE,
+ TPM_TRANSMIT_UNLOCKED, NULL);
+ if (rc) {
+ dev_warn(&chip->dev, "%s: loading failed with %d\n",
+ __func__, rc);
+ rc = -EFAULT;
+ goto out_err;
+ }
+
+ space->context_tbl[i] =
+ be32_to_cpup(
+ (__be32 *)&chip->tr_buf.data[TPM_HEADER_SIZE]);
+
+ j += s;
+ }
+
+ return 0;
+
+out_err:
+ tpm2_flush_space(chip);
+ return rc;
+}
+
+static int tpm2_map_command(struct tpm_chip *chip, u32 cc, u8 *cmd, size_t len)
+{
+ struct tpm_space *space = &chip->work_space;
+ unsigned int nr_handles;
+ u32 vhandle;
+ u32 phandle;
+ u32 attrs;
+ int i;
+ int j;
+ int rc;
+
+ if (!tpm2_find_cc_attrs(chip, cc, &attrs)) {
+ rc = -EINVAL;
+ goto out_err;
+ }
+
+ nr_handles = (attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0);
+
+ for (i = 0; i < nr_handles; i++) {
+ vhandle = be32_to_cpup((__be32 *)&cmd[TPM_HEADER_SIZE + 4 * i]);
+ if ((vhandle & 0xFF000000) != TPM2_HT_TRANSIENT)
+ continue;
+
+ j = 0xFFFFFF - (vhandle & 0xFFFFFF);
+ if (j > ARRAY_SIZE(space->context_tbl) ||
+ !space->context_tbl[j]) {
+ rc = -EINVAL;
+ goto out_err;
+ }
+
+ phandle = space->context_tbl[j];
+ *((__be32 *)&cmd[TPM_HEADER_SIZE + 4 * i]) =
+ cpu_to_be32(phandle);
+ }
+
+ return 0;
+
+out_err:
+ tpm2_flush_space(chip);
+ return rc;
+}
+
+int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space,
+ u32 cc, u8 *buf, size_t bufsiz)
+{
+ int rc;
+
+ if (!space)
+ return 0;
+
+ memcpy(&chip->work_space.context_tbl, &space->context_tbl,
+ sizeof(space->context_tbl));
+ memcpy(chip->work_space.context_buf, space->context_buf, PAGE_SIZE);
+
+ rc = tpm2_load_space(chip);
+ if (rc)
+ return rc;
+
+ rc = tpm2_map_command(chip, cc, buf, bufsiz);
+ if (rc)
+ return rc;
+
+ return 0;
+}
+
+static int tpm2_map_response(struct tpm_chip *chip, u32 cc, u8 *rsp, size_t len)
+{
+ struct tpm_space *space = &chip->work_space;
+ u32 phandle;
+ u32 vhandle;
+ u32 attrs;
+ int i;
+ int rc;
+
+ if (!tpm2_find_cc_attrs(chip, cc, &attrs)) {
+ /* should never happen */
+ dev_err(&chip->dev, "TPM returned a different CC: 0x%04x\n",
+ cc);
+ rc = -EFAULT;
+ goto out_err;
+ }
+
+ if (!((attrs >> TPM2_CC_ATTR_RHANDLE) & 1))
+ return 0;
+
+ phandle = be32_to_cpup((__be32 *)&rsp[TPM_HEADER_SIZE]);
+ if ((phandle & 0xFF000000) != TPM2_HT_TRANSIENT)
+ return 0;
+
+ /* Garbage collect a dead context. */
+ for (i = 0; i < ARRAY_SIZE(space->context_tbl); i++) {
+ if (space->context_tbl[i] == phandle) {
+ space->context_tbl[i] = 0;
+ break;
+ }
+ }
+
+ for (i = 0; i < ARRAY_SIZE(space->context_tbl); i++)
+ if (!space->context_tbl[i])
+ break;
+
+ if (i == ARRAY_SIZE(space->context_tbl)) {
+ dev_warn(&chip->dev, "%s: out of context slots\n", __func__);
+ tpm2_flush_context_cmd(chip, phandle, TPM_TRANSMIT_UNLOCKED);
+ rc = -ENOMEM;
+ goto out_err;
+ }
+
+ space->context_tbl[i] = phandle;
+ vhandle = TPM2_HT_TRANSIENT | (0xFFFFFF - i);
+ *(__be32 *)&rsp[TPM_HEADER_SIZE] = cpu_to_be32(vhandle);
+
+ return 0;
+
+out_err:
+ tpm2_flush_space(chip);
+ return rc;
+}
+
+static int tpm2_save_space(struct tpm_chip *chip)
+{
+ struct tpm_space *space = &chip->work_space;
+ int i;
+ int j;
+ int rc;
+ u32 s;
+
+ for (i = 0, j = 0; i < ARRAY_SIZE(space->context_tbl); i++) {
+ if (!(space->context_tbl[i] && ~space->context_tbl[i]))
+ continue;
+
+ tpm_buf_init(&chip->tr_buf, TPM2_ST_NO_SESSIONS,
+ TPM2_CC_CONTEXT_SAVE);
+ tpm_buf_append_u32(&chip->tr_buf, space->context_tbl[i]);
+
+ rc = tpm_transmit_cmd(chip, NULL, chip->tr_buf.data, PAGE_SIZE,
+ TPM_TRANSMIT_UNLOCKED, NULL);
+ if ((rc & TPM2_RC_HANDLE) == TPM2_RC_HANDLE) {
+ space->context_tbl[i] = 0;
+ continue;
+ } else if (rc) {
+ dev_warn(&chip->dev, "%s: saving failed with %d\n",
+ __func__, rc);
+ rc = -EFAULT;
+ goto out_err;
+ }
+
+ s = tpm_buf_length(&chip->tr_buf) - TPM_HEADER_SIZE;
+ if ((j + s) > PAGE_SIZE) {
+ dev_warn(&chip->dev, "%s: out of backing storage\n",
+ __func__);
+ rc = -ENOMEM;
+ goto out_err;
+ }
+
+ memcpy(&space->context_buf[j],
+ &chip->tr_buf.data[TPM_HEADER_SIZE], s);
+
+ tpm2_flush_context_cmd(chip, space->context_tbl[i],
+ TPM_TRANSMIT_UNLOCKED);
+
+ space->context_tbl[i] = ~0;
+
+ j += s;
+ }
+
+ return 0;
+
+out_err:
+ tpm2_flush_space(chip);
+ return rc;
+}
+
+int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
+ u32 cc, u8 *buf, size_t bufsiz)
+{
+ int rc;
+
+ if (!space)
+ return 0;
+
+ rc = tpm2_map_response(chip, cc, buf, bufsiz);
+ if (rc)
+ return rc;
+
+ rc = tpm2_save_space(chip);
+ if (rc)
+ return rc;
+
+ memcpy(&space->context_tbl, &chip->work_space.context_tbl,
+ sizeof(space->context_tbl));
+ memcpy(space->context_buf, chip->work_space.context_buf, PAGE_SIZE);
+
+ return 0;
+}
diff --git a/include/uapi/linux/tpm.h b/include/uapi/linux/tpm.h
new file mode 100644
index 0000000..1df5b61
--- /dev/null
+++ b/include/uapi/linux/tpm.h
@@ -0,0 +1,23 @@
+/*
+ * API and definitions for the TPM device driver
+ * Copyright (C) 2016 Intel Corporation
+ *
+ * Authors:
+ * Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ */
+
+#ifndef _UAPI_TPM_H
+#define _UAPI_TPM_H
+
+#include <linux/types.h>
+#include <linux/ioctl.h>
+
+#define TPM_IOC_MAGIC 0xa2
+#define TPM_IOC_NEW_SPACE _IO(TPM_IOC_MAGIC, 0x00)
+
+#endif /* _UAPI_TPM_H */
--
2.9.3
[toc] | [prev] | [next] | [standalone]
| From | Jason Gunthorpe <jgunthorpe@obsidianresearch.com> |
|---|---|
| Date | 2017-01-02 22:20 +0100 |
| Subject | Re: [PATCH RFC 4/4] tpm: add the infrastructure for TPM space for TPM 2.0 |
| Message-ID | <sVibo-4qO-17@gated-at.bofh.it> |
| In reply to | #1549183 |
On Mon, Jan 02, 2017 at 03:22:10PM +0200, Jarkko Sakkinen wrote: > Added a ioctl for creating a TPM space. The space is isolated from the > other users of the TPM. Only a process holding the file with the handle > can access the objects and only objects that are created through that > file handle can be accessed. I don't understand this comment. /dev/tpmX is forced to be single-process-open, so how can there ever be more than 1 FD for it? Since the space is tied to that single fd these patches just create a way for the single user-space process to auto-cleanup if it crashes? Is that the entire intent of this design? I guess it is OK as a stepping point.. > -ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz, > - unsigned int flags) > +ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space, > + u8 *buf, size_t bufsiz, unsigned int flags) Please split this patch so that 'struct tpm_space' introduction is in its own patch and the actual UAPI change is in a much smaller patch. It is very hard to see the uapi stuff in all of this churn. Jason
[toc] | [prev] | [next] | [standalone]
| From | Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> |
|---|---|
| Date | 2017-01-03 05:10 +0100 |
| Subject | Re: [PATCH RFC 4/4] tpm: add the infrastructure for TPM space for TPM 2.0 |
| Message-ID | <sVoAa-v7-3@gated-at.bofh.it> |
| In reply to | #1549400 |
On Mon, Jan 02, 2017 at 02:09:53PM -0700, Jason Gunthorpe wrote: > On Mon, Jan 02, 2017 at 03:22:10PM +0200, Jarkko Sakkinen wrote: > > Added a ioctl for creating a TPM space. The space is isolated from the > > other users of the TPM. Only a process holding the file with the handle > > can access the objects and only objects that are created through that > > file handle can be accessed. > > I don't understand this comment. /dev/tpmX is forced to be > single-process-open, so how can there ever be more than 1 FD for it? > > Since the space is tied to that single fd these patches just create a > way for the single user-space process to auto-cleanup if it crashes? > > Is that the entire intent of this design? I guess it is OK as a > stepping point.. is_open is cleared in tpm_ioc_new_space. /Jarkko
[toc] | [prev] | [next] | [standalone]
| From | Jason Gunthorpe <jgunthorpe@obsidianresearch.com> |
|---|---|
| Date | 2017-01-03 19:50 +0100 |
| Subject | Re: [PATCH RFC 4/4] tpm: add the infrastructure for TPM space for TPM 2.0 |
| Message-ID | <sVCjL-1Wi-13@gated-at.bofh.it> |
| In reply to | #1549525 |
On Tue, Jan 03, 2017 at 02:37:30AM +0200, Jarkko Sakkinen wrote: > On Mon, Jan 02, 2017 at 02:09:53PM -0700, Jason Gunthorpe wrote: > > On Mon, Jan 02, 2017 at 03:22:10PM +0200, Jarkko Sakkinen wrote: > > > Added a ioctl for creating a TPM space. The space is isolated from the > > > other users of the TPM. Only a process holding the file with the handle > > > can access the objects and only objects that are created through that > > > file handle can be accessed. > > > > I don't understand this comment. /dev/tpmX is forced to be > > single-process-open, so how can there ever be more than 1 FD for it? > > > > Since the space is tied to that single fd these patches just create a > > way for the single user-space process to auto-cleanup if it crashes? > > > > Is that the entire intent of this design? I guess it is OK as a > > stepping point.. > > is_open is cleared in tpm_ioc_new_space. That is no good, it is racy if the intention is to use multiple clients, and any single client that doesn't support the new API blocks all access. Jason
[toc] | [prev] | [next] | [standalone]
| From | Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> |
|---|---|
| Date | 2017-01-04 13:50 +0100 |
| Subject | Re: [PATCH RFC 4/4] tpm: add the infrastructure for TPM space for TPM 2.0 |
| Message-ID | <sVTaW-4JG-7@gated-at.bofh.it> |
| In reply to | #1550086 |
On Tue, Jan 03, 2017 at 11:46:27AM -0700, Jason Gunthorpe wrote: > On Tue, Jan 03, 2017 at 02:37:30AM +0200, Jarkko Sakkinen wrote: > > On Mon, Jan 02, 2017 at 02:09:53PM -0700, Jason Gunthorpe wrote: > > > On Mon, Jan 02, 2017 at 03:22:10PM +0200, Jarkko Sakkinen wrote: > > > > Added a ioctl for creating a TPM space. The space is isolated from the > > > > other users of the TPM. Only a process holding the file with the handle > > > > can access the objects and only objects that are created through that > > > > file handle can be accessed. > > > > > > I don't understand this comment. /dev/tpmX is forced to be > > > single-process-open, so how can there ever be more than 1 FD for it? > > > > > > Since the space is tied to that single fd these patches just create a > > > way for the single user-space process to auto-cleanup if it crashes? > > > > > > Is that the entire intent of this design? I guess it is OK as a > > > stepping point.. > > > > is_open is cleared in tpm_ioc_new_space. > > That is no good, it is racy if the intention is to use multiple > clients, and any single client that doesn't support the new API blocks > all access. Luckily this will be implicitly fixed with a separate device file in the non-RFC patch set :-) > Jason /Jarkko
[toc] | [prev] | [next] | [standalone]
| From | Jason Gunthorpe <jgunthorpe@obsidianresearch.com> |
|---|---|
| Date | 2017-01-03 20:20 +0100 |
| Subject | Re: [PATCH RFC 4/4] tpm: add the infrastructure for TPM space for TPM 2.0 |
| Message-ID | <sVCMO-2nX-31@gated-at.bofh.it> |
| In reply to | #1549525 |
On Tue, Jan 03, 2017 at 02:37:30AM +0200, Jarkko Sakkinen wrote: > On Mon, Jan 02, 2017 at 02:09:53PM -0700, Jason Gunthorpe wrote: > > On Mon, Jan 02, 2017 at 03:22:10PM +0200, Jarkko Sakkinen wrote: > > > Added a ioctl for creating a TPM space. The space is isolated from the > > > other users of the TPM. Only a process holding the file with the handle > > > can access the objects and only objects that are created through that > > > file handle can be accessed. > > > > I don't understand this comment. /dev/tpmX is forced to be > > single-process-open, so how can there ever be more than 1 FD for it? > > > > Since the space is tied to that single fd these patches just create a > > way for the single user-space process to auto-cleanup if it crashes? > > > > Is that the entire intent of this design? I guess it is OK as a > > stepping point.. > > is_open is cleared in tpm_ioc_new_space. There is also a bug with the uncondtional clear of is_open in tpm_release - this cannot happen if the ioctl is done - but I think this approach of using an ioctl is not a good idea. I have pondered using an open flag in the past - what about using something like O_EXCL to indicate that the fd is to be used in resource sharing mode? Not sure if that would be considered abuse of the open flags or not. Jason
[toc] | [prev] | [next] | [standalone]
| From | Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> |
|---|---|
| Date | 2017-01-04 13:50 +0100 |
| Subject | Re: [PATCH RFC 4/4] tpm: add the infrastructure for TPM space for TPM 2.0 |
| Message-ID | <sVTaW-4JG-5@gated-at.bofh.it> |
| In reply to | #1550112 |
On Tue, Jan 03, 2017 at 12:16:34PM -0700, Jason Gunthorpe wrote: > On Tue, Jan 03, 2017 at 02:37:30AM +0200, Jarkko Sakkinen wrote: > > On Mon, Jan 02, 2017 at 02:09:53PM -0700, Jason Gunthorpe wrote: > > > On Mon, Jan 02, 2017 at 03:22:10PM +0200, Jarkko Sakkinen wrote: > > > > Added a ioctl for creating a TPM space. The space is isolated from the > > > > other users of the TPM. Only a process holding the file with the handle > > > > can access the objects and only objects that are created through that > > > > file handle can be accessed. > > > > > > I don't understand this comment. /dev/tpmX is forced to be > > > single-process-open, so how can there ever be more than 1 FD for it? > > > > > > Since the space is tied to that single fd these patches just create a > > > way for the single user-space process to auto-cleanup if it crashes? > > > > > > Is that the entire intent of this design? I guess it is OK as a > > > stepping point.. > > > > is_open is cleared in tpm_ioc_new_space. > > There is also a bug with the uncondtional clear of is_open in > tpm_release - this cannot happen if the ioctl is done - but I think > this approach of using an ioctl is not a good idea. > > I have pondered using an open flag in the past - what about using > something like O_EXCL to indicate that the fd is to be used in > resource sharing mode? Not sure if that would be considered abuse of > the open flags or not. I've now leaned toward James' idea of having a separate /dev/tpms0. > Jason /Jarkko
[toc] | [prev] | [next] | [standalone]
| From | Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> |
|---|---|
| Date | 2017-01-09 23:20 +0100 |
| Subject | Re: [tpmdd-devel] [PATCH RFC 4/4] tpm: add the infrastructure for TPM space for TPM 2.0 |
| Message-ID | <sXQsi-1ot-3@gated-at.bofh.it> |
| In reply to | #1549183 |
On Wed, Jan 04, 2017 at 12:50:21PM -0500, Stefan Berger wrote:
> Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote on 01/02/2017
> 08:22:10 AM:
>
> >
> > Added a ioctl for creating a TPM space. The space is isolated from the
> > other users of the TPM. Only a process holding the file with the handle
> > can access the objects and only objects that are created through that
> > file handle can be accessed.
> >
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > ---
>
> > diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
> > index 912ad30..139638b 100644
> > --- a/drivers/char/tpm/tpm-dev.c
> > +++ b/drivers/char/tpm/tpm-dev.c
> > @@ -19,6 +19,7 @@
> > */
> > #include <linux/slab.h>
> > #include <linux/uaccess.h>
> > +#include <uapi/linux/tpm.h>
> > #include "tpm.h"
> >
> > struct file_priv {
> > @@ -32,6 +33,8 @@ struct file_priv {
> > struct work_struct work;
> >
> > u8 data_buffer[TPM_BUFSIZE];
> > + struct tpm_space space;
> > + bool has_space;
> > };
> >
> > static void user_reader_timeout(unsigned long ptr)
> > @@ -115,6 +118,7 @@ static ssize_t tpm_write(struct file *file,
> > const char __user *buf,
> > size_t size, loff_t *off)
> > {
> > struct file_priv *priv = file->private_data;
> > + struct tpm_space *space = NULL;
> > size_t in_size = size;
> > ssize_t out_size;
> >
> > @@ -130,6 +134,9 @@ static ssize_t tpm_write(struct file *file,
> > const char __user *buf,
> >
> > mutex_lock(&priv->buffer_mutex);
> >
> > + if (priv->has_space)
> > + space = &priv->space;
> > +
> > if (copy_from_user
> > (priv->data_buffer, (void __user *) buf, in_size)) {
> > mutex_unlock(&priv->buffer_mutex);
> > @@ -144,7 +151,7 @@ static ssize_t tpm_write(struct file *file,
> > const char __user *buf,
> > mutex_unlock(&priv->buffer_mutex);
> > return -EPIPE;
> > }
> > - out_size = tpm_transmit(priv->chip, priv->data_buffer,
> > + out_size = tpm_transmit(priv->chip, space, priv->data_buffer,
> > sizeof(priv->data_buffer), 0);
> >
> > tpm_put_ops(priv->chip);
> > @@ -162,6 +169,65 @@ static ssize_t tpm_write(struct file *file,
> > const char __user *buf,
> > return in_size;
> > }
> >
> > +/**
> > + * tpm_ioc_new_space - handler for %SGX_IOC_NEW_SPACE ioctl
> > + *
> > + * Creates a new TPM space that can hold a set of transient
> > objects. The space
> > + * is isolated with virtual handles that are mapped into physical
> > handles by the
> > + * driver.
> > + */
> > +static long tpm_ioc_new_space(struct file *file, unsigned int ioctl,
> > + unsigned long arg)
> > +{
> > + struct file_priv *priv = file->private_data;
> > + struct tpm_chip *chip = priv->chip;
> > + int rc = 0;
> > +
> > + if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> > + return -EOPNOTSUPP;
> > +
> > + mutex_lock(&priv->buffer_mutex);
> > +
> > + if (priv->has_space) {
> > + rc = -EBUSY;
> > + goto out;
> > + }
> > +
> > + priv->space.context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
> > + if (!priv->space.context_buf) {
> > + rc = -ENOMEM;
> > + goto out;
> > + }
> > +
> > + /* The TPM device can be opened again as this file has been moved to
> a
> > + * TPM handle space.
> > + */
> > + priv->has_space = true;
> > + clear_bit(0, &chip->is_open);
> > +out:
> > + mutex_unlock(&priv->buffer_mutex);
> > + return rc;
> > +}
> > +
> > +static long tpm_ioctl(struct file *file, unsigned int ioctl,
> > + unsigned long arg)
> > +{
> > + switch (ioctl) {
> > + case TPM_IOC_NEW_SPACE:
> > + return tpm_ioc_new_space(file, ioctl, arg);
> > + default:
> > + return -ENOIOCTLCMD;
> > + }
> > +}
> > +
> > +#ifdef CONFIG_COMPAT
> > +static long tpm_compat_ioctl(struct file *file, unsigned int ioctl,
> > + unsigned long arg)
> > +{
> > + return tpm_ioctl(file, ioctl, arg);
> > +}
> > +#endif
> > +
> > /*
> > * Called on file close
> > */
> > @@ -169,6 +235,14 @@ static int tpm_release(struct inode *inode,
> > struct file *file)
> > {
> > struct file_priv *priv = file->private_data;
> >
> > + if (tpm_try_get_ops(priv->chip)) {
> > + mutex_unlock(&priv->buffer_mutex);
> > + return -EPIPE;
> > + }
>
> That mutex_unlock looks wrong.
Thanks.
This will be anyway gone with own device file.
> Stefan
/Jarkko
[toc] | [prev] | [next] | [standalone]
| From | Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> |
|---|---|
| Date | 2017-01-02 14:30 +0100 |
| Subject | [PATCH RFC 1/4] tpm: migrate struct tpm_buf to struct tpm_chip |
| Message-ID | <sVaQy-7mn-27@gated-at.bofh.it> |
| In reply to | #1549178 |
Since there is only one thread using TPM chip at a time to transmit data
we can migrate struct tpm_buf to struct tpm_chip. This makes the use of
it more fail safe as the buffer is allocated from heap when the device
is created and not for every transaction.
This is needed characteristic for the resource manager so that we can
minimize the probability of failure for loading, saving and flushings
of TPM contexts.
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
drivers/char/tpm/tpm-chip.c | 8 +++
drivers/char/tpm/tpm.h | 67 ++++++++++-------------
drivers/char/tpm/tpm2-cmd.c | 128 ++++++++++++++++++--------------------------
3 files changed, 87 insertions(+), 116 deletions(-)
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index eefdc80..41e518e 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -128,6 +128,7 @@ static void tpm_dev_release(struct device *dev)
mutex_unlock(&idr_lock);
kfree(chip->log.bios_event_log);
+ kfree(chip->tr_buf.data);
kfree(chip);
}
@@ -189,6 +190,13 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
chip->cdev.owner = THIS_MODULE;
chip->cdev.kobj.parent = &chip->dev.kobj;
+ chip->tr_buf.data = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!chip->tr_buf.data) {
+ rc = -ENOMEM;
+ goto out;
+ }
+ chip->tr_buf.size = TPM_BUFSIZE;
+
return chip;
out:
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 1ae9768..c74a663 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -153,6 +153,20 @@ struct tpm_chip_seqops {
const struct seq_operations *seqops;
};
+/* A string buffer pointer type for constructing TPM commands. Does not own
+ * the data.
+ */
+
+enum tpm_buf_flags {
+ TPM_BUF_OVERFLOW = BIT(0),
+};
+
+struct tpm_buf {
+ unsigned int flags;
+ u8 *data;
+ unsigned int size;
+};
+
struct tpm_chip {
struct device dev;
struct cdev cdev;
@@ -191,6 +205,8 @@ struct tpm_chip {
acpi_handle acpi_dev_handle;
char ppi_version[TPM_PPI_VERSION_LEN + 1];
#endif /* CONFIG_ACPI */
+
+ struct tpm_buf tr_buf;
};
#define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
@@ -387,57 +403,28 @@ struct tpm_cmd_t {
tpm_cmd_params params;
} __packed;
-/* A string buffer type for constructing TPM commands. This is based on the
- * ideas of string buffer code in security/keys/trusted.h but is heap based
- * in order to keep the stack usage minimal.
- */
-
-enum tpm_buf_flags {
- TPM_BUF_OVERFLOW = BIT(0),
-};
+/* A string buffer for constructing TPM commands. */
-struct tpm_buf {
- struct page *data_page;
- unsigned int flags;
- u8 *data;
-};
-
-static inline int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
+static inline void tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
{
- struct tpm_input_header *head;
-
- buf->data_page = alloc_page(GFP_HIGHUSER);
- if (!buf->data_page)
- return -ENOMEM;
-
- buf->flags = 0;
- buf->data = kmap(buf->data_page);
-
- head = (struct tpm_input_header *) buf->data;
+ struct tpm_input_header *head = (struct tpm_input_header *)buf->data;
+ buf->flags &= ~TPM_BUF_OVERFLOW;
head->tag = cpu_to_be16(tag);
head->length = cpu_to_be32(sizeof(*head));
head->ordinal = cpu_to_be32(ordinal);
-
- return 0;
-}
-
-static inline void tpm_buf_destroy(struct tpm_buf *buf)
-{
- kunmap(buf->data_page);
- __free_page(buf->data_page);
}
static inline u32 tpm_buf_length(struct tpm_buf *buf)
{
- struct tpm_input_header *head = (struct tpm_input_header *) buf->data;
+ struct tpm_input_header *head = (struct tpm_input_header *)buf->data;
return be32_to_cpu(head->length);
}
static inline u16 tpm_buf_tag(struct tpm_buf *buf)
{
- struct tpm_input_header *head = (struct tpm_input_header *) buf->data;
+ struct tpm_input_header *head = (struct tpm_input_header *)buf->data;
return be16_to_cpu(head->tag);
}
@@ -446,14 +433,16 @@ static inline void tpm_buf_append(struct tpm_buf *buf,
const unsigned char *new_data,
unsigned int new_len)
{
- struct tpm_input_header *head = (struct tpm_input_header *) buf->data;
+
+ struct tpm_input_header *head = (struct tpm_input_header *)buf->data;
+
u32 len = tpm_buf_length(buf);
/* Return silently if overflow has already happened. */
if (buf->flags & TPM_BUF_OVERFLOW)
return;
- if ((len + new_len) > PAGE_SIZE) {
+ if ((len + new_len) > buf->size) {
WARN(1, "tpm_buf: overflow\n");
buf->flags |= TPM_BUF_OVERFLOW;
return;
@@ -472,14 +461,14 @@ static inline void tpm_buf_append_u16(struct tpm_buf *buf, const u16 value)
{
__be16 value2 = cpu_to_be16(value);
- tpm_buf_append(buf, (u8 *) &value2, 2);
+ tpm_buf_append(buf, (u8 *)&value2, 2);
}
static inline void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
{
__be32 value2 = cpu_to_be32(value);
- tpm_buf_append(buf, (u8 *) &value2, 4);
+ tpm_buf_append(buf, (u8 *)&value2, 4);
}
extern struct class *tpm_class;
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 6eda239..f0e0807 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -444,7 +444,6 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
struct trusted_key_options *options)
{
unsigned int blob_len;
- struct tpm_buf buf;
u32 hash;
int i;
int rc;
@@ -459,73 +458,67 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
if (i == ARRAY_SIZE(tpm2_hash_map))
return -EINVAL;
- rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
- if (rc)
- return rc;
+ tpm_buf_init(&chip->tr_buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
- tpm_buf_append_u32(&buf, options->keyhandle);
- tpm2_buf_append_auth(&buf, TPM2_RS_PW,
+ tpm_buf_append_u32(&chip->tr_buf, options->keyhandle);
+ tpm2_buf_append_auth(&chip->tr_buf, TPM2_RS_PW,
NULL /* nonce */, 0,
0 /* session_attributes */,
options->keyauth /* hmac */,
TPM_DIGEST_SIZE);
/* sensitive */
- tpm_buf_append_u16(&buf, 4 + TPM_DIGEST_SIZE + payload->key_len + 1);
+ tpm_buf_append_u16(&chip->tr_buf,
+ 4 + TPM_DIGEST_SIZE + payload->key_len + 1);
- tpm_buf_append_u16(&buf, TPM_DIGEST_SIZE);
- tpm_buf_append(&buf, options->blobauth, TPM_DIGEST_SIZE);
- tpm_buf_append_u16(&buf, payload->key_len + 1);
- tpm_buf_append(&buf, payload->key, payload->key_len);
- tpm_buf_append_u8(&buf, payload->migratable);
+ tpm_buf_append_u16(&chip->tr_buf, TPM_DIGEST_SIZE);
+ tpm_buf_append(&chip->tr_buf, options->blobauth, TPM_DIGEST_SIZE);
+ tpm_buf_append_u16(&chip->tr_buf, payload->key_len + 1);
+ tpm_buf_append(&chip->tr_buf, payload->key, payload->key_len);
+ tpm_buf_append_u8(&chip->tr_buf, payload->migratable);
/* public */
- tpm_buf_append_u16(&buf, 14 + options->policydigest_len);
- tpm_buf_append_u16(&buf, TPM2_ALG_KEYEDHASH);
- tpm_buf_append_u16(&buf, hash);
+ tpm_buf_append_u16(&chip->tr_buf, 14 + options->policydigest_len);
+ tpm_buf_append_u16(&chip->tr_buf, TPM2_ALG_KEYEDHASH);
+ tpm_buf_append_u16(&chip->tr_buf, hash);
/* policy */
if (options->policydigest_len) {
- tpm_buf_append_u32(&buf, 0);
- tpm_buf_append_u16(&buf, options->policydigest_len);
- tpm_buf_append(&buf, options->policydigest,
+ tpm_buf_append_u32(&chip->tr_buf, 0);
+ tpm_buf_append_u16(&chip->tr_buf, options->policydigest_len);
+ tpm_buf_append(&chip->tr_buf, options->policydigest,
options->policydigest_len);
} else {
- tpm_buf_append_u32(&buf, TPM2_OA_USER_WITH_AUTH);
- tpm_buf_append_u16(&buf, 0);
+ tpm_buf_append_u32(&chip->tr_buf, TPM2_OA_USER_WITH_AUTH);
+ tpm_buf_append_u16(&chip->tr_buf, 0);
}
/* public parameters */
- tpm_buf_append_u16(&buf, TPM2_ALG_NULL);
- tpm_buf_append_u16(&buf, 0);
+ tpm_buf_append_u16(&chip->tr_buf, TPM2_ALG_NULL);
+ tpm_buf_append_u16(&chip->tr_buf, 0);
/* outside info */
- tpm_buf_append_u16(&buf, 0);
+ tpm_buf_append_u16(&chip->tr_buf, 0);
/* creation PCR */
- tpm_buf_append_u32(&buf, 0);
+ tpm_buf_append_u32(&chip->tr_buf, 0);
- if (buf.flags & TPM_BUF_OVERFLOW) {
- rc = -E2BIG;
- goto out;
- }
+ if (chip->tr_buf.flags & TPM_BUF_OVERFLOW)
+ return -E2BIG;
- rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 0, "sealing data");
+ rc = tpm_transmit_cmd(chip, &chip->tr_buf, TPM_BUFSIZE, 0,
+ "sealing data");
if (rc)
- goto out;
+ return rc;
- blob_len = be32_to_cpup((__be32 *) &buf.data[TPM_HEADER_SIZE]);
- if (blob_len > MAX_BLOB_SIZE) {
- rc = -E2BIG;
- goto out;
- }
+ blob_len = be32_to_cpup((__be32 *)&chip->tr_buf.data[TPM_HEADER_SIZE]);
+ if (blob_len > MAX_BLOB_SIZE)
+ return -E2BIG;
- memcpy(payload->blob, &buf.data[TPM_HEADER_SIZE + 4], blob_len);
+ memcpy(payload->blob, &chip->tr_buf.data[TPM_HEADER_SIZE + 4],
+ blob_len);
payload->blob_len = blob_len;
-out:
- tpm_buf_destroy(&buf);
-
if (rc > 0) {
if ((rc & TPM2_RC_HASH) == TPM2_RC_HASH)
rc = -EINVAL;
@@ -555,7 +548,6 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
struct trusted_key_options *options,
u32 *blob_handle, unsigned int flags)
{
- struct tpm_buf buf;
unsigned int private_len;
unsigned int public_len;
unsigned int blob_len;
@@ -570,31 +562,25 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
if (blob_len > payload->blob_len)
return -E2BIG;
- rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
- if (rc)
- return rc;
+ tpm_buf_init(&chip->tr_buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
- tpm_buf_append_u32(&buf, options->keyhandle);
- tpm2_buf_append_auth(&buf, TPM2_RS_PW,
+ tpm_buf_append_u32(&chip->tr_buf, options->keyhandle);
+ tpm2_buf_append_auth(&chip->tr_buf, TPM2_RS_PW,
NULL /* nonce */, 0,
0 /* session_attributes */,
options->keyauth /* hmac */,
TPM_DIGEST_SIZE);
- tpm_buf_append(&buf, payload->blob, blob_len);
+ tpm_buf_append(&chip->tr_buf, payload->blob, blob_len);
- if (buf.flags & TPM_BUF_OVERFLOW) {
- rc = -E2BIG;
- goto out;
- }
+ if (chip->tr_buf.flags & TPM_BUF_OVERFLOW)
+ return -E2BIG;
- rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, flags, "loading blob");
+ rc = tpm_transmit_cmd(chip, &chip->tr_buf, TPM_BUFSIZE, flags,
+ "loading blob");
if (!rc)
*blob_handle = be32_to_cpup(
- (__be32 *) &buf.data[TPM_HEADER_SIZE]);
-
-out:
- tpm_buf_destroy(&buf);
+ (__be32 *)&chip->tr_buf.data[TPM_HEADER_SIZE]);
if (rc > 0)
rc = -EPERM;
@@ -614,25 +600,16 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
static void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
unsigned int flags)
{
- struct tpm_buf buf;
int rc;
- rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
- if (rc) {
- dev_warn(&chip->dev, "0x%08x was not flushed, out of memory\n",
- handle);
- return;
- }
-
- tpm_buf_append_u32(&buf, handle);
+ tpm_buf_init(&chip->tr_buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
+ tpm_buf_append_u32(&chip->tr_buf, handle);
- rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, flags,
+ rc = tpm_transmit_cmd(chip, &chip->tr_buf, TPM_BUFSIZE, flags,
"flushing context");
if (rc)
dev_warn(&chip->dev, "0x%08x was not flushed, rc=%d\n", handle,
rc);
-
- tpm_buf_destroy(&buf);
}
/**
@@ -653,17 +630,14 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
struct trusted_key_options *options,
u32 blob_handle, unsigned int flags)
{
- struct tpm_buf buf;
u16 data_len;
u8 *data;
int rc;
- rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
- if (rc)
- return rc;
+ tpm_buf_init(&chip->tr_buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
- tpm_buf_append_u32(&buf, blob_handle);
- tpm2_buf_append_auth(&buf,
+ tpm_buf_append_u32(&chip->tr_buf, blob_handle);
+ tpm2_buf_append_auth(&chip->tr_buf,
options->policyhandle ?
options->policyhandle : TPM2_RS_PW,
NULL /* nonce */, 0,
@@ -671,21 +645,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, chip->tr_buf.data, TPM_BUFSIZE, flags,
+ "unsealing");
if (rc > 0)
rc = -EPERM;
if (!rc) {
data_len = be16_to_cpup(
- (__be16 *) &buf.data[TPM_HEADER_SIZE + 4]);
- data = &buf.data[TPM_HEADER_SIZE + 6];
+ (__be16 *)&chip->tr_buf.data[TPM_HEADER_SIZE + 4]);
+ data = &chip->tr_buf.data[TPM_HEADER_SIZE + 6];
memcpy(payload->key, data, data_len - 1);
payload->key_len = data_len - 1;
payload->migratable = data[data_len - 1];
}
- tpm_buf_destroy(&buf);
return rc;
}
--
2.9.3
[toc] | [prev] | [next] | [standalone]
| From | Jason Gunthorpe <jgunthorpe@obsidianresearch.com> |
|---|---|
| Date | 2017-01-02 22:10 +0100 |
| Subject | Re: [PATCH RFC 1/4] tpm: migrate struct tpm_buf to struct tpm_chip |
| Message-ID | <sVi1I-4ma-29@gated-at.bofh.it> |
| In reply to | #1549184 |
On Mon, Jan 02, 2017 at 03:22:07PM +0200, Jarkko Sakkinen wrote: > Since there is only one thread using TPM chip at a time to transmit data > we can migrate struct tpm_buf to struct tpm_chip. This makes the use of > it more fail safe as the buffer is allocated from heap when the device > is created and not for every transaction. Eh? What? I don't think that is the case.. We don't serialize until we hit tramsit_cmd at which point the buffer is already being used and cannot be shared between threads. Only /dev/tpmX has any sort of locking, but even that is designed to be optional (eg I patch it out of my kernels), and only covers userspace, not contention with in-kernel threads. Why would the resource manager need a single global tpm buffer? That seems like a big regression from where we have been going. I don't think this is a good idea to go down this road. > - tpm_buf_append(buf, (u8 *) &value2, 4); > + tpm_buf_append(buf, (u8 *)&value2, 4); Please try and avoid this sort of churn in patches that change things.. Jason
[toc] | [prev] | [next] | [standalone]
| From | Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> |
|---|---|
| Date | 2017-01-03 05:10 +0100 |
| Subject | Re: [PATCH RFC 1/4] tpm: migrate struct tpm_buf to struct tpm_chip |
| Message-ID | <sVoAa-v7-5@gated-at.bofh.it> |
| In reply to | #1549395 |
On Mon, Jan 02, 2017 at 02:01:01PM -0700, Jason Gunthorpe wrote: > On Mon, Jan 02, 2017 at 03:22:07PM +0200, Jarkko Sakkinen wrote: > > Since there is only one thread using TPM chip at a time to transmit data > > we can migrate struct tpm_buf to struct tpm_chip. This makes the use of > > it more fail safe as the buffer is allocated from heap when the device > > is created and not for every transaction. > > Eh? What? I don't think that is the case.. > > We don't serialize until we hit tramsit_cmd at which point the buffer > is already being used and cannot be shared between threads. There is a regression in the patch. All functions that use 'tr_buf' should take tpm_mutex first and use TPM_TRANSMIT_UNLOCKED. There's also a similar regression in TPM space patch that I have to correct. > Why would the resource manager need a single global tpm buffer? That > seems like a big regression from where we have been going. I don't > think this is a good idea to go down this road. What? 'tr_buf' is not specifically for resource manager. This commit makes creating TPM commands more fail-safe because there is no need to allocate page for every transmit. For RM decorations this is really important because I rather would have them fail as rarely as possible. If this would become a scalability issue then the granularity could be reconsidered. > > - tpm_buf_append(buf, (u8 *) &value2, 4); > > + tpm_buf_append(buf, (u8 *)&value2, 4); > > Please try and avoid this sort of churn in patches that change things.. It wasn't there on purpose. I do not know how these slipped. I can clean these up. > Jason /Jarkko
[toc] | [prev] | [next] | [standalone]
| From | Jason Gunthorpe <jgunthorpe@obsidianresearch.com> |
|---|---|
| Date | 2017-01-03 20:20 +0100 |
| Subject | Re: [PATCH RFC 1/4] tpm: migrate struct tpm_buf to struct tpm_chip |
| Message-ID | <sVCMO-2nX-13@gated-at.bofh.it> |
| In reply to | #1549524 |
On Tue, Jan 03, 2017 at 02:57:37AM +0200, Jarkko Sakkinen wrote: > On Mon, Jan 02, 2017 at 02:01:01PM -0700, Jason Gunthorpe wrote: > > On Mon, Jan 02, 2017 at 03:22:07PM +0200, Jarkko Sakkinen wrote: > > > Since there is only one thread using TPM chip at a time to transmit data > > > we can migrate struct tpm_buf to struct tpm_chip. This makes the use of > > > it more fail safe as the buffer is allocated from heap when the device > > > is created and not for every transaction. > > > > Eh? What? I don't think that is the case.. > > > > We don't serialize until we hit tramsit_cmd at which point the buffer > > is already being used and cannot be shared between threads. > > There is a regression in the patch. All functions that use 'tr_buf' > should take tpm_mutex first and use TPM_TRANSMIT_UNLOCKED. There's > also a similar regression in TPM space patch that I have to correct. No, you can't steal TPM_TRANSMIT_UNLOCKED and tpm_mutex for this, that is to allow a chain of commands to execute atomicly, so a new lock is needed just for the tr_buf. > > Why would the resource manager need a single global tpm buffer? That > > seems like a big regression from where we have been going. I don't > > think this is a good idea to go down this road. > > What? 'tr_buf' is not specifically for resource manager. This commit > makes creating TPM commands more fail-safe because there is no need > to allocate page for every transmit. That doesn't seem all that important, honestly. There kernel does not fail single page allocations without a lot of duress. > For RM decorations this is really important because I rather would have > them fail as rarely as possible. If this would become a scalability > issue then the granularity could be reconsidered. Why? The RM design already seems to have the prepare/commit/abort kind of model so it can already fail. What does it matter if the caller can fail before getting that far? It seems like alot of dangerous churn to introduce a new locking model without a really good reason... Jason
[toc] | [prev] | [next] | [standalone]
| From | Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> |
|---|---|
| Date | 2017-01-04 13:40 +0100 |
| Subject | Re: [PATCH RFC 1/4] tpm: migrate struct tpm_buf to struct tpm_chip |
| Message-ID | <sVT1g-4FT-35@gated-at.bofh.it> |
| In reply to | #1550113 |
On Tue, Jan 03, 2017 at 12:13:28PM -0700, Jason Gunthorpe wrote: > On Tue, Jan 03, 2017 at 02:57:37AM +0200, Jarkko Sakkinen wrote: > > On Mon, Jan 02, 2017 at 02:01:01PM -0700, Jason Gunthorpe wrote: > > > On Mon, Jan 02, 2017 at 03:22:07PM +0200, Jarkko Sakkinen wrote: > > > > Since there is only one thread using TPM chip at a time to transmit data > > > > we can migrate struct tpm_buf to struct tpm_chip. This makes the use of > > > > it more fail safe as the buffer is allocated from heap when the device > > > > is created and not for every transaction. > > > > > > Eh? What? I don't think that is the case.. > > > > > > We don't serialize until we hit tramsit_cmd at which point the buffer > > > is already being used and cannot be shared between threads. > > > > There is a regression in the patch. All functions that use 'tr_buf' > > should take tpm_mutex first and use TPM_TRANSMIT_UNLOCKED. There's > > also a similar regression in TPM space patch that I have to correct. > > No, you can't steal TPM_TRANSMIT_UNLOCKED and tpm_mutex for this, that > is to allow a chain of commands to execute atomicly, so a new lock is > needed just for the tr_buf. > > > > Why would the resource manager need a single global tpm buffer? That > > > seems like a big regression from where we have been going. I don't > > > think this is a good idea to go down this road. > > > > What? 'tr_buf' is not specifically for resource manager. This commit > > makes creating TPM commands more fail-safe because there is no need > > to allocate page for every transmit. > > That doesn't seem all that important, honestly. There kernel does not > fail single page allocations without a lot of duress. > > > For RM decorations this is really important because I rather would have > > them fail as rarely as possible. If this would become a scalability > > issue then the granularity could be reconsidered. > > Why? The RM design already seems to have the prepare/commit/abort > kind of model so it can already fail. What does it matter if the > caller can fail before getting that far? Yeah, I just noticed it :-) That kind of formed by accident when I experimented with various models of rolling back in an error situation. > It seems like alot of dangerous churn to introduce a new locking model > without a really good reason... OK, thanks for the feedback. I understad your arguments but as this was an RFC patch set I don't want to go more details like these but I take your advice seriously. I'll start preparing the first non-RFC version. I'm happy that the beef (i.e. the stuff in tpm2-space.c) has been well accepted! > Jason /Jarkko
[toc] | [prev] | [next] | [standalone]
| From | James Bottomley <James.Bottomley@HansenPartnership.com> |
|---|---|
| Date | 2017-01-02 17:40 +0100 |
| Subject | Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager |
| Message-ID | <sVdOp-19V-19@gated-at.bofh.it> |
| In reply to | #1549178 |
On Mon, 2017-01-02 at 15:22 +0200, Jarkko Sakkinen wrote: > This patch set adds support for TPM spaces that provide a context > for isolating and swapping transient objects. This patch set does > not yet include support for isolating policy and HMAC sessions but > it is trivial to add once the basic approach is settled (and that's > why I created an RFC patch set). The approach looks fine to me. The only basic query I have is about the default: shouldn't it be with resource manager on rather than off? I can't really think of a use case that wants the RM off (even if you're running your own, having another doesn't hurt anything, and it's still required to share with in-kernel uses). > There's a test script for trying out TPM spaces in > > git://git.infradead.org/users/jjs/tpm2-scripts.git > > A simple smoke test can be run by > > sudo python -m unittest -v tpm2_smoke.SpaceTest I've also added an enabling patch to the tss https://build.opensuse.org/package/view_file/home:jejb1:Tumbleweed/tss2/0002-tssProperties-add-TPM_USE_RESOURCE_MANAGER.patch?expand=1 And with that, I've TPM 2 enabled both gnome-keyring and openssl: https://build.opensuse.org/package/show/home:jejb1:Tumbleweed/gnome-keyring https://build.opensuse.org/package/show/home:jejb1:Tumbleweed/openssl_tpm_engine I'm running them in production on my day to day laptop and so far everything's working nicely (better than 1.2, in fact, since tcsd periodically crashes necessitating a restart of everything). So you can definitely add my Tested-By. James
[toc] | [prev] | [next] | [standalone]
| From | James Bottomley <James.Bottomley@HansenPartnership.com> |
|---|---|
| Date | 2017-01-02 22:50 +0100 |
| Subject | Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager |
| Message-ID | <sViEq-4AZ-5@gated-at.bofh.it> |
| In reply to | #1549286 |
On Mon, 2017-01-02 at 21:33 +0200, Jarkko Sakkinen wrote: > On Mon, Jan 02, 2017 at 08:36:20AM -0800, James Bottomley wrote: > > On Mon, 2017-01-02 at 15:22 +0200, Jarkko Sakkinen wrote: > > > This patch set adds support for TPM spaces that provide a context > > > for isolating and swapping transient objects. This patch set does > > > not yet include support for isolating policy and HMAC sessions > > > but it is trivial to add once the basic approach is settled (and > > > that's why I created an RFC patch set). > > > > The approach looks fine to me. The only basic query I have is > > about the default: shouldn't it be with resource manager on rather > > than off? I can't really think of a use case that wants the RM off > > (even if you're running your own, having another doesn't hurt > > anything, and it's still required to share with in-kernel uses). > > This is a valid question and here's a longish explanation. > > In TPM2_GetCapability and maybe couple of other commands you can get > handles in the response body. I do not want to have special cases in > the kernel for response bodies because there is no a generic way to > do the substitution. What's worse, new commands in the standard > future revisions could have such commands requiring special cases. In > addition, vendor specific commans could have handles in the response > bodies. OK, in general I buy this ... what you're effectively saying is that we need a non-RM interface for certain management type commands. However, let me expand a bit on why I'm fretting about the non-RM use case. Right at the moment, we have a single TPM device which you use for access to the kernel TPM. The current tss2 just makes direct use of this, meaning it has to have 0666 permissions. This means that any local user can simply DoS the TPM by running us out of transient resources if they don't activate the RM. If they get a connection always via the RM, this isn't a worry. Perhaps the best way of fixing this is to expose two separate device nodes: one raw to the TPM which we could keep at 0600 and one with an always RM connection which we can set to 0666. That would mean that access to the non-RM connection is either root only or governed by a system set ACL. James
[toc] | [prev] | [next] | [standalone]
Page 1 of 4 [1] 2 3 4 Next page →
Back to top | Article view | linux.kernel
csiph-web