Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31597 )
Change subject: security/vboot: Add fmap measurements ......................................................................
Patch Set 28:
(6 comments)
https://review.coreboot.org/#/c/31597/28/src/security/vboot/Kconfig File src/security/vboot/Kconfig:
https://review.coreboot.org/#/c/31597/28/src/security/vboot/Kconfig@43 PS28, Line 43: list same comments as patch 27
https://review.coreboot.org/#/c/31597/27/src/security/vboot/vboot_crtm.c File src/security/vboot/vboot_crtm.c:
https://review.coreboot.org/#/c/31597/27/src/security/vboot/vboot_crtm.c@249 PS27, Line 249: if (!strcmp("COREBOOT", name) ||
mhhhhhhh? Where is the documentation?
I thought that was being done? It was a real question. If it isn't then I am mistaken.
https://review.coreboot.org/#/c/31597/28/src/security/vboot/vboot_crtm.c File src/security/vboot/vboot_crtm.c:
https://review.coreboot.org/#/c/31597/28/src/security/vboot/vboot_crtm.c@69 PS28, Line 69: if (tpm_measure_region(&fmap, TPM_RUNTIME_DATA_PCR, All these got moved to RUNTIME_DATA_PCR while some were CRTM_PCR (SI_DESC, SI_BE)
https://review.coreboot.org/#/c/31597/28/src/security/vboot/vboot_crtm.c@179 PS28, Line 179: static uint32_t is_runtime_data(const char *list, const char *name) Probably rename this to something indicating it's now returning pcr index.
https://review.coreboot.org/#/c/31597/28/src/security/vboot/vboot_crtm.c@187 PS28, Line 187: // foo,blah,blub ?
https://review.coreboot.org/#/c/31597/28/src/security/vboot/vboot_crtm.c@191 PS28, Line 191: !memcmp(list, name, name_len)) strncmp might be better and let it not match if it encounters NUL or ','. What do you think? Seems more straight forward.