tpm: add session handles to the save and restore of the tpm2 space manager
diff mbox

Message ID 1484335453.2527.31.camel@linux.vnet.ibm.com
State New
Headers show

Commit Message

James Bottomley Jan. 13, 2017, 7:24 p.m. UTC
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.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>


--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Ken Goldman Jan. 13, 2017, 9:23 p.m. UTC | #1
On 1/13/2017 2:24 PM, 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

This is correct.

Background:  A save context for a session implicitly does flush the 
session (loaded session in TCG jargon).  However, it keeps some state
(saved session in TCG jargon) to provide anti-replay protection when
the session is later context loaded.

If you explicitly flushcontext a saved session, you're actually flushing 
that state.

> +static int tpm2_map_sessions(struct tpm_chip *chip, u8 *buf, size_t len,
> +			     size_t start)
> +{

This looks like session handles are virtualized.  I believe that this 
will break the HMAC for commands (e.g. TPM2_PolicySecret) that have
a session handle in the handle area.  The session's handle is its
"Name" and is included in the cpHash (command parameter hash) data
that is HMACed.


--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jarkko Sakkinen Jan. 16, 2017, 10:04 a.m. UTC | #2
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 

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);
>  
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jarkko Sakkinen Jan. 16, 2017, 10:05 a.m. UTC | #3
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);
> >  
> > 
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
James Bottomley Jan. 16, 2017, 11:18 p.m. UTC | #4
On Mon, 2017-01-16 at 12:04 +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 
> 
> 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.

Yes, so Ken also had a reply to this which the Mailing List seems to
have eaten:

> This looks like session handles are virtualized.  I believe that this 
> will break the HMAC for commands (e.g. TPM2_PolicySecret) that have
> a session handle in the handle area.  The session's handle is its 
> "Name" and is included in the cpHash (command parameter hash) data 
> that is HMACed. 

Basically this means that the advice to virtualize session handles in
the TCG RM document is wrong and we have to use physical handles. I'll
redo the implementation for this ... and now, since we'll have nothing
to use as an index, it probably does make sense to have sessions in a
separate array.  I can also separate isolation from context switching
... although I really think this is less optimal: my TPM only allows
three active context handles, so if we don't context switch them
identially to transient object (which it also only allows three of) I'm
going to run out.  I actually redid my openssl_tpm_engine patches so
they use session handles for parameter encryption and HMAC based
authority, so this may end up biting me soon ...

James

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jarkko Sakkinen Jan. 17, 2017, 7:23 a.m. UTC | #5
On Mon, Jan 16, 2017 at 03:18:45PM -0800, James Bottomley wrote:
> On Mon, 2017-01-16 at 12:04 +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 
> > 
> > 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.
> 
> Yes, so Ken also had a reply to this which the Mailing List seems to
> have eaten:
> 
> > This looks like session handles are virtualized.  I believe that this 
> > will break the HMAC for commands (e.g. TPM2_PolicySecret) that have
> > a session handle in the handle area.  The session's handle is its 
> > "Name" and is included in the cpHash (command parameter hash) data 
> > that is HMACed. 
> 
> Basically this means that the advice to virtualize session handles in
> the TCG RM document is wrong and we have to use physical handles. I'll
> redo the implementation for this ... and now, since we'll have nothing
> to use as an index, it probably does make sense to have sessions in a
> separate array.  I can also separate isolation from context switching
> ... although I really think this is less optimal: my TPM only allows
> three active context handles, so if we don't context switch them
> identially to transient object (which it also only allows three of) I'm
> going to run out.  I actually redid my openssl_tpm_engine patches so
> they use session handles for parameter encryption and HMAC based
> authority, so this may end up biting me soon ...
> 
> James

This might be obvious to you but saying this just in case: everytime a
session handle is created, you must traverse through struct tpm_space
instances and check if they have that physical handle and remove it so
that they don't leak.

I would probably just create a linked list of struct tpm_space
instances. Radix tree does not make much sense with the amount of
sessions you might except to have on a system simultaneously.

Anyway, this kind of lazy approach where you clean up as new stuff
gets created is probably also the most straight forward...

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
James Bottomley Jan. 17, 2017, 2:18 p.m. UTC | #6
On Tue, 2017-01-17 at 09:23 +0200, Jarkko Sakkinen wrote:
> On Mon, Jan 16, 2017 at 03:18:45PM -0800, James Bottomley wrote:
> > On Mon, 2017-01-16 at 12:04 +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 
> > > 
> > > 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.
> > 
> > Yes, so Ken also had a reply to this which the Mailing List seems
> > to
> > have eaten:
> > 
> > > This looks like session handles are virtualized.  I believe that
> > > this 
> > > will break the HMAC for commands (e.g. TPM2_PolicySecret) that
> > > have
> > > a session handle in the handle area.  The session's handle is its
> > > "Name" and is included in the cpHash (command parameter hash)
> > > data 
> > > that is HMACed. 
> > 
> > Basically this means that the advice to virtualize session handles
> > in
> > the TCG RM document is wrong and we have to use physical handles.
> > I'll
> > redo the implementation for this ... and now, since we'll have
> > nothing
> > to use as an index, it probably does make sense to have sessions in
> > a
> > separate array.  I can also separate isolation from context
> > switching
> > ... although I really think this is less optimal: my TPM only
> > allows
> > three active context handles, so if we don't context switch them
> > identially to transient object (which it also only allows three of)
> > I'm
> > going to run out.  I actually redid my openssl_tpm_engine patches
> > so
> > they use session handles for parameter encryption and HMAC based
> > authority, so this may end up biting me soon ...
> > 
> > James
> 
> This might be obvious to you but saying this just in case: everytime 
> a session handle is created, you must traverse through struct 
> tpm_space instances and check if they have that physical handle and 
> remove it so that they don't leak.

Actually, the code pre-emptively manages handles, so it ensures that we
actively collect them (in the flush emulation and the command attribute
processing).  I've got an unpublished patch that actually does global
session management, but it's part of the resource exhaustion stuff
which I'm still testing.

Note that when we save contexts, there's no need even to check.  If a
handle gets re-used, the old context won't load.

> I would probably just create a linked list of struct tpm_space
> instances. Radix tree does not make much sense with the amount of
> sessions you might except to have on a system simultaneously.

Global array.  The tpm has a small limit for total number of sessions
TPM_PT_SESSIONS_MAX (it's about 64 in every TPM I've seen).

> Anyway, this kind of lazy approach where you clean up as new stuff
> gets created is probably also the most straight forward...

I think we need to manage handles proactively.  The problems come when
we have more saved contexts than TPM_PT_SESSIONS_MAX, so we need to
aggressively flush them.

James


--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jarkko Sakkinen Jan. 17, 2017, 4:21 p.m. UTC | #7
On Tue, Jan 17, 2017 at 09:01:59AM -0500, Ken Goldman wrote:
> On 1/16/2017 6:18 PM, James Bottomley wrote:
> >
> > Basically this means that the advice to virtualize session handles
> > in the TCG RM document is wrong and we have to use physical handles.
> > I'll redo the implementation for this ... and now, since we'll have
> > nothing to use as an index, it probably does make sense to have
> > sessions in a separate array.  I can also separate isolation from
> > context switching ... although I really think this is less optimal:
> > my TPM only allows three active context handles, so if we don't
> > context switch them identically to transient object (which it also
> > only allows three of) I'm going to run out.  I actually redid my
> > openssl_tpm_engine patches so they use session handles for parameter
> > encryption and HMAC based authority, so this may end up biting me
> > soon ...
> 
> I think you have to context save sessions, just as you do with transient 
> objects.  Otherwise, only one process at a time can connect.

Isolation is self-contained step that can be tested and possible
regressions catched.

I could even consider landing isolation in one release and swapping in
subsequent in order to keep the release content more digestable for
upper layer maintainers and risk of causing major regressions small.

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jarkko Sakkinen Jan. 17, 2017, 4:29 p.m. UTC | #8
On Tue, Jan 17, 2017 at 06:18:12AM -0800, James Bottomley wrote:
> On Tue, 2017-01-17 at 09:23 +0200, Jarkko Sakkinen wrote:
> > On Mon, Jan 16, 2017 at 03:18:45PM -0800, James Bottomley wrote:
> > > On Mon, 2017-01-16 at 12:04 +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 
> > > > 
> > > > 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.
> > > 
> > > Yes, so Ken also had a reply to this which the Mailing List seems
> > > to
> > > have eaten:
> > > 
> > > > This looks like session handles are virtualized.  I believe that
> > > > this 
> > > > will break the HMAC for commands (e.g. TPM2_PolicySecret) that
> > > > have
> > > > a session handle in the handle area.  The session's handle is its
> > > > "Name" and is included in the cpHash (command parameter hash)
> > > > data 
> > > > that is HMACed. 
> > > 
> > > Basically this means that the advice to virtualize session handles
> > > in
> > > the TCG RM document is wrong and we have to use physical handles.
> > > I'll
> > > redo the implementation for this ... and now, since we'll have
> > > nothing
> > > to use as an index, it probably does make sense to have sessions in
> > > a
> > > separate array.  I can also separate isolation from context
> > > switching
> > > ... although I really think this is less optimal: my TPM only
> > > allows
> > > three active context handles, so if we don't context switch them
> > > identially to transient object (which it also only allows three of)
> > > I'm
> > > going to run out.  I actually redid my openssl_tpm_engine patches
> > > so
> > > they use session handles for parameter encryption and HMAC based
> > > authority, so this may end up biting me soon ...
> > > 
> > > James
> > 
> > This might be obvious to you but saying this just in case: everytime 
> > a session handle is created, you must traverse through struct 
> > tpm_space instances and check if they have that physical handle and 
> > remove it so that they don't leak.
> 
> Actually, the code pre-emptively manages handles, so it ensures that we
> actively collect them (in the flush emulation and the command attribute
> processing).  I've got an unpublished patch that actually does global
> session management, but it's part of the resource exhaustion stuff
> which I'm still testing.
> 
> Note that when we save contexts, there's no need even to check.  If a
> handle gets re-used, the old context won't load.

Yes, I understand. In that way swapping simplifies things like it does
with transient objects.

> > I would probably just create a linked list of struct tpm_space
> > instances. Radix tree does not make much sense with the amount of
> > sessions you might except to have on a system simultaneously.
> 
> Global array.  The tpm has a small limit for total number of sessions
> TPM_PT_SESSIONS_MAX (it's about 64 in every TPM I've seen).

[x]

> > Anyway, this kind of lazy approach where you clean up as new stuff
> > gets created is probably also the most straight forward...
> 
> I think we need to manage handles proactively.  The problems come when
> we have more saved contexts than TPM_PT_SESSIONS_MAX, so we need to
> aggressively flush them.

I leave it up to you decide how you want to do it. In some ways swapping
does simplify things so it's not yet obvious whether a kind of breakdown
where isolation would be done as an iterative step would make sense in
the first place.

Forget what I adviced and I'll look forward for a new patch :-)

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

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);