| Message ID | 1484752186.2717.16.camel@HansenPartnership.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Jan 18, 2017 at 10:09:46AM -0500, James Bottomley wrote: > sessions should be isolated during each instance of a tpm space. This > means that spaces shouldn't be able to see each other's sessions and > also when a space is closed, all the sessions belonging to it should > be flushed. > > This is implemented by adding a session_tbl to the space to track the > created session handles. Sessions can be flushed either by not > setting the continueSession attribute in the session table or by an > explicit flush. In the first case we have to mark the session as > being ready to flush and explicitly forget it if the command completes > successfully and in the second case we have to intercept the flush > instruction and clear the session from our table. You could do this without these nasty corner cases by arbage collecting when a command emits a new session handle. When a session handle is created check if any of the spaces contain it and remove from the array. No special cases needed. This will render the need to do any kind of interception whatsoever unneeded. /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-19 at 13:58 +0200, Jarkko Sakkinen wrote: > On Wed, Jan 18, 2017 at 10:09:46AM -0500, James Bottomley wrote: > > sessions should be isolated during each instance of a tpm space. > > This means that spaces shouldn't be able to see each other's > > sessions and also when a space is closed, all the sessions > > belonging to it should be flushed. > > > > This is implemented by adding a session_tbl to the space to track > > the created session handles. Sessions can be flushed either by not > > setting the continueSession attribute in the session table or by an > > explicit flush. In the first case we have to mark the session as > > being ready to flush and explicitly forget it if the command > > completes successfully and in the second case we have to intercept > > the flush instruction and clear the session from our table. > > You could do this without these nasty corner cases by arbage > collecting when a command emits a new session handle. I could for this patch set. However, the global session accounting RFC requires strict accounting, because it needs to know exactly when to retry a command that failed because we were out of sessions and because we don't want to needlessly evict a session if there was one available which we didn't see because of lazy accounting. It would be a lot of churn to do it lazily in this patch set and then switch to strict in that one, so I chose to account sessions strictly always. > When a session handle is created check if any of the spaces contain > it and remove from the array. No special cases needed. > > This will render the need to do any kind of interception whatsoever > unneeded. It can be done either way for these two patches, but I think, for the reason above, it should begin life as it will go on (i.e. strict accounting). It's not that much extra code to do it and it's easier to follow the flow of the three patches. 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, Jan 19, 2017 at 07:11:23AM -0500, James Bottomley wrote: > On Thu, 2017-01-19 at 13:58 +0200, Jarkko Sakkinen wrote: > > On Wed, Jan 18, 2017 at 10:09:46AM -0500, James Bottomley wrote: > > > sessions should be isolated during each instance of a tpm space. > > > This means that spaces shouldn't be able to see each other's > > > sessions and also when a space is closed, all the sessions > > > belonging to it should be flushed. > > > > > > This is implemented by adding a session_tbl to the space to track > > > the created session handles. Sessions can be flushed either by not > > > setting the continueSession attribute in the session table or by an > > > explicit flush. In the first case we have to mark the session as > > > being ready to flush and explicitly forget it if the command > > > completes successfully and in the second case we have to intercept > > > the flush instruction and clear the session from our table. > > > > You could do this without these nasty corner cases by arbage > > collecting when a command emits a new session handle. > > I could for this patch set. However, the global session accounting RFC > requires strict accounting, because it needs to know exactly when to > retry a command that failed because we were out of sessions and because > we don't want to needlessly evict a session if there was one available > which we didn't see because of lazy accounting. It would be a lot of > churn to do it lazily in this patch set and then switch to strict in > that one, so I chose to account sessions strictly always. Lazy is kind of ambiguous word so I'll have to check that we have same definition for it in this context. I'm talking about not trying to detect if something gets deleted. When something gets created you would go through the global list of sessions and check if it is used. If so, it must be that the session was deleted at some point. Your argument is that in this scheme sometimes there might be a session marked as "reserved" but it is in fact free. This might lead to useless eviction. Am I correct? My argument is that the lazy scheme is more generic (does not require special cases). As a subsystem maintainer I tend to be more fond of that kind of solutions. Having special cases raises questios like (for example): 1. What if standard gets added something that does not fall into the current set of special cases? You never know. 2. What about vendor specific commands? The lazy scheme is compatible with them. The standard does not put any kind of constraints for vendor specific commands. You could solve the problem you are stating by getting the full the list of alive sessions with CAP_HANDLES and mark dangling sessions as free. PS. I've started to think that maybe also with sessions it is better to have just one change that implements full eviction like we have for transient objects after seeing your breakdown. I'm sorry about putting you extra trouble doing the isolation only patch. It's better to do this right once... /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/20/2017 8:23 AM, Jarkko Sakkinen wrote: > > I'm talking about not trying to detect if something gets deleted. When > something gets created you would go through the global list of sessions > and check if it is used. If so, it must be that the session was deleted > at some point. Are you saying that, when a process flushes (or continue = false) a session, this code won't actually flush the context? You'll wait until another startauthsession creates a handle, and then delete other occurrences of it? If so, I wonder about this: A process creates, uses, and flushes a session. It then tries to use the session again. It should fail, but will it with this design? There may also be a security reason that an application expects the context to be flushed when requested. E.g., it could have a satisfied policy. ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
On Fri, 2017-01-20 at 15:23 +0200, Jarkko Sakkinen wrote: > On Thu, Jan 19, 2017 at 07:11:23AM -0500, James Bottomley wrote: > > On Thu, 2017-01-19 at 13:58 +0200, Jarkko Sakkinen wrote: > > > On Wed, Jan 18, 2017 at 10:09:46AM -0500, James Bottomley wrote: > > > > sessions should be isolated during each instance of a tpm > > > > space. This means that spaces shouldn't be able to see each > > > > other's sessions and also when a space is closed, all the > > > > sessions belonging to it should be flushed. > > > > > > > > This is implemented by adding a session_tbl to the space to > > > > track the created session handles. Sessions can be flushed > > > > either by not setting the continueSession attribute in the > > > > session table or by an explicit flush. In the first case we > > > > have to mark the session as being ready to flush and explicitly > > > > forget it if the command completes successfully and in the > > > > second case we have to intercept the flush instruction and > > > > clear the session from our table. > > > > > > You could do this without these nasty corner cases by arbage > > > collecting when a command emits a new session handle. > > > > I could for this patch set. However, the global session accounting > > RFC requires strict accounting, because it needs to know exactly > > when to retry a command that failed because we were out of sessions > > and because we don't want to needlessly evict a session if there > > was one available which we didn't see because of lazy accounting. > > It would be a lot of churn to do it lazily in this patch set and > > then switch to strict in that one, so I chose to account sessions > > strictly always. > > Lazy is kind of ambiguous word so I'll have to check that we have > same definition for it in this context. > > I'm talking about not trying to detect if something gets deleted. > When something gets created you would go through the global list of > sessions and check if it is used. If so, it must be that the session > was deleted at some point. That's my terminology too. We're talking about lazy and strict tracking of session flushing. > Your argument is that in this scheme sometimes there might be a > session marked as "reserved" but it is in fact free. This might lead > to useless eviction. Am I correct? Yes, but not just that, it will also lead to over long waits because we can no longer wake the waiters the moment a session becomes free. > My argument is that the lazy scheme is more generic (does not require > special cases). As a subsystem maintainer I tend to be more fond of > that kind of solutions. Having special cases raises questios like > (for example): > > 1. What if standard gets added something that does not fall into the > current set of special cases? You never know. > 2. What about vendor specific commands? The lazy scheme is compatible > with them. The standard does not put any kind of constraints for > vendor specific commands. We rely on the assertion in the Manual that sessions are only returned in the handle area (as we do for objects). We also rely on the guarantee that they're only destroyed by flush or continueSession being 0 in the session attributes. If some mad vendor introduces a command that creates an object and doesn't return it in the handle area, we'll get screwed for both transient objects and sessions. Sessions also could have issues if some mad vendor creates a command that flushes them outside of the above description. That's why the standard has all these caveats about handle and session creation. In theory the vendors are not allowed to violate them in their own commands ... > You could solve the problem you are stating by getting the full the > list of alive sessions with CAP_HANDLES and mark dangling sessions > as free. That's a command which produces a huge output ... I'd have to do it at the end of every input command to get the list of current handles ... I don't really think it's a better solution. Let me describe the failure case with strict destruction accounting: supposing a mad vendor does introduce a command that flushes a session outside the standards prescribed way. What happens is that it gets re -used before the TPM exhausts handles, and tpm2_session_chip_add will simply to replace what it currently has. The only consequence is a single missed wakeup. So even in the face of vendor failure, this scheme will work almost all of the time and it will always work better than a lazy scheme because the failure case gives us properties identical to the lazy case. To make this case iron, we should get a failure on context save, so I can use that failure to drop the handle and I think the strict scheme will then always perform better than the lazy scheme (let's call it strict with lazy backup) will that suffice? > PS. I've started to think that maybe also with sessions it is better > to have just one change that implements full eviction like we have > for transient objects after seeing your breakdown. I'm sorry about > putting you extra trouble doing the isolation only patch. It's better > to do this right once... Well, I can put them back together again, but you could just apply them together as two patches ... they are now bisectable. 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 Fri, Jan 20, 2017 at 09:39:13AM -0500, James Bottomley wrote: > On Fri, 2017-01-20 at 15:23 +0200, Jarkko Sakkinen wrote: > > On Thu, Jan 19, 2017 at 07:11:23AM -0500, James Bottomley wrote: > > > On Thu, 2017-01-19 at 13:58 +0200, Jarkko Sakkinen wrote: > > > > On Wed, Jan 18, 2017 at 10:09:46AM -0500, James Bottomley wrote: > > > > > sessions should be isolated during each instance of a tpm > > > > > space. This means that spaces shouldn't be able to see each > > > > > other's sessions and also when a space is closed, all the > > > > > sessions belonging to it should be flushed. > > > > > > > > > > This is implemented by adding a session_tbl to the space to > > > > > track the created session handles. Sessions can be flushed > > > > > either by not setting the continueSession attribute in the > > > > > session table or by an explicit flush. In the first case we > > > > > have to mark the session as being ready to flush and explicitly > > > > > forget it if the command completes successfully and in the > > > > > second case we have to intercept the flush instruction and > > > > > clear the session from our table. > > > > > > > > You could do this without these nasty corner cases by arbage > > > > collecting when a command emits a new session handle. > > > > > > I could for this patch set. However, the global session accounting > > > RFC requires strict accounting, because it needs to know exactly > > > when to retry a command that failed because we were out of sessions > > > and because we don't want to needlessly evict a session if there > > > was one available which we didn't see because of lazy accounting. > > > It would be a lot of churn to do it lazily in this patch set and > > > then switch to strict in that one, so I chose to account sessions > > > strictly always. > > > > Lazy is kind of ambiguous word so I'll have to check that we have > > same definition for it in this context. > > > > I'm talking about not trying to detect if something gets deleted. > > When something gets created you would go through the global list of > > sessions and check if it is used. If so, it must be that the session > > was deleted at some point. > > That's my terminology too. We're talking about lazy and strict > tracking of session flushing. > > > Your argument is that in this scheme sometimes there might be a > > session marked as "reserved" but it is in fact free. This might lead > > to useless eviction. Am I correct? > > Yes, but not just that, it will also lead to over long waits because we > can no longer wake the waiters the moment a session becomes free. > > > My argument is that the lazy scheme is more generic (does not require > > special cases). As a subsystem maintainer I tend to be more fond of > > that kind of solutions. Having special cases raises questios like > > (for example): > > > > 1. What if standard gets added something that does not fall into the > > current set of special cases? You never know. > > 2. What about vendor specific commands? The lazy scheme is compatible > > with them. The standard does not put any kind of constraints for > > vendor specific commands. > > We rely on the assertion in the Manual that sessions are only returned > in the handle area (as we do for objects). We also rely on the > guarantee that they're only destroyed by flush or continueSession being > 0 in the session attributes. > > If some mad vendor introduces a command that creates an object and > doesn't return it in the handle area, we'll get screwed for both > transient objects and sessions. Sessions also could have issues if > some mad vendor creates a command that flushes them outside of the > above description. That's why the standard has all these caveats about > handle and session creation. In theory the vendors are not allowed to > violate them in their own commands ... > > > You could solve the problem you are stating by getting the full the > > list of alive sessions with CAP_HANDLES and mark dangling sessions > > as free. > > That's a command which produces a huge output ... I'd have to do it at > the end of every input command to get the list of current handles ... I > don't really think it's a better solution. > > Let me describe the failure case with strict destruction accounting: > supposing a mad vendor does introduce a command that flushes a session > outside the standards prescribed way. What happens is that it gets re > -used before the TPM exhausts handles, and tpm2_session_chip_add will > simply to replace what it currently has. The only consequence is a > single missed wakeup. So even in the face of vendor failure, this > scheme will work almost all of the time and it will always work better > than a lazy scheme because the failure case gives us properties > identical to the lazy case. To make this case iron, we should get a > failure on context save, so I can use that failure to drop the handle > and I think the strict scheme will then always perform better than the > lazy scheme (let's call it strict with lazy backup) will that suffice? > > > PS. I've started to think that maybe also with sessions it is better > > to have just one change that implements full eviction like we have > > for transient objects after seeing your breakdown. I'm sorry about > > putting you extra trouble doing the isolation only patch. It's better > > to do this right once... > > Well, I can put them back together again, but you could just apply them > together as two patches ... they are now bisectable. Sure forgot this last comment. It's really irrelevant. I'll reply properly later on. > 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 Fri, Jan 20, 2017 at 09:27:25AM -0500, Ken Goldman wrote: > On 1/20/2017 8:23 AM, Jarkko Sakkinen wrote: > > > > I'm talking about not trying to detect if something gets deleted. When > > something gets created you would go through the global list of sessions > > and check if it is used. If so, it must be that the session was deleted > > at some point. > > Are you saying that, when a process flushes (or continue = false) a > session, this code won't actually flush the context? You'll wait until > another startauthsession creates a handle, and then delete other > occurrences of it? I just wouldn't get care if a session gets deleted. You can detect it postmortem when something gets created with the same handle. /Jarkko ------------------------------------------------------------------------------ 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.h b/drivers/char/tpm/tpm.h index 3346c48..265b7f5 100644 --- a/drivers/char/tpm/tpm.h +++ b/drivers/char/tpm/tpm.h @@ -158,6 +158,7 @@ enum tpm2_cc_attrs { struct tpm_space { u32 context_tbl[14]; u8 *context_buf; + u32 session_tbl[6]; }; enum tpm_chip_flags { @@ -584,4 +585,5 @@ 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); #endif diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c index 3708e70..49048af 100644 --- a/drivers/char/tpm/tpm2-space.c +++ b/drivers/char/tpm/tpm2-space.c @@ -25,15 +25,83 @@ enum tpm2_handle_types { TPM2_HT_TRANSIENT = 0x80000000, }; -static void tpm2_flush_space(struct tpm_chip *chip) +#define TPM2_HT_TAG_FOR_FLUSH 0xF0000000 + +static int tpm2_session_find(struct tpm_space *space, u32 handle) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(space->session_tbl); i++) + if (handle == space->session_tbl[i]) + break; + if (i == ARRAY_SIZE(space->session_tbl)) + return -1; + return i; +} + +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; +} + +/* if a space is active, emulate some commands */ +static int tpm2_intercept(struct tpm_chip *chip, struct tpm_space *space, + u32 cc, u8 *buf, size_t bufsiz) +{ + int j; + u32 handle, handle_type; + + if (!space) + return 0; + + if (cc != TPM2_CC_FLUSH_CONTEXT) + return 0; + handle = get_unaligned_be32((__be32 *)&buf[10]); + handle_type = (handle & 0xFF000000); + + if (handle_type != TPM2_HT_HMAC_SESSION && + handle_type != TPM2_HT_POLICY_SESSION) + /* let the TPM figure out and return the error */ + return 0; + + j = tpm2_session_find(space, handle); + if (j < 0) + return -EINVAL; + + space->session_tbl[j] |= TPM2_HT_TAG_FOR_FLUSH; + + 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++) { + space->session_tbl[i] &= ~TPM2_HT_TAG_FOR_FLUSH; + if (space->session_tbl[i]) + tpm2_flush_context_cmd(chip, space->session_tbl[i], + TPM_TRANSMIT_UNLOCKED); + } } struct tpm2_context { @@ -94,10 +162,82 @@ 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->session_tbl); i++) { + if ((space->session_tbl[i] & TPM2_HT_TAG_FOR_FLUSH) != + TPM2_HT_TAG_FOR_FLUSH) + continue; + if (rc == TPM2_RC_SUCCESS) + space->session_tbl[i] = 0; + else + /* for unsuccessful command, keep session */ + space->session_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; + u8 attr; + int j; + u32 handle, handle_type; + + /* TPMI_SH_AUTH_SESSION */ + handle = get_unaligned_be32((__be32 *)&buf[i]); + handle_type = handle & 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 = tpm2_session_find(space, handle); + if (j < 0) + return -EINVAL; + if ((attr & 1) == 0) + /* session is flushed by the command */ + space->session_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; @@ -105,6 +245,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; @@ -132,11 +273,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; } @@ -150,8 +294,14 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, 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); + rc = tpm2_intercept(chip, space, cc, buf, bufsiz); + if (rc) + return rc; + rc = tpm2_load_space(chip); if (rc) return rc; @@ -166,13 +316,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; @@ -188,9 +342,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) { @@ -217,7 +377,7 @@ static int tpm2_map_response(struct tpm_chip *chip, u32 cc, u8 *rsp, size_t len) return 0; out_err: - tpm2_flush_space(chip); + tpm2_flush_space(chip, space); return rc; } @@ -277,7 +437,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; } @@ -299,6 +459,8 @@ 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); return 0; diff --git a/drivers/char/tpm/tpms-dev.c b/drivers/char/tpm/tpms-dev.c index 6bb687f..d6e3491 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->space.context_buf); kfree(priv);
sessions should be isolated during each instance of a tpm space. This means that spaces shouldn't be able to see each other's sessions and also when a space is closed, all the sessions belonging to it should be flushed. This is implemented by adding a session_tbl to the space to track the created session handles. Sessions can be flushed either by not setting the continueSession attribute in the session table or by an explicit flush. In the first case we have to mark the session as being ready to flush and explicitly forget it if the command completes successfully and in the second case we have to intercept the flush instruction and clear the session from our table. Finally, when the device handling the space is closed, we have to send explicit flushes to all the remaining sessions belonging to the space to ensure they are cleared out. Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> --- drivers/char/tpm/tpm.h | 2 + drivers/char/tpm/tpm2-space.c | 178 ++++++++++++++++++++++++++++++++++++++++-- drivers/char/tpm/tpms-dev.c | 1 + 3 files changed, 173 insertions(+), 8 deletions(-)