| Message ID | 20241210222608.598424-1-stefanb@linux.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | tpm/eventlog: Limit memory allocations for event logs with excessive size | expand |
On Wed Dec 11, 2024 at 12:26 AM EET, Stefan Berger wrote: > The TPM2 ACPI BIOS eventlog of a particular machine indicates that the > length of the log is 4MB, even though the actual length of its useful data, > when dumped, are only 69kb. To avoid allocating excessive amounts of memory > for the event log, limit the size of any eventlog to 128kb. This should be > sufficient memory and also not unnecessarily truncate event logs on any > other machine. > > Reported-by: Andy Liang <andy.liang@hpe.com> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219495 > Cc: Takashi Iwai <tiwai@suse.de> > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > --- > drivers/char/tpm/eventlog/acpi.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/char/tpm/eventlog/acpi.c b/drivers/char/tpm/eventlog/acpi.c > index 69533d0bfb51..701fd7d4cc28 100644 > --- a/drivers/char/tpm/eventlog/acpi.c > +++ b/drivers/char/tpm/eventlog/acpi.c > @@ -26,6 +26,8 @@ > #include "../tpm.h" > #include "common.h" > > +#define MAX_TPM_LOG_LEN (128 * 1024) Instead, to common.h: /* * Cap the log size to the given number of bytes. Applied to the TPM2 * ACPI logs. */ #define TPM_MAX_LOG_SIZE (128 * 1024) > > + > struct acpi_tcpa { > struct acpi_table_header hdr; > u16 platform_class; > @@ -135,6 +137,12 @@ int tpm_read_log_acpi(struct tpm_chip *chip) > return -EIO; > } > > + if (len > MAX_TPM_LOG_LEN) { > + dev_warn(&chip->dev, "Excessive TCPA log len %llu truncated to %u bytes\n", > + len, MAX_TPM_LOG_LEN); > + len = MAX_TPM_LOG_LEN; > + } First, you are changing also TPM1 code path. Also in the case of TPM2 code path the log message is incorrect as TCPA does not exist. Second, this does not make sense as the log ends up to be corrupted (i.e. not complete). Instead, in the TPM2 code path: start = tpm2_phy->log_area_start_address; if (!start || !len) { acpi_put_table((struct acpi_table_header *)tbl); return -ENODEV; } if (len > TPM_MAX_LOG_SIZE) { dev_warn(&chip->dev, "Excessive TPM2 log size of %llu bytes (> %u)\n", len, MAX_TPM_LOG_LEN); log->bios_event_log = start; chip->flags |= TPM_CHIP_FLAG_TPM2_ACPI; return 0; } This can then be used in tpm2.c to create a "slow path" in tpm2.c for parsing TPM2 ACPI log directly by mapping IO memory. BR, Jarkko
On 12/13/24 10:51 PM, Jarkko Sakkinen wrote: > On Wed Dec 11, 2024 at 12:26 AM EET, Stefan Berger wrote: >> The TPM2 ACPI BIOS eventlog of a particular machine indicates that the >> length of the log is 4MB, even though the actual length of its useful data, >> when dumped, are only 69kb. To avoid allocating excessive amounts of memory >> for the event log, limit the size of any eventlog to 128kb. This should be >> sufficient memory and also not unnecessarily truncate event logs on any >> other machine. >> >> Reported-by: Andy Liang <andy.liang@hpe.com> >> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219495 >> Cc: Takashi Iwai <tiwai@suse.de> >> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> >> --- >> drivers/char/tpm/eventlog/acpi.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/char/tpm/eventlog/acpi.c b/drivers/char/tpm/eventlog/acpi.c >> index 69533d0bfb51..701fd7d4cc28 100644 >> --- a/drivers/char/tpm/eventlog/acpi.c >> +++ b/drivers/char/tpm/eventlog/acpi.c >> @@ -26,6 +26,8 @@ >> #include "../tpm.h" >> #include "common.h" >> >> +#define MAX_TPM_LOG_LEN (128 * 1024) > > Instead, to common.h: > > /* > * Cap the log size to the given number of bytes. Applied to the TPM2 > * ACPI logs. > */ > #define TPM_MAX_LOG_SIZE (128 * 1024) Done. > >> >> + >> struct acpi_tcpa { >> struct acpi_table_header hdr; >> u16 platform_class; >> @@ -135,6 +137,12 @@ int tpm_read_log_acpi(struct tpm_chip *chip) >> return -EIO; >> } >> >> + if (len > MAX_TPM_LOG_LEN) { >> + dev_warn(&chip->dev, "Excessive TCPA log len %llu truncated to %u bytes\n", >> + len, MAX_TPM_LOG_LEN); >> + len = MAX_TPM_LOG_LEN; >> + } > > First, you are changing also TPM1 code path. Also in the case of Ok, let's move it into the TPM2 code path then. > TPM2 code path the log message is incorrect as TCPA does not exist. > > Second, this does not make sense as the log ends up to be corrupted > (i.e. not complete). Truncating the log is something I am trying to prevent by giving it a generous size of 128 kb: "To avoid allocating excessive amounts of memory for the event log, limit the size of any eventlog to 128kb. This should be sufficient memory and also not unnecessarily truncate event logs on any other machine." The 8MB the machine with the faulty BIOS indicates are holding 69kb of log data at the beginning and then unnecessary data after that. So truncating this one to 128kb doesn't affect the 69kb at the beginning. > > Instead, in the TPM2 code path: > > start = tpm2_phy->log_area_start_address; > if (!start || !len) { > acpi_put_table((struct acpi_table_header *)tbl); > return -ENODEV; > } > > if (len > TPM_MAX_LOG_SIZE) { > dev_warn(&chip->dev, "Excessive TPM2 log size of %llu bytes (> %u)\n", > len, MAX_TPM_LOG_LEN); > log->bios_event_log = start; > chip->flags |= TPM_CHIP_FLAG_TPM2_ACPI; > return 0; > } > > This can then be used in tpm2.c to create a "slow path" in tpm2.c for > parsing TPM2 ACPI log directly by mapping IO memory. I thought the problem when getting request for 8MB is the code a bit further below from here that cannot allocated the 8MB. /* malloc EventLog space */ log->bios_event_log = devm_kmalloc(&chip->dev, len, GFP_KERNEL); if (!log->bios_event_log) > > BR, Jarkko
diff --git a/drivers/char/tpm/eventlog/acpi.c b/drivers/char/tpm/eventlog/acpi.c index 69533d0bfb51..701fd7d4cc28 100644 --- a/drivers/char/tpm/eventlog/acpi.c +++ b/drivers/char/tpm/eventlog/acpi.c @@ -26,6 +26,8 @@ #include "../tpm.h" #include "common.h" +#define MAX_TPM_LOG_LEN (128 * 1024) + struct acpi_tcpa { struct acpi_table_header hdr; u16 platform_class; @@ -135,6 +137,12 @@ int tpm_read_log_acpi(struct tpm_chip *chip) return -EIO; } + if (len > MAX_TPM_LOG_LEN) { + dev_warn(&chip->dev, "Excessive TCPA log len %llu truncated to %u bytes\n", + len, MAX_TPM_LOG_LEN); + len = MAX_TPM_LOG_LEN; + } + /* malloc EventLog space */ log->bios_event_log = devm_kmalloc(&chip->dev, len, GFP_KERNEL); if (!log->bios_event_log)
The TPM2 ACPI BIOS eventlog of a particular machine indicates that the length of the log is 4MB, even though the actual length of its useful data, when dumped, are only 69kb. To avoid allocating excessive amounts of memory for the event log, limit the size of any eventlog to 128kb. This should be sufficient memory and also not unnecessarily truncate event logs on any other machine. Reported-by: Andy Liang <andy.liang@hpe.com> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219495 Cc: Takashi Iwai <tiwai@suse.de> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- drivers/char/tpm/eventlog/acpi.c | 8 ++++++++ 1 file changed, 8 insertions(+)