Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29563 )
Change subject: security/tpm: Fix TCPA log feature ......................................................................
Patch Set 55: Code-Review+1
(13 comments)
https://review.coreboot.org/#/c/29563/55/Documentation/security/index.md File Documentation/security/index.md:
https://review.coreboot.org/#/c/29563/55/Documentation/security/index.md@3 PS55, Line 3: contains documentation about describes
https://review.coreboot.org/#/c/29563/55/Documentation/security/vboot/measur... File Documentation/security/vboot/measured_boot.md:
https://review.coreboot.org/#/c/29563/55/Documentation/security/vboot/measur... PS55, Line 4: easy understandable "easy to understand"?
https://review.coreboot.org/#/c/29563/55/Documentation/security/vboot/measur... PS55, Line 36: SP Sp
https://review.coreboot.org/#/c/29563/55/Documentation/security/vboot/measur... PS55, Line 38: so : that ". Therefore"?
https://review.coreboot.org/#/c/29563/55/Documentation/security/vboot/measur... PS55, Line 41: So we We
https://review.coreboot.org/#/c/29563/55/Documentation/security/vboot/measur... PS55, Line 41: easy understandable easy to understand
https://review.coreboot.org/#/c/29563/55/Documentation/security/vboot/measur... PS55, Line 44: Some details regarding the eventlog table. this is either a new subtitle (e.g. ### Details, or "table format") or should be left out altogether.
https://review.coreboot.org/#/c/29563/55/Documentation/security/vboot/measur... PS55, Line 45: The : third "Third comes" or simply "The third column contains"
https://review.coreboot.org/#/c/29563/55/Documentation/security/vboot/measur... PS55, Line 47: . We have one prefix only. CBFS and : FMAP which describe where the measured data comes from (coreboot filesystem or : partition table). : first the namespace from where the data came from, CBFS or FMAP, then the name used to look up the data (region or file name).
https://review.coreboot.org/#/c/29563/55/Documentation/security/vboot/measur... PS55, Line 62: CBFS In some scenarios there are multiple CBFSes with duplicate filenames. What happens then?
https://review.coreboot.org/#/c/29563/55/Documentation/security/vboot/measur... PS55, Line 82: -f romstage -U && sha256sum -b romstage -f /dev/stdout -U | sha256sum would work, too
https://review.coreboot.org/#/c/29563/55/Documentation/security/vboot/measur... PS55, Line 87: -f intel-me && sha256sum -b intel-me same here.
https://review.coreboot.org/#/c/29563/55/src/security/vboot/vboot_crtm.c File src/security/vboot/vboot_crtm.c:
https://review.coreboot.org/#/c/29563/55/src/security/vboot/vboot_crtm.c@149 PS55, Line 149: strncpy(prefix_name, CBFS_PREFIX, TCPA_PCR_HASH_NAME - 1); : strncpy(prefix_name + sizeof(CBFS_PREFIX), : name, TCPA_PCR_HASH_NAME - sizeof(CBFS_PREFIX) - 1); snprintf?