Change in coreboot[master]: security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX

[Shelley Chen (Code Review)]

Shelley Chen has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46511 )

Change subject: security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX
......................................................................

security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX

Add new index for MRC_CACHE data in RW.  Also update antirollback
functions to handle this new index where necessary.

BUG=b:150502246
BRANCH=None
TEST=make sure memory training still works on nami

Change-Id: I2de3c23aa56d3b576ca54dbd85c75e5b80199560
Signed-off-by: Shelley Chen 
---
M src/security/vboot/antirollback.h
M src/security/vboot/mrc_cache_hash_tpm.c
2 files changed, 7 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/46511/1

diff --git a/src/security/vboot/antirollback.h b/src/security/vboot/antirollback.h
index 516c23e..d1309d9 100644
--- a/src/security/vboot/antirollback.h
+++ b/src/security/vboot/antirollback.h
@@ -24,6 +24,9 @@
 #define FWMP_NV_INDEX                   0x100a
 /* 0x100b: Hash of MRC_CACHE training data for recovery boot */
 #define MRC_REC_HASH_NV_INDEX           0x100b
+/* 0x100c: OOBE autoconfig public key hashes */
+/* 0x100d: Hash of MRC_CACHE training data for non-recovery boot */
+#define MRC_RW_HASH_NV_INDEX            0x100d
 #define HASH_NV_SIZE                    VB2_SHA256_DIGEST_SIZE

/* Structure definitions for TPM spaces */
diff --git a/src/security/vboot/mrc_cache_hash_tpm.c b/src/security/vboot/mrc_cache_hash_tpm.c
index ea67dda..144b51c 100644
--- a/src/security/vboot/mrc_cache_hash_tpm.c
+++ b/src/security/vboot/mrc_cache_hash_tpm.c
@@ -23,7 +23,8 @@
    	0xba, 0xad, 0xba, 0xad, /* BAADBAAD */
    };
    const uint8_t *hash_ptr = data_hash;
-	uint32_t hash_idx = MRC_REC_HASH_NV_INDEX;
+	uint32_t hash_idx = vboot_recovery_mode_enabled() ?
+		MRC_REC_HASH_NV_INDEX : MRC_RW_HASH_NV_INDEX;

/* Initialize TPM driver. */
    if (tlcl_lib_init() != VB2_SUCCESS) {
@@ -60,7 +61,8 @@
 {
    uint8_t data_hash[VB2_SHA256_DIGEST_SIZE];
    uint8_t tpm_hash[VB2_SHA256_DIGEST_SIZE];
-	uint32_t hash_idx = MRC_REC_HASH_NV_INDEX;
+	uint32_t hash_idx = vboot_recovery_mode_enabled() ?
+		MRC_REC_HASH_NV_INDEX : MRC_RW_HASH_NV_INDEX;

/* Calculate hash of data read from MRC_CACHE. */
    if (vb2_digest_buffer(data, size, VB2_HASH_SHA256, data_hash,

[Shelley Chen (Code Review)]

Hello Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/46511

to look at the new patch set (#2).

Change subject: security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX
......................................................................

security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX

Add new index for MRC_CACHE data in RW.  Also update antirollback
functions to handle this new index where necessary.

BUG=b:150502246
BRANCH=None
TEST=make sure memory training still works on nami

Change-Id: I2de3c23aa56d3b576ca54dbd85c75e5b80199560
Signed-off-by: Shelley Chen 
---
M src/security/vboot/Makefile.inc
M src/security/vboot/antirollback.h
M src/security/vboot/mrc_cache_hash_tpm.c
3 files changed, 8 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/46511/2

[Shelley Chen (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/46511

to look at the new patch set (#6).

Change subject: security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX
......................................................................

security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX

Add new index for MRC_CACHE data in RW.  Also update antirollback
functions to handle this new index where necessary.

BUG=b:150502246
BRANCH=None
TEST=make sure memory training still works on nami

Change-Id: I2de3c23aa56d3b576ca54dbd85c75e5b80199560
Signed-off-by: Shelley Chen 
---
M src/security/vboot/Makefile.inc
M src/security/vboot/antirollback.h
M src/security/vboot/mrc_cache_hash_tpm.c
M src/security/vboot/secdata_tpm.c
4 files changed, 12 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/46511/6

[Shelley Chen (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/46511

to look at the new patch set (#7).

Change subject: security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX
......................................................................

security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX

Add new index for MRC_CACHE data in RW.  Also update antirollback
functions to handle this new index where necessary.

BUG=b:150502246
BRANCH=None
TEST=make sure memory training still works on nami

Change-Id: I2de3c23aa56d3b576ca54dbd85c75e5b80199560
Signed-off-by: Shelley Chen 
---
M src/security/vboot/Makefile.inc
M src/security/vboot/antirollback.h
M src/security/vboot/mrc_cache_hash_tpm.c
M src/security/vboot/secdata_tpm.c
4 files changed, 12 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/46511/7

[Shelley Chen (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/46511

to look at the new patch set (#8).

Change subject: security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX
......................................................................

security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX

Add new index for MRC_CACHE data in RW.  Also update antirollback
functions to handle this new index where necessary.

BUG=b:150502246
BRANCH=None
TEST=make sure memory training still works on nami

Change-Id: I2de3c23aa56d3b576ca54dbd85c75e5b80199560
Signed-off-by: Shelley Chen 
---
M src/security/vboot/antirollback.h
M src/security/vboot/mrc_cache_hash_tpm.c
M src/security/vboot/secdata_tpm.c
3 files changed, 23 insertions(+), 13 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/46511/8

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46511 )

Change subject: security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX
......................................................................

Patch Set 8:

(1 comment)

https://review.coreboot.org/c/coreboot/+/46511/8/src/security/vboot/secdata_... 
File src/security/vboot/secdata_tpm.c:

https://review.coreboot.org/c/coreboot/+/46511/8/src/security/vboot/secdata_... 
PS8, Line 194: 		RETURN_ON_FAILURE(set_mrc_hash_space(MRC_REC_HASH_NV_INDEX, mrc_hash_data));
First of all, this still says REC_HASH.

Second, I'm not sure this is really necessary. The space gets automatically created on first use anyway. The reason we still do this here for the recovery space, I think, is to make sure the space is created at some point (with the right permissions) so no attacker gets a chance to create the same space with weaker permissions before we ever did. But for normal mode, I don't think that's a concern (we're not sure when the device will first boot in recovery mode, that might be a long time after shipping, but we can be sure it was booted at least once in normal mode before anything bad could happen).

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46511 )

Change subject: security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX
......................................................................

Patch Set 9:

(2 comments)

https://review.coreboot.org/c/coreboot/+/46511/7/src/security/vboot/mrc_cach... 
File src/security/vboot/mrc_cache_hash_tpm.c:

https://review.coreboot.org/c/coreboot/+/46511/7/src/security/vboot/mrc_cach... 
PS7, Line 26: uint32_t hash_idx = vboot_recovery_mode_enabled() ?
            : 		MRC_REC_HASH_NV_INDEX : MRC_RW_HASH_NV_INDEX;
This should happen in mrc_cache.c.

https://review.coreboot.org/c/coreboot/+/46511/8/src/security/vboot/secdata_... 
File src/security/vboot/secdata_tpm.c:

https://review.coreboot.org/c/coreboot/+/46511/8/src/security/vboot/secdata_... 
PS8, Line 194: 		RETURN_ON_FAILURE(set_mrc_hash_space(MRC_REC_HASH_NV_INDEX, mrc_hash_data));
> First of all, this still says REC_HASH. […]

Agreed. I think it is fine to not create this space until really required which would be in ramstage/romstage in the same boot. My question here https://review.coreboot.org/c/coreboot/+/46432/4/src/security/vboot/secdata_... was just to ensure my understanding matched the intent. We can potentially add a comment in factory_initialize_tpm to indicate why the creation is being skipped here if that is helpful.

[Shelley Chen (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/46511

to look at the new patch set (#11).

Change subject: security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX
......................................................................

security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX

Add new index for MRC_CACHE data in RW.  Also update antirollback
functions to handle this new index where necessary.

BUG=b:150502246
BRANCH=None
TEST=make sure memory training still works on nami

Change-Id: I2de3c23aa56d3b576ca54dbd85c75e5b80199560
Signed-off-by: Shelley Chen 
---
M src/security/vboot/antirollback.h
M src/security/vboot/mrc_cache_hash_tpm.c
M src/security/vboot/secdata_tpm.c
3 files changed, 41 insertions(+), 18 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/46511/11

[Shelley Chen (Code Review)]

Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46511 )

Change subject: security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX
......................................................................

Patch Set 11:

(1 comment)

https://review.coreboot.org/c/coreboot/+/46511/8/src/security/vboot/secdata_... 
File src/security/vboot/secdata_tpm.c:

https://review.coreboot.org/c/coreboot/+/46511/8/src/security/vboot/secdata_... 
PS8, Line 194: 		RETURN_ON_FAILURE(set_mrc_hash_space(MRC_REC_HASH_NV_INDEX, mrc_hash_data));
> Agreed. […]

Done

[Shelley Chen (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/46511

to look at the new patch set (#12).

Change subject: security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX
......................................................................

security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX

Add new index for MRC_CACHE data in RW.  Also update antirollback
functions to handle this new index where necessary.

BUG=b:150502246
BRANCH=None
TEST=make sure memory training still works on nami

Change-Id: I2de3c23aa56d3b576ca54dbd85c75e5b80199560
Signed-off-by: Shelley Chen 
---
M src/security/vboot/antirollback.h
M src/security/vboot/mrc_cache_hash_tpm.c
M src/security/vboot/secdata_tpm.c
3 files changed, 41 insertions(+), 17 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/46511/12

[Shelley Chen (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/46511

to look at the new patch set (#13).

Change subject: security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX
......................................................................

security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX

Add new index for MRC_CACHE data in RW.  Also update antirollback
functions to handle this new index where necessary.

BUG=b:150502246
BRANCH=None
TEST=make sure memory training still works on nami

Change-Id: I2de3c23aa56d3b576ca54dbd85c75e5b80199560
Signed-off-by: Shelley Chen 
---
M src/drivers/mrc_cache/mrc_cache.c
M src/security/vboot/antirollback.h
M src/security/vboot/mrc_cache_hash_tpm.c
M src/security/vboot/secdata_tpm.c
4 files changed, 41 insertions(+), 17 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/46511/13

[Shelley Chen (Code Review)]

Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46511 )

Change subject: security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX
......................................................................

Patch Set 13:

(1 comment)

https://review.coreboot.org/c/coreboot/+/46511/7/src/security/vboot/mrc_cach... 
File src/security/vboot/mrc_cache_hash_tpm.c:

https://review.coreboot.org/c/coreboot/+/46511/7/src/security/vboot/mrc_cach... 
PS7, Line 26: uint32_t hash_idx = vboot_recovery_mode_enabled() ?
            : 		MRC_REC_HASH_NV_INDEX : MRC_RW_HASH_NV_INDEX;
> This should happen in mrc_cache.c.

Done

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46511 )

Change subject: security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX
......................................................................

Patch Set 13: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/46511/13/src/security/vboot/secdata... 
File src/security/vboot/secdata_tpm.c:

https://review.coreboot.org/c/coreboot/+/46511/13/src/security/vboot/secdata... 
PS13, Line 324: 	 * our hands.
Can you also remove all this TPM1 stuff concerning the MRC hash, now that we have forbidden that anyway (can be a later patch if you want)?

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46511 )

Change subject: security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX
......................................................................

Patch Set 13:

(4 comments)

https://review.coreboot.org/c/coreboot/+/46511/13/src/drivers/mrc_cache/mrc_... 
File src/drivers/mrc_cache/mrc_cache.c:

https://review.coreboot.org/c/coreboot/+/46511/13/src/drivers/mrc_cache/mrc_... 
PS13, Line 183: vboot_recovery_mode_enabled() ?
              : 		MRC_REC_HASH_NV_INDEX : MRC_RW_HASH_NV_INDEX
This should still be kept as MRC_REC_HASH_NV_INDEX. In the next CL, you add tpm_hash_index in the cache_region which takes care of handling recovery and non-recovery cases. The changes in this file and in mrc_cache_hash_tpm.c should be added to the next CL i.e. supporting tpm space for MRC hash in recovery and non-recovery modes. (Else, this CL on its own will break the variable data part for APL/GLK).

https://review.coreboot.org/c/coreboot/+/46511/13/src/security/vboot/antirol... 
File src/security/vboot/antirollback.h:

https://review.coreboot.org/c/coreboot/+/46511/13/src/security/vboot/antirol... 
PS13, Line 63: recovery
s/recovery/MRC?

https://review.coreboot.org/c/coreboot/+/46511/13/src/security/vboot/antirol... 
PS13, Line 73: recovery
s/recovery/MRC hash?

https://review.coreboot.org/c/coreboot/+/46511/13/src/security/vboot/antirol... 
PS13, Line 84: recovery
s/recovery/MRC?

[Shelley Chen (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Julius Werner, Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/46511

to look at the new patch set (#14).

Change subject: security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX
......................................................................

security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX

Add new index for MRC_CACHE data in RW.  Also update antirollback
functions to handle this new index where necessary.

BUG=b:150502246
BRANCH=None
TEST=make sure memory training still works on nami

Change-Id: I2de3c23aa56d3b576ca54dbd85c75e5b80199560
Signed-off-by: Shelley Chen 
---
M src/security/vboot/antirollback.h
M src/security/vboot/secdata_tpm.c
2 files changed, 40 insertions(+), 10 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/46511/14

[Shelley Chen (Code Review)]

Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46511 )

Change subject: security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX
......................................................................

Patch Set 14:

(5 comments)

https://review.coreboot.org/c/coreboot/+/46511/13/src/drivers/mrc_cache/mrc_... 
File src/drivers/mrc_cache/mrc_cache.c:

https://review.coreboot.org/c/coreboot/+/46511/13/src/drivers/mrc_cache/mrc_... 
PS13, Line 183: vboot_recovery_mode_enabled() ?
              : 		MRC_REC_HASH_NV_INDEX : MRC_RW_HASH_NV_INDEX
> This should still be kept as MRC_REC_HASH_NV_INDEX. […]

Done

https://review.coreboot.org/c/coreboot/+/46511/13/src/security/vboot/antirol... 
File src/security/vboot/antirollback.h:

https://review.coreboot.org/c/coreboot/+/46511/13/src/security/vboot/antirol... 
PS13, Line 63: recovery
> s/recovery/MRC?

Done

https://review.coreboot.org/c/coreboot/+/46511/13/src/security/vboot/antirol... 
PS13, Line 73: recovery
> s/recovery/MRC hash?

Done

https://review.coreboot.org/c/coreboot/+/46511/13/src/security/vboot/antirol... 
PS13, Line 84: recovery
> s/recovery/MRC?

Done

https://review.coreboot.org/c/coreboot/+/46511/13/src/security/vboot/secdata... 
File src/security/vboot/secdata_tpm.c:

https://review.coreboot.org/c/coreboot/+/46511/13/src/security/vboot/secdata... 
PS13, Line 324: 	 * our hands.
> Can you also remove all this TPM1 stuff concerning the MRC hash, now that we have forbidden that any […]

Ok, I think that it was CL-8 in the list that Furquan described here: https://review.coreboot.org/c/coreboot/+/46432/1/src/security/vboot/mrc_cach...

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46511 )

Change subject: security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX
......................................................................

Patch Set 14: Code-Review+2

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46511 )

Change subject: security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX
......................................................................

Patch Set 14:

(1 comment)

https://review.coreboot.org/c/coreboot/+/46511/13/src/security/vboot/secdata... 
File src/security/vboot/secdata_tpm.c:

https://review.coreboot.org/c/coreboot/+/46511/13/src/security/vboot/secdata... 
PS13, Line 324: 	 * our hands.
> Ok, I think that it was CL-8 in the list that Furquan described here: https://review.coreboot. […]

That's right.

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46511 )

Change subject: security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX
......................................................................

Patch Set 14:

(1 comment)

https://review.coreboot.org/c/coreboot/+/46511/13/src/security/vboot/secdata... 
File src/security/vboot/secdata_tpm.c:

https://review.coreboot.org/c/coreboot/+/46511/13/src/security/vboot/secdata... 
PS13, Line 324: 	 * our hands.
> That's right.

Ack

[Julius Werner (Code Review)]

Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46511 )

Change subject: security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX
......................................................................

security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEX

Add new index for MRC_CACHE data in RW.  Also update antirollback
functions to handle this new index where necessary.

BUG=b:150502246
BRANCH=None
TEST=make sure memory training still works on nami

Change-Id: I2de3c23aa56d3b576ca54dbd85c75e5b80199560
Signed-off-by: Shelley Chen 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46511
Tested-by: build bot (Jenkins) 
Reviewed-by: Furquan Shaikh 
---
M src/security/vboot/antirollback.h
M src/security/vboot/secdata_tpm.c
2 files changed, 40 insertions(+), 10 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Furquan Shaikh: Looks good to me, approved

diff --git a/src/security/vboot/antirollback.h b/src/security/vboot/antirollback.h
index 8b183da..fcfa7a2 100644
--- a/src/security/vboot/antirollback.h
+++ b/src/security/vboot/antirollback.h
@@ -24,6 +24,9 @@
 #define FWMP_NV_INDEX                   0x100a
 /* 0x100b: Hash of MRC_CACHE training data for recovery boot */
 #define MRC_REC_HASH_NV_INDEX           0x100b
+/* 0x100c: OOBE autoconfig public key hashes */
+/* 0x100d: Hash of MRC_CACHE training data for non-recovery boot */
+#define MRC_RW_HASH_NV_INDEX            0x100d
 #define HASH_NV_SIZE                    VB2_SHA256_DIGEST_SIZE

/* Structure definitions for TPM spaces */
@@ -57,23 +60,32 @@
 uint32_t antirollback_lock_space_firmware(void);

/*
- * Read recovery hash data from TPM.
- * @param index index into TPM NVRAM where hash is stored
+ * Read MRC hash data from TPM.
+ * @param index index into TPM NVRAM where hash is stored The index
+ *              can be set to either MRC_REC_HASH_NV_INDEX or
+ *              MRC_RW_HASH_NV_INDEX depending upon whether we are
+ *              booting in recovery or normal mode.
  * @param data  pointer to buffer where hash from TPM read into
  * @param size  size of buffer
  */
 uint32_t antirollback_read_space_mrc_hash(uint32_t index, uint8_t *data, uint32_t size);
 /*
- * Write new hash data to recovery space in TPM.\
- * @param index index into TPM NVRAM where hash is stored
+ * Write new hash data to MRC space in TPM.\
+ * @param index index into TPM NVRAM where hash is stored The index
+ *              can be set to either MRC_REC_HASH_NV_INDEX or
+ *              MRC_RW_HASH_NV_INDEX depending upon whether we are
+ *              booting in recovery or normal mode.
  * @param data  pointer to buffer of hash value to be written
  * @param size  size of buffer
 */
 uint32_t antirollback_write_space_mrc_hash(uint32_t index, const uint8_t *data,
    				   uint32_t size);
 /*
- * Lock down recovery hash space in TPM.
- * @param index index into TPM NVRAM where hash is stored
+ * Lock down MRC hash space in TPM.
+ * @param index index into TPM NVRAM where hash is stored The index
+ *              can be set to either MRC_REC_HASH_NV_INDEX or
+ *              MRC_RW_HASH_NV_INDEX depending upon whether we are
+ *              booting in recovery or normal mode.
 */
 uint32_t antirollback_lock_space_mrc_hash(uint32_t index);

diff --git a/src/security/vboot/secdata_tpm.c b/src/security/vboot/secdata_tpm.c
index 451f043..0304b92 100644
--- a/src/security/vboot/secdata_tpm.c
+++ b/src/security/vboot/secdata_tpm.c
@@ -164,9 +164,14 @@

static uint32_t set_mrc_hash_space(uint32_t index, const uint8_t *data)
 {
-	return set_space("MRC Hash", index, data, HASH_NV_SIZE,
-			 ro_space_attributes, pcr0_unchanged_policy,
-			 sizeof(pcr0_unchanged_policy));
+	if (index == MRC_REC_HASH_NV_INDEX) {
+		return set_space("RO MRC Hash", index, data, HASH_NV_SIZE,
+				 ro_space_attributes, pcr0_unchanged_policy,
+				 sizeof(pcr0_unchanged_policy));
+	} else {
+		return set_space("RW MRC Hash", index, data, HASH_NV_SIZE,
+				 rw_space_attributes, NULL, 0);
+	}
 }

static uint32_t _factory_initialize_tpm(struct vb2_context *ctx)
@@ -183,6 +188,13 @@
     */
    RETURN_ON_FAILURE(set_kernel_space(ctx->secdata_kernel));

+	/*
+	 * Define and set rec hash space, if available.  No need to
+	 * create the RW hash space because we will definitely boot
+	 * once in normal mode before shipping, meaning that the space
+	 * will get created with correct permissions while still in in
+	 * our hands.
+	 */
    if (CONFIG(VBOOT_HAS_REC_HASH_SPACE))
    	RETURN_ON_FAILURE(set_mrc_hash_space(MRC_REC_HASH_NV_INDEX, mrc_hash_data));

@@ -304,7 +316,13 @@
    			     ctx->secdata_firmware,
    				VB2_SECDATA_FIRMWARE_SIZE));

-	/* Define and set rec hash space, if available. */
+	/*
+	 * Define and set rec hash space, if available.  No need to
+	 * create the RW hash space because we will definitely boot
+	 * once in normal mode before shipping, meaning that the space
+	 * will get created with correct permissions while still in in
+	 * our hands.
+	 */
    if (CONFIG(VBOOT_HAS_REC_HASH_SPACE))
    	RETURN_ON_FAILURE(set_mrc_hash_space(MRC_REC_HASH_NV_INDEX, mrc_hash_data));