Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31597 )
Change subject: security/vboot: Add fmap measurements ......................................................................
Patch Set 33:
(6 comments)
https://review.coreboot.org/#/c/31597/33/Documentation/security/vboot/measur... File Documentation/security/vboot/measured_boot.md:
https://review.coreboot.org/#/c/31597/33/Documentation/security/vboot/measur... PS33, Line 42: what does it mean?
https://review.coreboot.org/#/c/31597/33/Documentation/security/vboot/measur... PS33, Line 43: #### CBFS files (stages, blobs) only if EC is in same boot flash
https://review.coreboot.org/#/c/31597/33/src/drivers/intel/fsp1_0/fastboot_c... File src/drivers/intel/fsp1_0/fastboot_cache.c:
https://review.coreboot.org/#/c/31597/33/src/drivers/intel/fsp1_0/fastboot_c... PS33, Line 64: if (vboot_measure_fmap(&rdev, "RW_MRC_CACHE")) { would be possibly measured twice, once on read and once before update.
https://review.coreboot.org/#/c/31597/33/src/drivers/mrc_cache/mrc_cache.c File src/drivers/mrc_cache/mrc_cache.c:
https://review.coreboot.org/#/c/31597/33/src/drivers/mrc_cache/mrc_cache.c@3... PS33, Line 317: return -1; why does it return on error? it's not in verified mode, is it? Applies to all calls to this function.
https://review.coreboot.org/#/c/31597/33/src/drivers/smmstore/store.c File src/drivers/smmstore/store.c:
https://review.coreboot.org/#/c/31597/33/src/drivers/smmstore/store.c@103 PS33, Line 103: if (vboot_measure_fmap(&store, CONFIG_SMMSTORE_REGION)) that would cause to code to run on every call to SMM store. What happens if TPM is in use by operating system?
https://review.coreboot.org/#/c/31597/33/src/lib/cbfs.c File src/lib/cbfs.c:
https://review.coreboot.org/#/c/31597/33/src/lib/cbfs.c@101 PS33, Line 101: return -1; seams unrelated.