| Message ID | 20211015212214.482348-1-erichte@linux.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] secvar/secboot_tpm: correctly reset the control index on secboot format | expand |
On Fri, Oct 15, 2021 at 5:22 PM Eric Richter <erichte@linux.ibm.com> wrote: > > When the SECBOOT partition is formatted, the bank hash stored in the > control TPM NV index must be updated to match, or else we will immediately > fail to load the freshly formatted data at the .load_bank() step. > > However, while the secboot_format() function does calculate and update the > bank hash, it only writes the new hash for bank 0. It does not update the > value for bank 1, or set the current active bank. This works as expected if > the active bank bit happens to be set to 0. On the other hand, if the active > bit is set to 1, the freshly formatted bank 1 will be compared against the > unchanged bank hash in bank 1 at the load step, therefore causing an error. > > This patch fixes this issue by also setting the active bit to 0 to match > the freshly calculated hash. > > Signed-off-by: Eric Richter <erichte@linux.ibm.com> Manually tested and ran with op-test, works as expected. Tested-by: Nick Child <nick.child@ibm.com> > --- > libstb/secvar/storage/secboot_tpm.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/libstb/secvar/storage/secboot_tpm.c b/libstb/secvar/storage/secboot_tpm.c > index 129f674a..5907ff07 100644 > --- a/libstb/secvar/storage/secboot_tpm.c > +++ b/libstb/secvar/storage/secboot_tpm.c > @@ -127,12 +127,15 @@ static int secboot_format(void) > prlog(PR_ERR, "Bank hash failed to calculate somehow\n"); > return rc; > } > + /* Clear bank_hash[1] anyway, to match state of PNOR */ > + memset(tpmnv_control_image->bank_hash[1], 0x00, sizeof(tpmnv_control_image->bank_hash[1])); > + > + tpmnv_control_image->active_bit = 0; > > rc = tpmnv_ops.write(SECBOOT_TPMNV_CONTROL_INDEX, > - tpmnv_control_image->bank_hash[0], > - SHA256_DIGEST_SIZE, > - offsetof(struct tpmnv_control, > - bank_hash[0])); > + tpmnv_control_image, > + sizeof(struct tpmnv_control), > + 0); > if (rc) { > prlog(PR_ERR, "Could not write fresh formatted bank hashes to CONTROL index, rc=%d\n", rc); > return rc; > -- > 2.29.2
diff --git a/libstb/secvar/storage/secboot_tpm.c b/libstb/secvar/storage/secboot_tpm.c index 129f674a..5907ff07 100644 --- a/libstb/secvar/storage/secboot_tpm.c +++ b/libstb/secvar/storage/secboot_tpm.c @@ -127,12 +127,15 @@ static int secboot_format(void) prlog(PR_ERR, "Bank hash failed to calculate somehow\n"); return rc; } + /* Clear bank_hash[1] anyway, to match state of PNOR */ + memset(tpmnv_control_image->bank_hash[1], 0x00, sizeof(tpmnv_control_image->bank_hash[1])); + + tpmnv_control_image->active_bit = 0; rc = tpmnv_ops.write(SECBOOT_TPMNV_CONTROL_INDEX, - tpmnv_control_image->bank_hash[0], - SHA256_DIGEST_SIZE, - offsetof(struct tpmnv_control, - bank_hash[0])); + tpmnv_control_image, + sizeof(struct tpmnv_control), + 0); if (rc) { prlog(PR_ERR, "Could not write fresh formatted bank hashes to CONTROL index, rc=%d\n", rc); return rc;
When the SECBOOT partition is formatted, the bank hash stored in the control TPM NV index must be updated to match, or else we will immediately fail to load the freshly formatted data at the .load_bank() step. However, while the secboot_format() function does calculate and update the bank hash, it only writes the new hash for bank 0. It does not update the value for bank 1, or set the current active bank. This works as expected if the active bank bit happens to be set to 0. On the other hand, if the active bit is set to 1, the freshly formatted bank 1 will be compared against the unchanged bank hash in bank 1 at the load step, therefore causing an error. This patch fixes this issue by also setting the active bit to 0 to match the freshly calculated hash. Signed-off-by: Eric Richter <erichte@linux.ibm.com> --- libstb/secvar/storage/secboot_tpm.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)