Attention is currently required from: Michał Żygowski, Maciej Pijanowski, Christian Walter, Julius Werner, Krystian Hebel.
Sergii Dmytruk has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68747 )
Change subject: security/tpm: add TPM log format as per 1.2 spec ......................................................................
Patch Set 6:
(7 comments)
File src/commonlib/include/commonlib/tpm_log_defs.h:
https://review.coreboot.org/c/coreboot/+/68747/comment/7580f484_f7ccef71 PS5, Line 1: /* SPDX-License-Identifier: GPL-2.0-only */
Note that licensing this as GPLv2 would make it hard to share with libpayload in the future. […]
It's partially based on a commit by Michał Żygowski, so asked him. I suspect the license can be changed.
File src/security/tpm/Kconfig:
https://review.coreboot.org/c/coreboot/+/68747/comment/8e216d5e_a8b06d70 PS5, Line 107: TPM_LOG_TPM12
We generally use "TPM1", not "TPM12" in the code, so I would suggest calling this TPM_LOG_TPM1 as we […]
Done
https://review.coreboot.org/c/coreboot/+/68747/comment/705687b8_e2eed3b6 PS5, Line 108: bool "TPM 1.2 format"
Can this log format be used with 2.0 TPMs? Otherwise, this should probably have a `depends on TPM1`.
I don't think such usage is forbidden.
File src/security/tpm/tspi.h:
https://review.coreboot.org/c/coreboot/+/68747/comment/13e92a6e_dd5cee0d PS5, Line 11: #include "tpm12_log_serialized.h"
Please use angled brackets notation (i.e. <security/tpm/tpm1_log_serialized. […]
Done
File src/security/tpm/tspi/log-tpm12.c:
https://review.coreboot.org/c/coreboot/+/68747/comment/1a19a732_e41ed206 PS5, Line 115: strncpy((char *)tce->data, name, sizeof(tce->data) - 1);
This needs to guarantee that tce->data gets null-terminated (your other functions rely on that), so […]
I interpreted it as a reliance on memory being zeroed in the original code, then I wasn't sure it's the case and zeroed header explicitly, but forgot about this. Thanks.
Added CB:69708 for old format.
https://review.coreboot.org/c/coreboot/+/68747/comment/d01b8704_8289550f PS5, Line 123: tclt->spec_id.num_entries = 0;
So this means that the pre-RAM version of the log table will never be fully initialized (e.g. […]
Added a comment for TPM1 and TPM2.
https://review.coreboot.org/c/coreboot/+/68747/comment/13113eda_36577803 PS5, Line 161: struct tpm_12_log_entry *tce = &to_log->entries[to_log->spec_id.num_entries++];
Why not just memcpy() the whole entry?
Done