Attention is currently required from: Andrey Pronin, Raul Rangel, Yu-Ping Wu, Karthik Ramasubramanian. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59476 )
Change subject: src/security/vboot: Setup secure counter space in TPM NVRAM ......................................................................
Patch Set 1:
(1 comment)
File src/security/vboot/secdata_tpm.c:
https://review.coreboot.org/c/coreboot/+/59476/comment/c1d7f0ab_8357bc22 PS1, Line 368: rv = tlcl_read(index, &value, SECURE_COUNTER_SIZE);
Does TPM return TPM2_RC_NV_DEFINED if the NV index already exists? If so, I can treat it similar to […]
I don't think you should do anything there. This code is not supposed to recover from power loss in the middle of factory setup. It's better to not have error recovery at all and always fail hard than to only do it partially and risk missing certain edge cases in an unsafe way.
(If we wanted to generally make this recover correctly from aborted factory setup for all the spaces, that's a different thing, but that would require a bit more planning and careful consideration of all the different intermediate states that it could be stuck in. Since I don't think this has ever been an issue in practice, I don't think it's worth the effort.)
-
Julius Werner (Code Review)