| Message ID | 1485236231.2534.71.camel@HansenPartnership.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Jan 23, 2017 at 09:37:11PM -0800, James Bottomley wrote: > 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. > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> I'm wondering if you ever need more than two sessions at once? If we would limit the number of sessions to that you could probably simplify a lot. > --- > drivers/char/tpm/tpm-chip.c | 6 +++ > drivers/char/tpm/tpm.h | 3 ++ > drivers/char/tpm/tpm2-space.c | 99 +++++++++++++++++++++++++++++++++++++++---- > drivers/char/tpm/tpms-dev.c | 8 ++++ > 4 files changed, 107 insertions(+), 9 deletions(-) > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c > index ed4f887..6282ad0 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); > } > > @@ -223,6 +224,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 c48255e..b77fc60 100644 > --- a/drivers/char/tpm/tpm.h > +++ b/drivers/char/tpm/tpm.h > @@ -159,6 +159,8 @@ enum tpm2_cc_attrs { > struct tpm_space { > u32 context_tbl[3]; > u8 *context_buf; > + u32 session_tbl[6]; > + u8 *session_buf; > }; > > enum tpm_chip_flags { > @@ -588,4 +590,5 @@ 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, > u32 cc, u8 *buf, size_t bufsiz); > +void tpm2_flush_space(struct tpm_chip *chip, struct tpm_space *space); Why the extra parameter? > #endif > diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c > index 7fd2fc5..ba4310a 100644 > --- a/drivers/char/tpm/tpm2-space.c > +++ b/drivers/char/tpm/tpm2-space.c > @@ -59,11 +59,16 @@ static int tpm2_load_context(struct tpm_chip *chip, u8 *buf, > > rc = tpm_transmit_cmd(chip, NULL, tbuf.data, PAGE_SIZE, 4, > TPM_TRANSMIT_UNLOCKED, NULL); > + cruft > if (rc < 0) { > dev_warn(&chip->dev, "%s: failed with a system error %d\n", > __func__, rc); > tpm_buf_destroy(&tbuf); > return -EFAULT; > + } else if ((rc & TPM2_RC_HANDLE) == TPM2_RC_HANDLE || > + rc == TPM2_RC_REFERENCE_H0) { > + 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); > @@ -116,21 +121,44 @@ 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 void tpm2_flush_space(struct tpm_chip *chip) > +static int tpm2_session_add(struct tpm_chip *chip, > + struct tpm_space *space, u32 handle) > +{ > + 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; > +} > + > +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->session_tbl); i++) { > + if (space->session_tbl[i]) > + tpm2_flush_context_cmd(chip, space->session_tbl[i], > + TPM_TRANSMIT_UNLOCKED); > + } > } > > static int tpm2_load_space(struct tpm_chip *chip) > @@ -156,6 +184,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, space); > + return rc; > + } > + if (handle != space->session_tbl[i]) { > + dev_warn(&chip->dev, "session restored to wrong handle\n"); > + tpm2_flush_space(chip, space); > + return -EFAULT; > + } > + } > + > return 0; > } > > @@ -215,17 +265,20 @@ 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); For transient objects the rollback is straight forward and totally predictable. Given that with sessions you always keep some information in the TPM the rollback would be a bit more complicated. Now your code seems to just keep the previous session_buf, doesn't it? Does that always work or not? PS. I have a high-level idea of attack vectors that are prevented by having meta-data for session inside the TPM but can you point me to the correct place in the TPM 2.0 specification that discusses about this? /Jarkko ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
On Thu, 2017-01-26 at 14:51 +0200, Jarkko Sakkinen wrote: > On Mon, Jan 23, 2017 at 09:37:11PM -0800, James Bottomley wrote: > > 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. > > > > Signed-off-by: James Bottomley < > > James.Bottomley@HansenPartnership.com> > > I'm wondering if you ever need more than two sessions at once? If we > would limit the number of sessions to that you could probably > simplify a lot. Three seems to be the agreed maximum: hmac authority, parameter encryption and command audit. I'll fix up the rest James ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
On Thu, 2017-01-26 at 14:51 +0200, Jarkko Sakkinen wrote: [...] > > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h > > index c48255e..b77fc60 100644 > > --- a/drivers/char/tpm/tpm.h > > +++ b/drivers/char/tpm/tpm.h > > @@ -159,6 +159,8 @@ enum tpm2_cc_attrs { > > struct tpm_space { > > u32 context_tbl[3]; > > u8 *context_buf; > > + u32 session_tbl[6]; > > + u8 *session_buf; > > }; > > > > enum tpm_chip_flags { > > @@ -588,4 +590,5 @@ 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, > > u32 cc, u8 *buf, size_t bufsiz); > > +void tpm2_flush_space(struct tpm_chip *chip, struct tpm_space > > *space); > > Why the extra parameter? Because it was called from tpms-dev in release to flush all the sessions still active. At that point the work space is gone, so we have to call it with the real space. However, I realised that callsite didn't hold the tpm_mutex like it should and that we don't need to flush the contexts because they'll be guaranteed empty, so I added a new function tpm2_kill_space() that does this. > > #endif > > diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2 > > -space.c > > index 7fd2fc5..ba4310a 100644 > > --- a/drivers/char/tpm/tpm2-space.c > > +++ b/drivers/char/tpm/tpm2-space.c > > @@ -59,11 +59,16 @@ static int tpm2_load_context(struct tpm_chip > > *chip, u8 *buf, > > > > rc = tpm_transmit_cmd(chip, NULL, tbuf.data, PAGE_SIZE, 4, > > TPM_TRANSMIT_UNLOCKED, NULL); > > + > > cruft Removed. [...] > > @@ -215,17 +265,20 @@ 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); > > For transient objects the rollback is straight forward and totally > predictable. Given that with sessions you always keep some > information in the TPM the rollback would be a bit more complicated. There is basically no rollback unless you really want to understand what the commands did. If we get a fault on the context save or load that isn't one we understand, like TPM_RC_HANDLE meaning the session won't load or TPM_RC_REFERENCE_H0 meaning the session no longer exists, then I think the only option is flushing everything. I'd like us to agree on a hard failure model: if we get some unexplained error during our context loads or saves, we should clear out the entire space (which would allow us to use pointers instead of copyring) but we don't. So we follow the soft failure. However, sessions will mostly fail to load after this with RPM_RC_HANDLE, so we get the equivalent of soft failure for transients and hard failure for sessions. > Now your code seems to just keep the previous session_buf, doesn't > it? Does that always work or not? Yes, after tpm_flush_space, the session memory is gone and all the sessions will refuse to load with RC_HANDLE, so we end up effectively clearing them out. It seemed better to do it this way than to try to special case all the session stuff. > PS. I have a high-level idea of attack vectors that are prevented by > having meta-data for session inside the TPM but can you point me to > the correct place in the TPM 2.0 specification that discusses about > this? The problem is replay. If I'm snooping your TPM commands and I capture your session context, if we don't have replay detection, I can re-load your session HMAC and replay your command because the session has the authorization or the encryption. The TPM designers thought the only way to avoid replay was to use a counter which was part of the session context which increments every time a session is saved. On loading you check that the counters match and fail if they don't. The only way to implement this is to keep a memory of the counter in the TPM, hence the annoying vestigial sessions. It's note 2 of the architecture Part 1 guide, chapter "15.4 Session Handles (MSO=02_16 and 03_16 )" James > > /Jarkko > ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
On Thu, Jan 26, 2017 at 07:18:49AM -0800, James Bottomley wrote: > On Thu, 2017-01-26 at 14:51 +0200, Jarkko Sakkinen wrote: > > On Mon, Jan 23, 2017 at 09:37:11PM -0800, James Bottomley wrote: > > > 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. > > > > > > Signed-off-by: James Bottomley < > > > James.Bottomley@HansenPartnership.com> > > > > I'm wondering if you ever need more than two sessions at once? If we > > would limit the number of sessions to that you could probably > > simplify a lot. > > Three seems to be the agreed maximum: hmac authority, parameter > encryption and command audit. > > I'll fix up the rest > > James Right. I've also set the limit for trasient objects to three. /Jarkko ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
On Thu, Jan 26, 2017 at 08:26:19AM -0800, James Bottomley wrote: > On Thu, 2017-01-26 at 14:51 +0200, Jarkko Sakkinen wrote: > [...] > > > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h > > > index c48255e..b77fc60 100644 > > > --- a/drivers/char/tpm/tpm.h > > > +++ b/drivers/char/tpm/tpm.h > > > @@ -159,6 +159,8 @@ enum tpm2_cc_attrs { > > > struct tpm_space { > > > u32 context_tbl[3]; > > > u8 *context_buf; > > > + u32 session_tbl[6]; > > > + u8 *session_buf; > > > }; > > > > > > enum tpm_chip_flags { > > > @@ -588,4 +590,5 @@ 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, > > > u32 cc, u8 *buf, size_t bufsiz); > > > +void tpm2_flush_space(struct tpm_chip *chip, struct tpm_space > > > *space); > > > > Why the extra parameter? > > Because it was called from tpms-dev in release to flush all the > sessions still active. At that point the work space is gone, so we > have to call it with the real space. However, I realised that callsite > didn't hold the tpm_mutex like it should and that we don't need to > flush the contexts because they'll be guaranteed empty, so I added a > new function tpm2_kill_space() that does this. > > > > #endif > > > diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2 > > > -space.c > > > index 7fd2fc5..ba4310a 100644 > > > --- a/drivers/char/tpm/tpm2-space.c > > > +++ b/drivers/char/tpm/tpm2-space.c > > > @@ -59,11 +59,16 @@ static int tpm2_load_context(struct tpm_chip > > > *chip, u8 *buf, > > > > > > rc = tpm_transmit_cmd(chip, NULL, tbuf.data, PAGE_SIZE, 4, > > > TPM_TRANSMIT_UNLOCKED, NULL); > > > + > > > > cruft > > Removed. > > [...] > > > @@ -215,17 +265,20 @@ 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); > > > > For transient objects the rollback is straight forward and totally > > predictable. Given that with sessions you always keep some > > information in the TPM the rollback would be a bit more complicated. > > There is basically no rollback unless you really want to understand > what the commands did. If we get a fault on the context save or load > that isn't one we understand, like TPM_RC_HANDLE meaning the session > won't load or TPM_RC_REFERENCE_H0 meaning the session no longer exists, > then I think the only option is flushing everything. > > I'd like us to agree on a hard failure model: if we get some > unexplained error during our context loads or saves, we should clear > out the entire space (which would allow us to use pointers instead of > copyring) but we don't. So we follow the soft failure. However, > sessions will mostly fail to load after this with RPM_RC_HANDLE, so we > get the equivalent of soft failure for transients and hard failure for > sessions. > > > Now your code seems to just keep the previous session_buf, doesn't > > it? Does that always work or not? > > Yes, after tpm_flush_space, the session memory is gone and all the > sessions will refuse to load with RC_HANDLE, so we end up effectively > clearing them out. It seemed better to do it this way than to try to > special case all the session stuff. Maybe it would make sense to have a comment in code to state this? Otherwise, I'm fine with this semantics. > > PS. I have a high-level idea of attack vectors that are prevented by > > having meta-data for session inside the TPM but can you point me to > > the correct place in the TPM 2.0 specification that discusses about > > this? > > The problem is replay. If I'm snooping your TPM commands and I capture > your session context, if we don't have replay detection, I can re-load > your session HMAC and replay your command because the session has the > authorization or the encryption. The TPM designers thought the only > way to avoid replay was to use a counter which was part of the session > context which increments every time a session is saved. On loading you > check that the counters match and fail if they don't. The only way to > implement this is to keep a memory of the counter in the TPM, hence the > annoying vestigial sessions. > > It's note 2 of the architecture Part 1 guide, chapter "15.4 Session > Handles (MSO=02_16 and 03_16 )" > > James Thank you. Once these are in shape I think we have something that could be put into a release, don't you think? /Jarkko ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
On 1/26/2017 10:18 AM, James Bottomley wrote: >> >> I'm wondering if you ever need more than two sessions at once? If we >> would limit the number of sessions to that you could probably >> simplify a lot. > > Three seems to be the agreed maximum: hmac authority, parameter > encryption and command audit. 3 is recommended. There is also the possibility that the encrypt (response) and decrypt (command) parameters could use different keys, and thus need their own session. FYI: It is possible to have one session that does HMAC, parameter encryption for command and response, and audit. ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c index ed4f887..6282ad0 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); } @@ -223,6 +224,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 c48255e..b77fc60 100644 --- a/drivers/char/tpm/tpm.h +++ b/drivers/char/tpm/tpm.h @@ -159,6 +159,8 @@ enum tpm2_cc_attrs { struct tpm_space { u32 context_tbl[3]; u8 *context_buf; + u32 session_tbl[6]; + u8 *session_buf; }; enum tpm_chip_flags { @@ -588,4 +590,5 @@ 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, u32 cc, u8 *buf, size_t bufsiz); +void tpm2_flush_space(struct tpm_chip *chip, struct tpm_space *space); #endif diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c index 7fd2fc5..ba4310a 100644 --- a/drivers/char/tpm/tpm2-space.c +++ b/drivers/char/tpm/tpm2-space.c @@ -59,11 +59,16 @@ static int tpm2_load_context(struct tpm_chip *chip, u8 *buf, rc = tpm_transmit_cmd(chip, NULL, tbuf.data, PAGE_SIZE, 4, TPM_TRANSMIT_UNLOCKED, NULL); + if (rc < 0) { dev_warn(&chip->dev, "%s: failed with a system error %d\n", __func__, rc); tpm_buf_destroy(&tbuf); return -EFAULT; + } else if ((rc & TPM2_RC_HANDLE) == TPM2_RC_HANDLE || + rc == TPM2_RC_REFERENCE_H0) { + 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); @@ -116,21 +121,44 @@ 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 void tpm2_flush_space(struct tpm_chip *chip) +static int tpm2_session_add(struct tpm_chip *chip, + struct tpm_space *space, u32 handle) +{ + 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; +} + +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->session_tbl); i++) { + if (space->session_tbl[i]) + tpm2_flush_context_cmd(chip, space->session_tbl[i], + TPM_TRANSMIT_UNLOCKED); + } } static int tpm2_load_space(struct tpm_chip *chip) @@ -156,6 +184,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, space); + return rc; + } + if (handle != space->session_tbl[i]) { + dev_warn(&chip->dev, "session restored to wrong handle\n"); + tpm2_flush_space(chip, space); + return -EFAULT; + } + } + return 0; } @@ -215,17 +265,20 @@ 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) { - tpm2_flush_space(chip); + tpm2_flush_space(chip, &chip->work_space); return rc; } rc = tpm2_map_command(chip, cc, cmd); if (rc) { - tpm2_flush_space(chip); + tpm2_flush_space(chip, &chip->work_space); return rc; } @@ -235,7 +288,7 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc, 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]); @@ -254,9 +307,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, space, phandle); + /* Garbage collect a dead context. */ for (i = 0; i < ARRAY_SIZE(space->context_tbl); i++) { if (space->context_tbl[i] == phandle) { @@ -302,9 +361,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, space); + return rc; + } + } + return 0; } @@ -318,19 +396,22 @@ int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space, rc = tpm2_map_response(chip, cc, buf, bufsiz); if (rc) { - tpm2_flush_space(chip); + tpm2_flush_space(chip, space); return rc; } rc = tpm2_save_space(chip); if (rc) { - tpm2_flush_space(chip); + tpm2_flush_space(chip, space); return rc; } 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 2ac2537..6efead3 100644 --- a/drivers/char/tpm/tpms-dev.c +++ b/drivers/char/tpm/tpms-dev.c @@ -27,6 +27,12 @@ static int tpms_open(struct inode *inode, struct file *file) kfree(priv); return -ENOMEM; } + priv->space.session_buf = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (priv->space.session_buf == NULL) { + kfree(priv->space.context_buf); + kfree(priv); + return -ENOMEM; + } tpm_common_open(file, chip, &priv->priv); @@ -38,8 +44,10 @@ 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->space.context_buf); + kfree(priv->space.session_buf); kfree(priv); return 0;
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. Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> --- drivers/char/tpm/tpm-chip.c | 6 +++ drivers/char/tpm/tpm.h | 3 ++ drivers/char/tpm/tpm2-space.c | 99 +++++++++++++++++++++++++++++++++++++++---- drivers/char/tpm/tpms-dev.c | 8 ++++ 4 files changed, 107 insertions(+), 9 deletions(-)