Hi Ilias, Thanks for the review!
Am 01.11.26 um 12:52 schrieb Ilias Apalodimas: > Yes, but users that will call measurement_init etc will have > initialized the TPM. > The PCR code exists because some users want to use the TPM only as an RNG. My use case is a custom measurement implementation on top of lib/tpm-v2.c, which cannot use the tcg2_* functions. So tcg2_log_prepare_buffer() never runs for us, and nothing makes sure the unsupported banks are deactivated. I think the low-level API together with CONFIG_TPM_PCR_ALLOCATE should be able to handle this. Regarding RNG-only users: TPM_PCR_ALLOCATE depends on MEASURED_BOOT || EFI_TCG2_PROTOCOL and is off by default. Its help text states that U-Boot sends a PCR allocate and reboots, without specifying at which point during boot. So a board enabling it opts in to that reboot. With MEASURED_BOOT, the first measurement resets the board anyway (with patches 1-4). With only EFI_TCG2_PROTOCOL, the reset currently happens once the EFI subsystem is initialised; boards which boot without EFI do not get it today, but arguably should, if they enabled TPM_PCR_ALLOCATE. > IIRC we can't get rid of the EFI call, because it's there to configure > algos that where handed over to us from a previous stage loader that > only handed us an EventLog. Agreed, that call needs to stay, since only there the banks can be aligned with the event log. > Can't we fet rid of it in pcr_extend though? I would rather keep it as a fallback: callers may use tpm2_startup() directly instead of tpm_auto_start(), e.g. custom board code or "tpm2 startup" followed by "tpm2 pcr_extend" on the command line. For them, the check in tpm2_pcr_extend() is the only place where active unsupported banks are caught. With patch 4, it only deactivates the unsupported banks, and only with CONFIG_TPM_PCR_ALLOCATE. Thanks, Tycho >> >> tcg2_create_digest: unsupported algorithm 28 >> Cannot extend PCRs if all the TPM enabled algorithms are not supported >> >> Deactivate them in tpm2_auto_start() already, before anything gets >> measured. If this fails, e.g. because the TPM refuses the allocation or >> no supported bank would remain active, return an error. The rationale >> is that with CONFIG_TPM_PCR_ALLOCATE enabled, U-Boot is expected to >> manage the banks, so fail early instead of at the first PCR extend. >> This is in line with the TPM_PCR_ALLOCATE help text, which does not >> specify at which point during boot the allocation takes place. >> >> Signed-off-by: Tycho Kirchner <[email protected]> >> --- >> lib/tpm-v2.c | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c >> index 18ec9eb960d..bd049fed748 100644 >> --- a/lib/tpm-v2.c >> +++ b/lib/tpm-v2.c >> @@ -232,6 +232,18 @@ u32 tpm2_auto_start(struct udevice *dev) >> if (rc) >> return rc; >> >> + /* >> + * Early deactivate active banks with unsupported algorithms. On >> success, >> + * this resets the board. >> + */ >> + if (IS_ENABLED(CONFIG_TPM_PCR_ALLOCATE)) { >> + rc = tpm2_activate_banks(dev, 0); >> + if (rc) { >> + log_err("TPM: failed to deactivate unsupported >> banks\n"); >> + return rc; >> + } >> + } >> + >> return tpm2_update_active_banks(dev); >> } >> >> -- >> 2.47.3 >> -- M.Sc. Tycho Kirchner emlix GmbH Headquarters: Berliner Str. 12, 37073 Göttingen, Germany Phone +49 (0)551 30664-0, e-mail [email protected] District Court of Göttingen, Registry Number HR B 3160 Managing Directors: Heike Jordan, Dr. Uwe Kracke VAT ID No. DE 205 198 055 Office Berlin: Panoramastr. 1, 10178 Berlin, Germany Office Bonn: Bachstr. 6, 53115 Bonn, Germany Office München: Am Knie 16, 81241 München, Germany http://www.emlix.com emlix - your embedded Linux partner
