[tpmdd-devel,6/6] tpm2: add session handle context saving and restoring to the space code

Submitted by Jarkko Sakkinen on Feb. 8, 2017, 11:07 a.m.

Details

Message ID 20170208110713.14070-7-jarkko.sakkinen@linux.intel.com
State New
Headers show

Commit Message

Jarkko Sakkinen Feb. 8, 2017, 11:07 a.m.
From: James Bottomley <James.Bottomley@HansenPartnership.com>

Sessions are different from transient objects in that their handles
may not be virtualized (because they're used for some hmac
calculations).  Additionally when a session is context saved, a
vestigial memory remains in the TPM and if it is also flushed, that
will be lost and the session context will refuse to load next time, so
the code is updated to flush only transient objects after a context
save.  Add a separate array (chip->session_tbl) to save and restore
sessions by handle.  Use the failure of a context save or load to
signal that the session has been flushed from the TPM and we can
remove its memory from chip->session_tbl.

Sessions are also isolated during each instance of a tpm space.  This
means that spaces shouldn't be able to see each other's sessions and
is enforced by ensuring that a space user may only refer to sessions
handles that are present in their own chip->session_tbl.  Finally when
a space is closed, all the sessions belonging to it should be flushed
so the handles may be re-used by other spaces.

Note that if we get a session save or load error, all sessions are
effectively flushed.  Even though we restore the session buffer, all
the old sessions will refuse to load after the flush and they'll be
purged from our session memory.  This means that while transient
context handling is still soft in the face of errors, session handling
is hard (any failure of the model means all sessions are lost).

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/char/tpm/tpm-chip.c   |   6 +++
 drivers/char/tpm/tpm.h        |   4 +-
 drivers/char/tpm/tpm2-space.c | 115 ++++++++++++++++++++++++++++++++++++++++--
 drivers/char/tpm/tpms-dev.c   |   2 +-
 4 files changed, 122 insertions(+), 5 deletions(-)

Comments

Jarkko Sakkinen Feb. 10, 2017, 8:52 a.m.
On Wed, Feb 08, 2017 at 01:07:08PM +0200, Jarkko Sakkinen wrote:
> +		rc = tpm2_load_context(chip, space->session_buf,
> +				       &offset, &handle);
> +		if (rc == -ENOENT) {
> +			/* load failed, just forget session */
> +			space->session_tbl[i] = 0;

This is my only concern in this commit. Should we also in this case just
flush the space or not?

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
Jarkko Sakkinen Feb. 10, 2017, 12:32 p.m.
On Wed, Feb 08, 2017 at 01:07:08PM +0200, Jarkko Sakkinen wrote:
> From: James Bottomley <James.Bottomley@HansenPartnership.com>
> 
> Sessions are different from transient objects in that their handles
> may not be virtualized (because they're used for some hmac
> calculations).  Additionally when a session is context saved, a
> vestigial memory remains in the TPM and if it is also flushed, that
> will be lost and the session context will refuse to load next time, so
> the code is updated to flush only transient objects after a context
> save.  Add a separate array (chip->session_tbl) to save and restore
> sessions by handle.  Use the failure of a context save or load to
> signal that the session has been flushed from the TPM and we can
> remove its memory from chip->session_tbl.
> 
> Sessions are also isolated during each instance of a tpm space.  This
> means that spaces shouldn't be able to see each other's sessions and
> is enforced by ensuring that a space user may only refer to sessions
> handles that are present in their own chip->session_tbl.  Finally when
> a space is closed, all the sessions belonging to it should be flushed
> so the handles may be re-used by other spaces.
> 
> Note that if we get a session save or load error, all sessions are
> effectively flushed.  Even though we restore the session buffer, all
> the old sessions will refuse to load after the flush and they'll be
> purged from our session memory.  This means that while transient
> context handling is still soft in the face of errors, session handling
> is hard (any failure of the model means all sessions are lost).
> 
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> ---
>  drivers/char/tpm/tpm-chip.c   |   6 +++
>  drivers/char/tpm/tpm.h        |   4 +-
>  drivers/char/tpm/tpm2-space.c | 115 ++++++++++++++++++++++++++++++++++++++++--
>  drivers/char/tpm/tpms-dev.c   |   2 +-
>  4 files changed, 122 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index c71c353..e8536ab 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -130,6 +130,7 @@ static void tpm_dev_release(struct device *dev)
>  
>  	kfree(chip->log.bios_event_log);
>  	kfree(chip->work_space.context_buf);
> +	kfree(chip->work_space.session_buf);
>  	kfree(chip);
>  }
>  
> @@ -224,6 +225,11 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
>  		rc = -ENOMEM;
>  		goto out;
>  	}
> +	chip->work_space.session_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
> +	if (!chip->work_space.session_buf) {
> +		rc = -ENOMEM;
> +		goto out;
> +	}
>  
>  	return chip;
>  
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 8670cc5..9261223 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -160,6 +160,8 @@ enum tpm2_cc_attrs {
>  struct tpm_space {
>  	u32 context_tbl[3];
>  	u8 *context_buf;
> +	u32 session_tbl[3];
> +	u8 *session_buf;
>  };
>  
>  enum tpm_chip_flags {
> @@ -588,7 +590,7 @@ int tpm2_probe(struct tpm_chip *chip);
>  ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip);
>  int tpm2_find_cc(struct tpm_chip *chip, u32 cc);
>  int tpm2_init_space(struct tpm_space *space);
> -void tpm2_del_space(struct tpm_space *space);
> +void tpm2_del_space(struct tpm_chip *chip, struct tpm_space *space);
>  int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
>  		       u8 *cmd);
>  int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
> diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
> index d241c2a..fb817ca 100644
> --- a/drivers/char/tpm/tpm2-space.c
> +++ b/drivers/char/tpm/tpm2-space.c
> @@ -32,18 +32,39 @@ struct tpm2_context {
>  	__be16 blob_size;
>  } __packed;
>  
> +static void tpm2_flush_sessions(struct tpm_chip *chip, struct tpm_space *space)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(space->session_tbl); i++) {
> +		if (space->session_tbl[i])
> +			tpm2_flush_context_cmd(chip, space->session_tbl[i],
> +					       TPM_TRANSMIT_UNLOCKED);
> +	}
> +}
> +
>  int tpm2_init_space(struct tpm_space *space)
>  {
>  	space->context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
>  	if (!space->context_buf)
>  		return -ENOMEM;
>  
> +	space->session_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
> +	if (space->session_buf == NULL) {
> +		kfree(space->context_buf);
> +		return -ENOMEM;
> +	}
> +
>  	return 0;
>  }
>  
> -void tpm2_del_space(struct tpm_space *space)
> +void tpm2_del_space(struct tpm_chip *chip, struct tpm_space *space)
>  {
> +	mutex_lock(&chip->tpm_mutex);
> +	tpm2_flush_sessions(chip, space);
> +	mutex_unlock(&chip->tpm_mutex);
>  	kfree(space->context_buf);
> +	kfree(space->session_buf);
>  }
>  
>  static int tpm2_load_context(struct tpm_chip *chip, u8 *buf,
> @@ -69,6 +90,20 @@ static int tpm2_load_context(struct tpm_chip *chip, u8 *buf,
>  			 __func__, rc);
>  		tpm_buf_destroy(&tbuf);
>  		return -EFAULT;
> +	} else if (tpm2_rc_value(rc) == TPM2_RC_HANDLE ||
> +		   rc == TPM2_RC_REFERENCE_H0) {
> +		/*
> +		 * TPM_RC_HANDLE means that the session context can't
> +		 * be loaded because of an internal counter mismatch
> +		 * that makes the TPM think there might have been a
> +		 * replay.  This might happen if the context was saved
> +		 * and loaded outside the space.
> +		 *
> +		 * TPM_RC_REFERENCE_H0 means the session has been
> +		 * flushed outside the space
> +		 */
> +		rc = -ENOENT;
> +		tpm_buf_destroy(&tbuf);
>  	} else if (rc > 0) {
>  		dev_warn(&chip->dev, "%s: failed with a TPM error 0x%04X\n",
>  			 __func__, rc);
> @@ -121,12 +156,30 @@ static int tpm2_save_context(struct tpm_chip *chip, u32 handle, u8 *buf,
>  	}
>  
>  	memcpy(&buf[*offset], &tbuf.data[TPM_HEADER_SIZE], body_size);
> -	tpm2_flush_context_cmd(chip, handle, TPM_TRANSMIT_UNLOCKED);
>  	*offset += body_size;
>  	tpm_buf_destroy(&tbuf);
>  	return 0;
>  }
>  
> +static int tpm2_session_add(struct tpm_chip *chip, u32 handle)
> +{
> +	struct tpm_space *space = &chip->work_space;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(space->session_tbl); i++)
> +		if (space->session_tbl[i] == 0)
> +			break;
> +	if (i == ARRAY_SIZE(space->session_tbl)) {
> +		dev_err(&chip->dev, "out of session slots\n");

This really should be dev_dbg.

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
James Bottomley Feb. 10, 2017, 4:11 p.m.
On Fri, 2017-02-10 at 10:52 +0200, Jarkko Sakkinen wrote:
> On Wed, Feb 08, 2017 at 01:07:08PM +0200, Jarkko Sakkinen wrote:
> > +		rc = tpm2_load_context(chip, space->session_buf,
> > +				       &offset, &handle);
> > +		if (rc == -ENOENT) {
> > +			/* load failed, just forget session */
> > +			space->session_tbl[i] = 0;
> 
> This is my only concern in this commit. Should we also in this case 
> just flush the space or not?

I elected not to.  If the handle is flushed by an external resource
manager, we get this event.  If the RM and the app agreed to release
the session handle, then flushing the space would be overkill because
it would destroy the client session, so simply removing the handle
works.  If the client tries to use the session again, it gets an error
and if it doesn't everything just works, which seems to be optimal.

James



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
James Bottomley Feb. 10, 2017, 4:17 p.m.
On Fri, 2017-02-10 at 14:32 +0200, Jarkko Sakkinen wrote:
> On Wed, Feb 08, 2017 at 01:07:08PM +0200, Jarkko Sakkinen wrote:
> > From: James Bottomley <James.Bottomley@HansenPartnership.com>
[...] 
> > +static int tpm2_session_add(struct tpm_chip *chip, u32 handle)
> > +{
> > +	struct tpm_space *space = &chip->work_space;
> > +	int i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(space->session_tbl); i++)
> > +		if (space->session_tbl[i] == 0)
> > +			break;
> > +	if (i == ARRAY_SIZE(space->session_tbl)) {
> > +		dev_err(&chip->dev, "out of session slots\n");
> 
> This really should be dev_dbg.

This was my reply to the comment the last time:

    I can do that, but I think this should be higher than debug.  If
    this trips, something an application was doing will fail with a non
    TPM error and someone may wish to investigate why.  Having a kernel
    message would help with that (but they won't see it if it's debug).

    I'm also leaning towards the idea that we should actually have one
    more _tbl slot than we know the TPM does, so that if someone goes
    over it's the TPM that gives them a real TPM out of memory error
    rather than the space code returning -ENOMEM.

    If you agree, I think it should be four for both sessions_tbl and
    context_tbl.

So I really don't think it should be debug.  Could we compromise on
dev_info?

James


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
Jarkko Sakkinen Feb. 10, 2017, 7:07 p.m.
On Fri, Feb 10, 2017 at 08:11:18AM -0800, James Bottomley wrote:
> On Fri, 2017-02-10 at 10:52 +0200, Jarkko Sakkinen wrote:
> > On Wed, Feb 08, 2017 at 01:07:08PM +0200, Jarkko Sakkinen wrote:
> > > +		rc = tpm2_load_context(chip, space->session_buf,
> > > +				       &offset, &handle);
> > > +		if (rc == -ENOENT) {
> > > +			/* load failed, just forget session */
> > > +			space->session_tbl[i] = 0;
> > 
> > This is my only concern in this commit. Should we also in this case 
> > just flush the space or not?
> 
> I elected not to.  If the handle is flushed by an external resource
> manager, we get this event.  If the RM and the app agreed to release
> the session handle, then flushing the space would be overkill because
> it would destroy the client session, so simply removing the handle
> works.  If the client tries to use the session again, it gets an error
> and if it doesn't everything just works, which seems to be optimal.
> 
> James

Makes sense. Just wanted the check the logic you had in this decision.

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
Jarkko Sakkinen Feb. 10, 2017, 7:10 p.m.
On Fri, Feb 10, 2017 at 08:17:11AM -0800, James Bottomley wrote:
> On Fri, 2017-02-10 at 14:32 +0200, Jarkko Sakkinen wrote:
> > On Wed, Feb 08, 2017 at 01:07:08PM +0200, Jarkko Sakkinen wrote:
> > > From: James Bottomley <James.Bottomley@HansenPartnership.com>
> [...] 
> > > +static int tpm2_session_add(struct tpm_chip *chip, u32 handle)
> > > +{
> > > +	struct tpm_space *space = &chip->work_space;
> > > +	int i;
> > > +
> > > +	for (i = 0; i < ARRAY_SIZE(space->session_tbl); i++)
> > > +		if (space->session_tbl[i] == 0)
> > > +			break;
> > > +	if (i == ARRAY_SIZE(space->session_tbl)) {
> > > +		dev_err(&chip->dev, "out of session slots\n");
> > 
> > This really should be dev_dbg.
> 
> This was my reply to the comment the last time:
> 
>     I can do that, but I think this should be higher than debug.  If
>     this trips, something an application was doing will fail with a non
>     TPM error and someone may wish to investigate why.  Having a kernel
>     message would help with that (but they won't see it if it's debug).
> 
>     I'm also leaning towards the idea that we should actually have one
>     more _tbl slot than we know the TPM does, so that if someone goes
>     over it's the TPM that gives them a real TPM out of memory error
>     rather than the space code returning -ENOMEM.
> 
>     If you agree, I think it should be four for both sessions_tbl and
>     context_tbl.
> 
> So I really don't think it should be debug.  Could we compromise on
> dev_info?
> 
> James

Oops, I'm sorry about that. I use the release chaos as an excuse :-)
I would lower it to dev_warn().

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
James Bottomley Feb. 10, 2017, 7:12 p.m.
On Fri, 2017-02-10 at 21:10 +0200, Jarkko Sakkinen wrote:
> On Fri, Feb 10, 2017 at 08:17:11AM -0800, James Bottomley wrote:
> > On Fri, 2017-02-10 at 14:32 +0200, Jarkko Sakkinen wrote:
> > > On Wed, Feb 08, 2017 at 01:07:08PM +0200, Jarkko Sakkinen wrote:
> > > > From: James Bottomley <James.Bottomley@HansenPartnership.com>
> > [...] 
> > > > +static int tpm2_session_add(struct tpm_chip *chip, u32 handle)
> > > > +{
> > > > +	struct tpm_space *space = &chip->work_space;
> > > > +	int i;
> > > > +
> > > > +	for (i = 0; i < ARRAY_SIZE(space->session_tbl); i++)
> > > > +		if (space->session_tbl[i] == 0)
> > > > +			break;
> > > > +	if (i == ARRAY_SIZE(space->session_tbl)) {
> > > > +		dev_err(&chip->dev, "out of session slots\n");
> > > 
> > > This really should be dev_dbg.
> > 
> > This was my reply to the comment the last time:
> > 
> >     I can do that, but I think this should be higher than debug. 
> >  If
> >     this trips, something an application was doing will fail with a
> > non
> >     TPM error and someone may wish to investigate why.  Having a
> > kernel
> >     message would help with that (but they won't see it if it's
> > debug).
> > 
> >     I'm also leaning towards the idea that we should actually have
> > one
> >     more _tbl slot than we know the TPM does, so that if someone
> > goes
> >     over it's the TPM that gives them a real TPM out of memory
> > error
> >     rather than the space code returning -ENOMEM.
> > 
> >     If you agree, I think it should be four for both sessions_tbl
> > and
> >     context_tbl.
> > 
> > So I really don't think it should be debug.  Could we compromise on
> > dev_info?
> > 
> > James
> 
> Oops, I'm sorry about that. I use the release chaos as an excuse :-)
> I would lower it to dev_warn().

That works.  Do you want me to resend the patch?

James



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
Jarkko Sakkinen Feb. 10, 2017, 10:48 p.m.
On Fri, Feb 10, 2017 at 11:12:15AM -0800, James Bottomley wrote:
> On Fri, 2017-02-10 at 21:10 +0200, Jarkko Sakkinen wrote:
> > On Fri, Feb 10, 2017 at 08:17:11AM -0800, James Bottomley wrote:
> > > On Fri, 2017-02-10 at 14:32 +0200, Jarkko Sakkinen wrote:
> > > > On Wed, Feb 08, 2017 at 01:07:08PM +0200, Jarkko Sakkinen wrote:
> > > > > From: James Bottomley <James.Bottomley@HansenPartnership.com>
> > > [...] 
> > > > > +static int tpm2_session_add(struct tpm_chip *chip, u32 handle)
> > > > > +{
> > > > > +	struct tpm_space *space = &chip->work_space;
> > > > > +	int i;
> > > > > +
> > > > > +	for (i = 0; i < ARRAY_SIZE(space->session_tbl); i++)
> > > > > +		if (space->session_tbl[i] == 0)
> > > > > +			break;
> > > > > +	if (i == ARRAY_SIZE(space->session_tbl)) {
> > > > > +		dev_err(&chip->dev, "out of session slots\n");
> > > > 
> > > > This really should be dev_dbg.
> > > 
> > > This was my reply to the comment the last time:
> > > 
> > >     I can do that, but I think this should be higher than debug. 
> > >  If
> > >     this trips, something an application was doing will fail with a
> > > non
> > >     TPM error and someone may wish to investigate why.  Having a
> > > kernel
> > >     message would help with that (but they won't see it if it's
> > > debug).
> > > 
> > >     I'm also leaning towards the idea that we should actually have
> > > one
> > >     more _tbl slot than we know the TPM does, so that if someone
> > > goes
> > >     over it's the TPM that gives them a real TPM out of memory
> > > error
> > >     rather than the space code returning -ENOMEM.
> > > 
> > >     If you agree, I think it should be four for both sessions_tbl
> > > and
> > >     context_tbl.
> > > 
> > > So I really don't think it should be debug.  Could we compromise on
> > > dev_info?
> > > 
> > > James
> > 
> > Oops, I'm sorry about that. I use the release chaos as an excuse :-)
> > I would lower it to dev_warn().
> 
> That works.  Do you want me to resend the patch?
> 
> James

No need for that.

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

Patch hide | download patch | download mbox

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index c71c353..e8536ab 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -130,6 +130,7 @@  static void tpm_dev_release(struct device *dev)
 
 	kfree(chip->log.bios_event_log);
 	kfree(chip->work_space.context_buf);
+	kfree(chip->work_space.session_buf);
 	kfree(chip);
 }
 
@@ -224,6 +225,11 @@  struct tpm_chip *tpm_chip_alloc(struct device *pdev,
 		rc = -ENOMEM;
 		goto out;
 	}
+	chip->work_space.session_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!chip->work_space.session_buf) {
+		rc = -ENOMEM;
+		goto out;
+	}
 
 	return chip;
 
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 8670cc5..9261223 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -160,6 +160,8 @@  enum tpm2_cc_attrs {
 struct tpm_space {
 	u32 context_tbl[3];
 	u8 *context_buf;
+	u32 session_tbl[3];
+	u8 *session_buf;
 };
 
 enum tpm_chip_flags {
@@ -588,7 +590,7 @@  int tpm2_probe(struct tpm_chip *chip);
 ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip);
 int tpm2_find_cc(struct tpm_chip *chip, u32 cc);
 int tpm2_init_space(struct tpm_space *space);
-void tpm2_del_space(struct tpm_space *space);
+void tpm2_del_space(struct tpm_chip *chip, struct tpm_space *space);
 int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
 		       u8 *cmd);
 int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
index d241c2a..fb817ca 100644
--- a/drivers/char/tpm/tpm2-space.c
+++ b/drivers/char/tpm/tpm2-space.c
@@ -32,18 +32,39 @@  struct tpm2_context {
 	__be16 blob_size;
 } __packed;
 
+static void tpm2_flush_sessions(struct tpm_chip *chip, struct tpm_space *space)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(space->session_tbl); i++) {
+		if (space->session_tbl[i])
+			tpm2_flush_context_cmd(chip, space->session_tbl[i],
+					       TPM_TRANSMIT_UNLOCKED);
+	}
+}
+
 int tpm2_init_space(struct tpm_space *space)
 {
 	space->context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
 	if (!space->context_buf)
 		return -ENOMEM;
 
+	space->session_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
+	if (space->session_buf == NULL) {
+		kfree(space->context_buf);
+		return -ENOMEM;
+	}
+
 	return 0;
 }
 
-void tpm2_del_space(struct tpm_space *space)
+void tpm2_del_space(struct tpm_chip *chip, struct tpm_space *space)
 {
+	mutex_lock(&chip->tpm_mutex);
+	tpm2_flush_sessions(chip, space);
+	mutex_unlock(&chip->tpm_mutex);
 	kfree(space->context_buf);
+	kfree(space->session_buf);
 }
 
 static int tpm2_load_context(struct tpm_chip *chip, u8 *buf,
@@ -69,6 +90,20 @@  static int tpm2_load_context(struct tpm_chip *chip, u8 *buf,
 			 __func__, rc);
 		tpm_buf_destroy(&tbuf);
 		return -EFAULT;
+	} else if (tpm2_rc_value(rc) == TPM2_RC_HANDLE ||
+		   rc == TPM2_RC_REFERENCE_H0) {
+		/*
+		 * TPM_RC_HANDLE means that the session context can't
+		 * be loaded because of an internal counter mismatch
+		 * that makes the TPM think there might have been a
+		 * replay.  This might happen if the context was saved
+		 * and loaded outside the space.
+		 *
+		 * TPM_RC_REFERENCE_H0 means the session has been
+		 * flushed outside the space
+		 */
+		rc = -ENOENT;
+		tpm_buf_destroy(&tbuf);
 	} else if (rc > 0) {
 		dev_warn(&chip->dev, "%s: failed with a TPM error 0x%04X\n",
 			 __func__, rc);
@@ -121,12 +156,30 @@  static int tpm2_save_context(struct tpm_chip *chip, u32 handle, u8 *buf,
 	}
 
 	memcpy(&buf[*offset], &tbuf.data[TPM_HEADER_SIZE], body_size);
-	tpm2_flush_context_cmd(chip, handle, TPM_TRANSMIT_UNLOCKED);
 	*offset += body_size;
 	tpm_buf_destroy(&tbuf);
 	return 0;
 }
 
+static int tpm2_session_add(struct tpm_chip *chip, u32 handle)
+{
+	struct tpm_space *space = &chip->work_space;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(space->session_tbl); i++)
+		if (space->session_tbl[i] == 0)
+			break;
+	if (i == ARRAY_SIZE(space->session_tbl)) {
+		dev_err(&chip->dev, "out of session slots\n");
+		tpm2_flush_context_cmd(chip, handle, TPM_TRANSMIT_UNLOCKED);
+		return -ENOMEM;
+	}
+
+	space->session_tbl[i] = handle;
+
+	return 0;
+}
+
 static void tpm2_flush_space(struct tpm_chip *chip)
 {
 	struct tpm_space *space = &chip->work_space;
@@ -136,6 +189,8 @@  static void tpm2_flush_space(struct tpm_chip *chip)
 		if (space->context_tbl[i] && ~space->context_tbl[i])
 			tpm2_flush_context_cmd(chip, space->context_tbl[i],
 					       TPM_TRANSMIT_UNLOCKED);
+
+	tpm2_flush_sessions(chip, space);
 }
 
 static int tpm2_load_space(struct tpm_chip *chip)
@@ -161,6 +216,28 @@  static int tpm2_load_space(struct tpm_chip *chip)
 			return rc;
 	}
 
+	for (i = 0, offset = 0; i < ARRAY_SIZE(space->session_tbl); i++) {
+		u32 handle;
+
+		if (!space->session_tbl[i])
+			continue;
+
+		rc = tpm2_load_context(chip, space->session_buf,
+				       &offset, &handle);
+		if (rc == -ENOENT) {
+			/* load failed, just forget session */
+			space->session_tbl[i] = 0;
+		} else if (rc) {
+			tpm2_flush_space(chip);
+			return rc;
+		}
+		if (handle != space->session_tbl[i]) {
+			dev_warn(&chip->dev, "session restored to wrong handle\n");
+			tpm2_flush_space(chip);
+			return -EFAULT;
+		}
+	}
+
 	return 0;
 }
 
@@ -220,7 +297,10 @@  int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
 
 	memcpy(&chip->work_space.context_tbl, &space->context_tbl,
 	       sizeof(space->context_tbl));
+	memcpy(&chip->work_space.session_tbl, &space->session_tbl,
+	       sizeof(space->session_tbl));
 	memcpy(chip->work_space.context_buf, space->context_buf, PAGE_SIZE);
+	memcpy(chip->work_space.session_buf, space->session_buf, PAGE_SIZE);
 
 	rc = tpm2_load_space(chip);
 	if (rc) {
@@ -241,6 +321,7 @@  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_type;
 	u32 vhandle;
 	u32 attrs;
 	u32 return_code = get_unaligned_be32((__be32 *)&rsp[6]);
@@ -259,9 +340,15 @@  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;
 
+	if (phandle_type != TPM2_HT_TRANSIENT)
+		return tpm2_session_add(chip, phandle);
+
 	/* Garbage collect a dead context. */
 	for (i = 0; i < ARRAY_SIZE(space->context_tbl); i++) {
 		if (space->context_tbl[i] == phandle) {
@@ -307,9 +394,28 @@  static int tpm2_save_space(struct tpm_chip *chip)
 		} else if (rc)
 			return rc;
 
+		tpm2_flush_context_cmd(chip, space->context_tbl[i],
+				       TPM_TRANSMIT_UNLOCKED);
 		space->context_tbl[i] = ~0;
 	}
 
+	for (i = 0, offset = 0; i < ARRAY_SIZE(space->session_tbl); i++) {
+		if (!space->session_tbl[i])
+			continue;
+
+		rc = tpm2_save_context(chip, space->session_tbl[i],
+				       space->session_buf, PAGE_SIZE,
+				       &offset);
+
+		if (rc == -ENOENT) {
+			/* handle error saving session, just forget it */
+			space->session_tbl[i] = 0;
+		} else if (rc < 0) {
+			tpm2_flush_space(chip);
+			return rc;
+		}
+	}
+
 	return 0;
 }
 
@@ -335,7 +441,10 @@  int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
 
 	memcpy(&space->context_tbl, &chip->work_space.context_tbl,
 	       sizeof(space->context_tbl));
+	memcpy(&space->session_tbl, &chip->work_space.session_tbl,
+	       sizeof(space->session_tbl));
 	memcpy(space->context_buf, chip->work_space.context_buf, PAGE_SIZE);
+	memcpy(space->session_buf, chip->work_space.session_buf, PAGE_SIZE);
 
 	return 0;
 }
diff --git a/drivers/char/tpm/tpms-dev.c b/drivers/char/tpm/tpms-dev.c
index 5720885..4c98db8 100644
--- a/drivers/char/tpm/tpms-dev.c
+++ b/drivers/char/tpm/tpms-dev.c
@@ -39,7 +39,7 @@  static int tpms_release(struct inode *inode, struct file *file)
 	struct tpms_priv *priv = container_of(fpriv, struct tpms_priv, priv);
 
 	tpm_common_release(file, fpriv);
-	tpm2_del_space(&priv->space);
+	tpm2_del_space(fpriv->chip, &priv->space);
 	kfree(priv);
 
 	return 0;