Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31597 )
Change subject: security/vboot: Add fmap measurements ......................................................................
Patch Set 39:
(5 comments)
https://review.coreboot.org/c/coreboot/+/31597/39/src/drivers/smmstore/store... File src/drivers/smmstore/store.c:
https://review.coreboot.org/c/coreboot/+/31597/39/src/drivers/smmstore/store... PS39, Line 114: int smmstore_read_region(void *buf, ssize_t *bufsize) that's currently only called in SMM
https://review.coreboot.org/c/coreboot/+/31597/39/src/lib/cbfs.c File src/lib/cbfs.c:
https://review.coreboot.org/c/coreboot/+/31597/39/src/lib/cbfs.c@99 PS39, Line 99: if (vboot_measure_cbfs_hook(fh, name)) that has nothing to do with fmaps
https://review.coreboot.org/c/coreboot/+/31597/39/src/security/vboot/Kconfig File src/security/vboot/Kconfig:
https://review.coreboot.org/c/coreboot/+/31597/39/src/security/vboot/Kconfig... PS39, Line 64: This includes the IFD, GbE, ME, EC and Platform Data regions. that are only a few partitions, the IFD has more. Why is EC measured? How does it influence the CRT? Why is GbE measured? How does it influence the CRT?
https://review.coreboot.org/c/coreboot/+/31597/39/src/security/vboot/vboot_c... File src/security/vboot/vboot_crtm.c:
https://review.coreboot.org/c/coreboot/+/31597/39/src/security/vboot/vboot_c... PS39, Line 61: "SI_PDR"}; why is PDR static?
https://review.coreboot.org/c/coreboot/+/31597/39/src/security/vboot/vboot_c... PS39, Line 73: if (tpm_measure_region(&fmap, TPM_CRTM_PCR, you need to check if the partition can be read from SPI master "host CPU". The ME region might not be accessible at all.