Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29563 )
Change subject: security/tpm: Fix TCPA log feature ......................................................................
Patch Set 62:
(8 comments)
https://review.coreboot.org/#/c/29563/62/Documentation/security/vboot/measur... File Documentation/security/vboot/measured_boot.md:
https://review.coreboot.org/#/c/29563/62/Documentation/security/vboot/measur... PS62, Line 46: The second column is the hash of the raw data. The third column contains that seems important. Add a custom section what is measured.
Answer the following questions: * Is compressed or uncompressed data measured? * which part of the CBFS file is measured? ** is the header measured, too?
https://review.coreboot.org/#/c/29563/62/src/arch/x86/car.ld File src/arch/x86/car.ld:
https://review.coreboot.org/#/c/29563/62/src/arch/x86/car.ld@34 PS62, Line 34: /* Vboot measured boot TCPA log measurements .
https://review.coreboot.org/#/c/29563/62/src/arch/x86/car.ld@35 PS62, Line 35: * needs to be transferred until CBMEM is available Needs
https://review.coreboot.org/#/c/29563/62/src/arch/x86/car.ld@37 PS62, Line 37: #if IS_ENABLED(CONFIG_VBOOT_TCPA_LOG_RECOVERY) why is it guarded by IS_ENABLED(CONFIG_VBOOT_TCPA_LOG_RECOVERY) but the other linker script doesn't contain it?
https://review.coreboot.org/#/c/29563/62/src/security/tpm/tspi/log.c File src/security/tpm/tspi/log.c:
https://review.coreboot.org/#/c/29563/62/src/security/tpm/tspi/log.c@94 PS62, Line 94: enum vb2_hash_algorithm digest_algo, const uint8_t *digest,
line over 80 characters
use new line for const uint8_t *digest
https://review.coreboot.org/#/c/29563/62/src/security/vboot/vboot_crtm.c File src/security/vboot/vboot_crtm.c:
https://review.coreboot.org/#/c/29563/62/src/security/vboot/vboot_crtm.c@20 PS62, Line 20: short description what it does
https://review.coreboot.org/#/c/29563/62/src/security/vboot/vboot_crtm.c@25 PS62, Line 25: use an array and loop over fmap names
https://review.coreboot.org/#/c/29563/62/src/security/vboot/vboot_crtm.c@78 PS62, Line 78: /* Clear TCPA PRERAM log */ better: Initialize TCPE PRERAM log. That it's cleared can be deviated from the function name.