Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31597 )
Change subject: security/vboot: Add fmap measurements ......................................................................
Patch Set 35:
(1 comment)
https://review.coreboot.org/c/coreboot/+/31597/35/src/security/vboot/vboot_c... File src/security/vboot/vboot_crtm.c:
https://review.coreboot.org/c/coreboot/+/31597/35/src/security/vboot/vboot_c... PS35, Line 67: for (i = 0; i < ARRAY_SIZE(ifd_static); i++) { : if (fmap_locate_area_as_rdev(ifd_static[i], &fmap) == 0) { : char log_string[TCPA_PCR_HASH_NAME]; : snprintf(log_string, TCPA_PCR_HASH_NAME, : "FMAP: %s", ifd_static[i]); : : if (tpm_measure_region(&fmap, TPM_CRTM_PCR, : log_string) != TPM_SUCCESS) : return VB2_ERROR_UNKNOWN; : } : } : : for (i = 0; i < ARRAY_SIZE(ifd_dynamic); i++) { : if (fmap_locate_area_as_rdev(ifd_dynamic[i], &fmap) == 0) { : char log_string[TCPA_PCR_HASH_NAME]; : snprintf(log_string, TCPA_PCR_HASH_NAME, : "FMAP: %s", ifd_dynamic[i]); : : if (tpm_measure_region(&fmap, TPM_RUNTIME_DATA_PCR, : log_string) != TPM_SUCCESS) : return VB2_ERROR_UNKNOWN; : } : } These two for-loops do exactly the same except the PCR where the region is measured to. You could use a struct with region name and PCR pairs, then use just one ifd_... array of this struct and get rid of the second for-loop. Will make the code nicer and smaller in my point of view.