Messages in this thread | | | Date | Mon, 16 Jan 2017 12:05:52 +0200 | From | Jarkko Sakkinen <> | Subject | Re: [PATCH] tpm: add session handles to the save and restore of the tpm2 space manager |
| |
On Mon, Jan 16, 2017 at 12:04:15PM +0200, Jarkko Sakkinen wrote: > On Fri, Jan 13, 2017 at 11:24:13AM -0800, James Bottomley wrote: > > Session handles are slightly more difficult to manage because any TPM > > only has a finite number of allowed handles, even if the session has > > been saved; so when you context save a session, you must not flush it > > because that would destroy the ability to context load it (you only > > flush sessions when you're done with them) and the tpm won't re-use > > the handle. Additionally, sessions can be flushed as part of the > > successful execution of a command if the continueSession attribute is > > clear, so we have to mark any session we find in the command (using > > TPM2_HT_TAG_FOR_FLUSH) so it can be cleared from the space if the > > command successfully executes. > > > > Finally, a session may also be cleared by flushing it, so we have to > > emulate the TPM2_FlushContext command to see if a session is being > > flushed and manually clear it from the space. > > > > We also fully flush all sessions on device close. > > Some big overview comments without going deeply into details. I will > use more time for this once the
... transient object swapping and /dev/tpms0 are in the shape :-)
/Jarkko
> > Please do not use handle allocation code for sessions. This commit > makes the implementation a mess. Just use the phandle directly and > have array of session phandles for each space. > > I would also almost require to have at minimum two patches: one that > implements purely isolation and another that implements swapping. > > It might be for example that I want to land TPM spaces with session > isolation to one release and swapping to n+1 because my hunch tells > me that it is better to bake the swapping part for a while. > > One more thing. Maybe commit messages should speak uniformally about > TPM spaces? They are a tool to implement resource manager, not a > resource manager. > > /Jarkko > > > > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> > > > > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c > > index f5c9355..d8e896e 100644 > > --- a/drivers/char/tpm/tpm-interface.c > > +++ b/drivers/char/tpm/tpm-interface.c > > @@ -400,6 +400,10 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space, > > if (chip->dev.parent) > > pm_runtime_get_sync(chip->dev.parent); > > > > + rc = tpm2_emulate(chip, space, ordinal, buf, bufsiz); > > + if (rc) > > + goto out; > > + > > rc = tpm2_prepare_space(chip, space, ordinal, buf, bufsiz); > > if (rc) > > goto out; > > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h > > index adf7810..b922652 100644 > > --- a/drivers/char/tpm/tpm.h > > +++ b/drivers/char/tpm/tpm.h > > @@ -136,7 +136,8 @@ enum tpm2_capabilities { > > }; > > > > enum tpm2_properties { > > - TPM_PT_TOTAL_COMMANDS = 0x0129, > > + TPM_PT_ACTIVE_SESSIONS_MAX = 0x0111, > > + TPM_PT_TOTAL_COMMANDS = 0x0129, > > }; > > > > enum tpm2_startup_types { > > @@ -214,6 +215,8 @@ struct tpm_chip { > > struct tpm_space *work_space; > > u32 nr_commands; > > u32 *cc_attrs_tbl; > > + struct tpm_sessions *sessions; > > + int max_sessions; > > }; > > > > #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev) > > @@ -583,4 +586,7 @@ 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); > > +void tpm2_flush_space(struct tpm_chip *chip, struct tpm_space *space); > > +int tpm2_emulate(struct tpm_chip *chip, struct tpm_space *space, > > + u32 cc, u8 *buf, size_t bufsiz); > > #endif > > diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c > > index 285361e..e8e9f32 100644 > > --- a/drivers/char/tpm/tpm2-space.c > > +++ b/drivers/char/tpm/tpm2-space.c > > @@ -25,15 +25,29 @@ enum tpm2_handle_types { > > TPM2_HT_TRANSIENT = 0x80000000, > > }; > > > > -static void tpm2_flush_space(struct tpm_chip *chip) > > +#define TPM2_HT_TAG_FOR_FLUSH 0xF0000000 > > + > > +void tpm2_flush_space(struct tpm_chip *chip, struct tpm_space *space) > > { > > - 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); > > + for (i = 0; i < ARRAY_SIZE(space->context_tbl); i++) { > > + u32 handle = space->context_tbl[i]; > > + u32 handle_type; > > + > > + if (handle == 0 || handle == ~0) > > + continue; > > + > > + if ((handle & TPM2_HT_TAG_FOR_FLUSH) == > > + TPM2_HT_TAG_FOR_FLUSH) > > + handle &= ~TPM2_HT_TAG_FOR_FLUSH; > > + > > + handle_type = (handle & 0xFF000000); > > + > > + tpm2_flush_context_cmd(chip, handle, TPM_TRANSMIT_UNLOCKED); > > + > > + space->context_tbl[i] = 0; > > + } > > } > > > > struct tpm2_context { > > @@ -54,15 +68,11 @@ static int tpm2_load_space(struct tpm_chip *chip) > > u32 s; > > > > for (i = 0, j = 0; i < ARRAY_SIZE(space->context_tbl); i++) { > > + u32 phandle, phandle_type; > > + > > 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; > > - } > > - > > rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, > > TPM2_CC_CONTEXT_LOAD); > > if (rc) > > @@ -80,9 +90,23 @@ static int tpm2_load_space(struct tpm_chip *chip) > > rc = -EFAULT; > > goto out_err; > > } > > + phandle = get_unaligned_be32((__be32 *)&buf.data[TPM_HEADER_SIZE]); > > + phandle_type = (phandle & 0xFF000000); > > + if (phandle_type == TPM2_HT_TRANSIENT && > > + space->context_tbl[i] != ~0) { > > + dev_err(&chip->dev, "context table is inconsistent"); > > + rc = -EFAULT; > > + goto out_err; > > + } > > + if ((phandle_type == TPM2_HT_HMAC_SESSION || > > + phandle_type == TPM2_HT_POLICY_SESSION) && > > + space->context_tbl[i] != phandle) { > > + dev_err(&chip->dev, "session handle changed\n"); > > + rc = -EFAULT; > > + goto out_err; > > + } > > > > - space->context_tbl[i] = > > - be32_to_cpup((__be32 *)&buf.data[TPM_HEADER_SIZE]); > > + space->context_tbl[i] = phandle; > > > > j += s; > > > > @@ -93,10 +117,85 @@ static int tpm2_load_space(struct tpm_chip *chip) > > > > out_err: > > tpm_buf_destroy(&buf); > > - tpm2_flush_space(chip); > > + tpm2_flush_space(chip, space); > > return rc; > > } > > > > +static void tpm2_unmap_sessions(struct tpm_chip *chip, u32 rc) > > +{ > > + struct tpm_space *space = chip->work_space; > > + int i; > > + > > + for (i = 0; i < ARRAY_SIZE(space->context_tbl); i++) { > > + if ((space->context_tbl[i] & TPM2_HT_TAG_FOR_FLUSH) != > > + TPM2_HT_TAG_FOR_FLUSH) > > + continue; > > + if (rc == TPM2_RC_SUCCESS) > > + space->context_tbl[i] = 0; > > + else > > + /* for unsuccessful command, keep session */ > > + space->context_tbl[i] &= ~TPM2_HT_TAG_FOR_FLUSH; > > + } > > +} > > + > > +static int tpm2_map_sessions(struct tpm_chip *chip, u8 *buf, size_t len, > > + size_t start) > > +{ > > + struct tpm_space *space = chip->work_space; > > + u32 size = be32_to_cpup((__be32 *)&buf[start]); > > + int i; > > + > > + /* skip over authorizationSize */ > > + start += 4; > > + > > + if (size > len - start) { > > + dev_err(&chip->dev, "Invalid authorization header size %u\n", > > + size); > > + return -EINVAL; > > + } > > + > > + for (i = start; i < start+size; ) { > > + u16 skip; > > + __be32 *handlep; > > + u8 attr; > > + int j; > > + u32 handle_type; > > + > > + /* TPMI_SH_AUTH_SESSION */ > > + handlep = (__be32 *)&buf[i]; > > + handle_type = get_unaligned_be32(handlep) & 0xFF000000; > > + i += 4; > > + /* TPM2B_DIGEST */ > > + skip = get_unaligned_be16((__be16 *)&buf[i]); > > + i += skip + sizeof(skip); > > + /* TPMA_SESSION */ > > + attr = buf[i++]; > > + /* TPM2B_AUTH */ > > + skip = get_unaligned_be16((__be16 *)&buf[i]); > > + i += skip + sizeof(skip); > > + > > + if (handle_type != TPM2_HT_HMAC_SESSION && > > + handle_type != TPM2_HT_POLICY_SESSION) > > + continue; > > + > > + j = 0xFFFFFF - (get_unaligned_be32(handlep) & 0xFFFFFF); > > + if (j > ARRAY_SIZE(space->context_tbl) || > > + !space->context_tbl[j]) > > + return -EINVAL; > > + put_unaligned_be32(space->context_tbl[j], handlep); > > + if ((attr & 1) == 0) > > + /* session is flushed by the command */ > > + space->context_tbl[j] |= TPM2_HT_TAG_FOR_FLUSH; > > + } > > + > > + if (i != start+size) { > > + dev_err(&chip->dev, "Authorization session overflow\n"); > > + return -EINVAL; > > + } > > + > > + return 0; > > +} > > + > > static int tpm2_map_command(struct tpm_chip *chip, u32 cc, u8 *cmd, size_t len) > > { > > struct tpm_space *space = chip->work_space; > > @@ -104,6 +203,7 @@ static int tpm2_map_command(struct tpm_chip *chip, u32 cc, u8 *cmd, size_t len) > > u32 vhandle; > > u32 phandle; > > u32 attrs; > > + u16 tag = get_unaligned_be16((__be16 *)cmd); > > int i; > > int j; > > int rc; > > @@ -131,11 +231,14 @@ static int tpm2_map_command(struct tpm_chip *chip, u32 cc, u8 *cmd, size_t len) > > *((__be32 *)&cmd[TPM_HEADER_SIZE + 4 * i]) = > > cpu_to_be32(phandle); > > } > > + if (tag == TPM2_ST_SESSIONS) > > + tpm2_map_sessions(chip, cmd, len, > > + TPM_HEADER_SIZE + 4*nr_handles); > > > > return 0; > > > > out_err: > > - tpm2_flush_space(chip); > > + tpm2_flush_space(chip, space); > > return rc; > > } > > > > @@ -163,13 +266,17 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, > > 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 phandle, phandle_type; > > u32 vhandle; > > u32 attrs; > > u32 return_code = get_unaligned_be32((__be32 *)&rsp[6]); > > + u16 tag = get_unaligned_be16((__be16 *)rsp); > > int i; > > int rc; > > > > + if (tag == TPM2_ST_SESSIONS) > > + tpm2_unmap_sessions(chip, return_code); > > + > > if (return_code != TPM2_RC_SUCCESS) > > return 0; > > > > @@ -185,7 +292,10 @@ static int tpm2_map_response(struct tpm_chip *chip, u32 cc, u8 *rsp, size_t len) > > return 0; > > > > phandle = be32_to_cpup((__be32 *)&rsp[TPM_HEADER_SIZE]); > > - if ((phandle & 0xFF000000) != TPM2_HT_TRANSIENT) > > + phandle_type = (phandle & 0xFF000000); > > + if (phandle_type != TPM2_HT_TRANSIENT && > > + phandle_type != TPM2_HT_HMAC_SESSION && > > + phandle_type != TPM2_HT_POLICY_SESSION) > > return 0; > > > > /* Garbage collect a dead context. */ > > @@ -208,13 +318,13 @@ static int tpm2_map_response(struct tpm_chip *chip, u32 cc, u8 *rsp, size_t len) > > } > > > > space->context_tbl[i] = phandle; > > - vhandle = TPM2_HT_TRANSIENT | (0xFFFFFF - i); > > + vhandle = phandle_type | (0xFFFFFF - i); > > *(__be32 *)&rsp[TPM_HEADER_SIZE] = cpu_to_be32(vhandle); > > > > return 0; > > > > out_err: > > - tpm2_flush_space(chip); > > + tpm2_flush_space(chip, space); > > return rc; > > } > > > > @@ -228,9 +338,20 @@ static int tpm2_save_space(struct tpm_chip *chip) > > u32 s; > > > > for (i = 0, j = 0; i < ARRAY_SIZE(space->context_tbl); i++) { > > - if (!(space->context_tbl[i] && ~space->context_tbl[i])) > > + u32 phandle, phandle_type; > > + > > + phandle = space->context_tbl[i]; > > + > > + if (phandle == 0) > > continue; > > > > + if (phandle == ~0) { > > + dev_err(&chip->dev, "context table is inconsistent\n"); > > + return -EFAULT; > > + } > > + > > + phandle_type = (phandle & 0xFF000000); > > + > > rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, > > TPM2_CC_CONTEXT_SAVE); > > if (rc) > > @@ -260,10 +381,11 @@ static int tpm2_save_space(struct tpm_chip *chip) > > > > memcpy(&space->context_buf[j], &buf.data[TPM_HEADER_SIZE], s); > > > > - tpm2_flush_context_cmd(chip, space->context_tbl[i], > > - TPM_TRANSMIT_UNLOCKED); > > - > > - space->context_tbl[i] = ~0; > > + if (phandle_type == TPM2_HT_TRANSIENT) { > > + tpm2_flush_context_cmd(chip, space->context_tbl[i], > > + TPM_TRANSMIT_UNLOCKED); > > + space->context_tbl[i] = ~0; > > + } > > > > j += s; > > > > @@ -273,7 +395,7 @@ static int tpm2_save_space(struct tpm_chip *chip) > > return 0; > > out_err: > > tpm_buf_destroy(&buf); > > - tpm2_flush_space(chip); > > + tpm2_flush_space(chip, space); > > return rc; > > } > > > > @@ -297,3 +419,60 @@ int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space, > > > > return 0; > > } > > + > > +/* if a space is active, emulate some commands */ > > +int tpm2_emulate(struct tpm_chip *chip, struct tpm_space *space, > > + u32 cc, u8 *buf, size_t bufsiz) > > +{ > > + int i, j, k; > > + u32 vhandle, handle_type, phandle; > > + struct tpm2_context *ctx; > > + static struct tpm_output_header buf_rc = { > > + .tag = cpu_to_be16(TPM2_ST_NO_SESSIONS), > > + .length = cpu_to_be32(sizeof(struct tpm_output_header)), > > + }; > > + > > + if (!space) > > + return 0; > > + > > + if (cc != TPM2_CC_FLUSH_CONTEXT) > > + return 0; > > + vhandle = get_unaligned_be32((__be32 *)&buf[10]); > > + handle_type = (vhandle & 0xFF000000); > > + > > + if (handle_type != TPM2_HT_HMAC_SESSION && > > + handle_type != TPM2_HT_POLICY_SESSION && > > + handle_type != TPM2_HT_TRANSIENT) > > + /* let the TPM figure out and return the error */ > > + return 0; > > + > > + buf_rc.return_code = TPM2_RC_HANDLE; > > + > > + j = 0xFFFFFF - (vhandle & 0xFFFFFF); > > + if (j > ARRAY_SIZE(space->context_tbl)) > > + goto error; > > + phandle = space->context_tbl[j]; > > + if (phandle != ~0) > > + goto error; > > + > > + for (i = 0, k = 0; i <= j; i++) { > > + ctx = (struct tpm2_context *)&space->context_buf[k]; > > + > > + if (space->context_tbl[i] == 0) > > + continue; > > + > > + k += sizeof(*ctx) + get_unaligned_be16(&ctx->blob_size); > > + } > > + /* move all the contexts up */ > > + memcpy(ctx, &space->context_buf[k], PAGE_SIZE - k); > > + space->context_tbl[j] = 0; > > + > > + if (handle_type != TPM2_HT_TRANSIENT) > > + tpm2_flush_context_cmd(chip, phandle, TPM_TRANSMIT_UNLOCKED); > > + > > + buf_rc.return_code = TPM2_RC_SUCCESS; > > + > > + error: > > + memcpy(buf, &buf_rc, sizeof(buf_rc)); > > + return sizeof(buf_rc); > > +} > > diff --git a/drivers/char/tpm/tpms-dev.c b/drivers/char/tpm/tpms-dev.c > > index c10b308..3eb5955 100644 > > --- a/drivers/char/tpm/tpms-dev.c > > +++ b/drivers/char/tpm/tpms-dev.c > > @@ -36,6 +36,7 @@ static int tpms_release(struct inode *inode, struct file *file) > > struct file_priv *fpriv = file->private_data; > > struct tpms_priv *priv = container_of(fpriv, struct tpms_priv, priv); > > > > + tpm2_flush_space(fpriv->chip, &priv->space); > > tpm_common_release(file, fpriv); > > kfree(priv); > > > >
| |