Joel Kitching has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34790 )
Change subject: vboot: fix conditional using vboot_setup_tpm return value ......................................................................
vboot: fix conditional using vboot_setup_tpm return value
vboot_setup_tpm returns (TPM_SUCCESS == 0) on success. In this case, call antirollback_read_space_firmware.
BUG=b:139101213 TEST=make clean && make test-abuild BRANCH=none
Change-Id: Ifdea1d85167a50a1ada5afe9b107408e3a2e0d6f Signed-off-by: Joel Kitching kitching@google.com --- M src/security/vboot/vboot_logic.c 1 file changed, 1 insertion(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/34790/1
diff --git a/src/security/vboot/vboot_logic.c b/src/security/vboot/vboot_logic.c index c61d6be..30c288a 100644 --- a/src/security/vboot/vboot_logic.c +++ b/src/security/vboot/vboot_logic.c @@ -335,8 +335,7 @@ * check the return value here because vb2api_fw_phase1 will catch * invalid secdata and tell us what to do (=reboot). */ timestamp_add_now(TS_START_TPMINIT); - rv = vboot_setup_tpm(&ctx); - if (rv) + if (TPM_SUCCESS == vboot_setup_tpm(&ctx)) antirollback_read_space_firmware(&ctx); timestamp_add_now(TS_END_TPMINIT);
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34790 )
Change subject: vboot: fix conditional using vboot_setup_tpm return value ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34790/1/src/security/vboot/vboot_lo... File src/security/vboot/vboot_logic.c:
https://review.coreboot.org/c/coreboot/+/34790/1/src/security/vboot/vboot_lo... PS1, Line 338: if (TPM_SUCCESS == vboot_setup_tpm(&ctx)) Comparisons should place the constant on the right side of the test
Hello Andrey Pronin, Aaron Durbin, Julius Werner, caveh jalali, Christian Walter, Philipp Deppenwiese,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34790
to look at the new patch set (#2).
Change subject: vboot: fix conditional using vboot_setup_tpm return value ......................................................................
vboot: fix conditional using vboot_setup_tpm return value
vboot_setup_tpm returns (TPM_SUCCESS == 0) on success. In this case, call antirollback_read_space_firmware.
BUG=b:139101213 TEST=make clean && make test-abuild BRANCH=none
Change-Id: Ifdea1d85167a50a1ada5afe9b107408e3a2e0d6f Signed-off-by: Joel Kitching kitching@google.com --- M src/security/vboot/vboot_logic.c 1 file changed, 1 insertion(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/34790/2
Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34790 )
Change subject: vboot: fix conditional using vboot_setup_tpm return value ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34790/1/src/security/vboot/vboot_lo... File src/security/vboot/vboot_logic.c:
https://review.coreboot.org/c/coreboot/+/34790/1/src/security/vboot/vboot_lo... PS1, Line 338: if (TPM_SUCCESS == vboot_setup_tpm(&ctx))
Comparisons should place the constant on the right side of the test
Done
caveh jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34790 )
Change subject: vboot: fix conditional using vboot_setup_tpm return value ......................................................................
Patch Set 2: Code-Review+1
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34790 )
Change subject: vboot: fix conditional using vboot_setup_tpm return value ......................................................................
Patch Set 2: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/34790/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34790/2//COMMIT_MSG@10 PS2, Line 10: In this case, call antirollback_read_space_firmware. It would be good to mention that the regression was introduced here: CB:34510
Hello Andrey Pronin, Aaron Durbin, Julius Werner, caveh jalali, Christian Walter, Philipp Deppenwiese, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34790
to look at the new patch set (#3).
Change subject: vboot: fix conditional using vboot_setup_tpm return value ......................................................................
vboot: fix conditional using vboot_setup_tpm return value
vboot_setup_tpm returns (TPM_SUCCESS == 0) on success. In this case, call antirollback_read_space_firmware.
This regression was introduced in CB:34510.
BUG=b:139101213 TEST=make clean && make test-abuild BRANCH=none
Change-Id: Ifdea1d85167a50a1ada5afe9b107408e3a2e0d6f Signed-off-by: Joel Kitching kitching@google.com --- M src/security/vboot/vboot_logic.c 1 file changed, 1 insertion(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/34790/3
Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34790 )
Change subject: vboot: fix conditional using vboot_setup_tpm return value ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34790/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34790/2//COMMIT_MSG@10 PS2, Line 10: In this case, call antirollback_read_space_firmware.
It would be good to mention that the regression was introduced here: CB:34510
Done
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34790 )
Change subject: vboot: fix conditional using vboot_setup_tpm return value ......................................................................
Patch Set 3: Code-Review+2
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34790 )
Change subject: vboot: fix conditional using vboot_setup_tpm return value ......................................................................
Patch Set 3: Code-Review+2
Ronak Kanabar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34790 )
Change subject: vboot: fix conditional using vboot_setup_tpm return value ......................................................................
Patch Set 3: Code-Review+1
V Sowmya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34790 )
Change subject: vboot: fix conditional using vboot_setup_tpm return value ......................................................................
Patch Set 3: Code-Review+2
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34790 )
Change subject: vboot: fix conditional using vboot_setup_tpm return value ......................................................................
Patch Set 3:
merging now as we have multiple +2 to unblock Hatch CPFE build brokenness issue
Subrata Banik has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34790 )
Change subject: vboot: fix conditional using vboot_setup_tpm return value ......................................................................
vboot: fix conditional using vboot_setup_tpm return value
vboot_setup_tpm returns (TPM_SUCCESS == 0) on success. In this case, call antirollback_read_space_firmware.
This regression was introduced in CB:34510.
BUG=b:139101213 TEST=make clean && make test-abuild BRANCH=none
Change-Id: Ifdea1d85167a50a1ada5afe9b107408e3a2e0d6f Signed-off-by: Joel Kitching kitching@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34790 Reviewed-by: Subrata Banik subrata.banik@intel.com Reviewed-by: Aamir Bohra aamir.bohra@intel.com Reviewed-by: Ronak Kanabar ronak.kanabar@intel.com Reviewed-by: V Sowmya v.sowmya@intel.com Reviewed-by: caveh jalali caveh@chromium.org Reviewed-by: Furquan Shaikh furquan@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/security/vboot/vboot_logic.c 1 file changed, 1 insertion(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Subrata Banik: Looks good to me, approved Aamir Bohra: Looks good to me, approved V Sowmya: Looks good to me, approved caveh jalali: Looks good to me, but someone else must approve Ronak Kanabar: Looks good to me, but someone else must approve
diff --git a/src/security/vboot/vboot_logic.c b/src/security/vboot/vboot_logic.c index c61d6be..7f00df5 100644 --- a/src/security/vboot/vboot_logic.c +++ b/src/security/vboot/vboot_logic.c @@ -335,8 +335,7 @@ * check the return value here because vb2api_fw_phase1 will catch * invalid secdata and tell us what to do (=reboot). */ timestamp_add_now(TS_START_TPMINIT); - rv = vboot_setup_tpm(&ctx); - if (rv) + if (vboot_setup_tpm(&ctx) == TPM_SUCCESS) antirollback_read_space_firmware(&ctx); timestamp_add_now(TS_END_TPMINIT);
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34790 )
Change subject: vboot: fix conditional using vboot_setup_tpm return value ......................................................................
Patch Set 4: Code-Review+2
Sorry about that.
-
Aamir Bohra (Code Review) -
build bot (Jenkins) (Code Review)
-
caveh jalali (Code Review)
-
Furquan Shaikh (Code Review)
-
Joel Kitching (Code Review)
-
Julius Werner (Code Review)
-
Ronak Kanabar (Code Review)
-
Subrata Banik (Code Review)
-
V Sowmya (Code Review)