Philipp Deppenwiese has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38858 )
Change subject: security/tpm: Include mrc.bin in CRTM if present ......................................................................
security/tpm: Include mrc.bin in CRTM if present
mrc.bin, on platforms where it is present, is code executed on CPU, so it should be considered a part of CRTM.
cbfs_locate_file_in_region() is hooked to measurement here too, since mrc.bin is loaded with it, and CBFS_TYPE_MRC (the type of mrc.bin) is measured to TPM_CRTM_PCR rather than TPM_RUNTIME_DATA_PCR.
TODO: I have heard that SMM is too resource-limited to link with vboot library, so currently tspi_measure_cbfs_hook() is masked in SMM. Please correct me if I am wrong.
Change-Id: Ib4c3cf47b919864056baf725001ca8a4aaafa110 Signed-off-by: Bill XIE persmule@hardenedlinux.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/38858 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Julius Werner jwerner@chromium.org --- M src/lib/cbfs.c M src/security/tpm/tspi/crtm.c M src/security/tpm/tspi/crtm.h 3 files changed, 16 insertions(+), 5 deletions(-)
Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved
diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c index 4392ab7..ccd7e6a 100644 --- a/src/lib/cbfs.c +++ b/src/lib/cbfs.c @@ -56,7 +56,10 @@ * Files can be added to the RO_REGION_ONLY config option to use this feature. */ printk(BIOS_DEBUG, "Fall back to RO region for %s\n", name); - ret = cbfs_locate_file_in_region(fh, "COREBOOT", name, type); + if (fmap_locate_area_as_rdev("COREBOOT", &rdev)) + ERROR("RO region not found\n"); + else + ret = cbfs_locate(fh, &rdev, name, type); }
if (!ret) @@ -86,14 +89,18 @@ const char *name, uint32_t *type) { struct region_device rdev; - + int ret = 0; if (fmap_locate_area_as_rdev(region_name, &rdev)) { LOG("%s region not found while looking for %s\n", region_name, name); return -1; }
- return cbfs_locate(fh, &rdev, name, type); + ret = cbfs_locate(fh, &rdev, name, type); + if (!ret) + if (tspi_measure_cbfs_hook(fh, name)) + return -1; + return ret; }
size_t cbfs_load_and_decompress(const struct region_device *rdev, size_t offset, diff --git a/src/security/tpm/tspi/crtm.c b/src/security/tpm/tspi/crtm.c index dc7d7d2..304cea3 100644 --- a/src/security/tpm/tspi/crtm.c +++ b/src/security/tpm/tspi/crtm.c @@ -133,10 +133,14 @@ cbfs_file_data(&rdev, fh);
switch (cbfs_type) { - case CBFS_TYPE_MRC: case CBFS_TYPE_MRC_CACHE: pcr_index = TPM_RUNTIME_DATA_PCR; break; + /* + * mrc.bin is code executed on CPU, so it + * should not be considered runtime data + */ + case CBFS_TYPE_MRC: case CBFS_TYPE_STAGE: case CBFS_TYPE_SELF: case CBFS_TYPE_FIT: diff --git a/src/security/tpm/tspi/crtm.h b/src/security/tpm/tspi/crtm.h index dfd91e1..eb62495 100644 --- a/src/security/tpm/tspi/crtm.h +++ b/src/security/tpm/tspi/crtm.h @@ -50,7 +50,7 @@ */ int tspi_measure_cache_to_pcr(void);
-#if CONFIG(TPM_MEASURED_BOOT) +#if !ENV_SMM && CONFIG(TPM_MEASURED_BOOT) /* * Measures cbfs data via hook (cbfs) * fh is the cbfs file handle to measure