On Fri, Sep 13, 2024 at 09:40:30AM +0300, Ilias Apalodimas wrote: > Hi Gregory, > > On Fri, 6 Sept 2024 at 23:28, Gregory Price <gourry@xxxxxxxxxx> wrote: > > > > If the log version is not sane (0 or >2), don't attempt to use > > the rest of the log values for anything to avoid potential corruption. > > > > Signed-off-by: Gregory Price <gourry@xxxxxxxxxx> > > --- > > drivers/firmware/efi/tpm.c | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c > > index 6e03eed0dc6f..9a080887a3e0 100644 > > --- a/drivers/firmware/efi/tpm.c > > +++ b/drivers/firmware/efi/tpm.c > > @@ -60,6 +60,15 @@ int __init efi_tpm_eventlog_init(void) > > return -ENOMEM; > > } > > > > + if (!log_tbl->version || > > + log_tbl->version > EFI_TCG2_EVENT_LOG_FORMAT_TCG_2) { > > + pr_err(FW_BUG "TPM Events table version invalid (%x)\n", > > + log_tbl->version); > > + early_memunmap(log_tbl, sizeof(*log_tbl)); > > + efi.tpm_log = EFI_INVALID_TABLE_ADDR; > > + return -EINVAL; > > I don't think we need this check at all. Did you actually see this happening? > efi_retrieve_eventlog() that runs during the efistub tries to retrieve > the log and the EFI protocol itself explicitly says that the firmware > *must* return EFI_INVALID_PARAMETER if the event log is not in 1.2 or > 2.0 format. If the firmware does something wrong, we should report the > FW BUG in that function, instead of installing the config tables Linux > uses internally to handover the log and catching it late. > > Thanks > /Ilias > We saw this happen and discovered it was a disagreement between EFI/OS/kexec causing the table to be overwritten during kexec. We've since found a fix for that. So the result was that it appeared the firmware was doing something wrong. The sanity check at least allowed us to boot without immediately crashing - because the tables don't get reinstalled, they get re-used (at least that's by best understanding of the whole interaction). If the check seems superfluous, i can drop it. > > > > + } > > + > > tbl_size = sizeof(*log_tbl) + log_tbl->size; > > if (memblock_reserve(efi.tpm_log, tbl_size)) { > > pr_err("TPM Event Log memblock reserve fails (0x%lx, 0x%x)\n", > > -- > > 2.43.0 > >