| Message ID | 20241227060053.174314-2-jarkko@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v7,1/2] tpm: Map the ACPI provided event log | expand |
On Fri, 27 Dec 2024 at 07:01, Jarkko Sakkinen <jarkko@kernel.org> wrote: > > The following failure was reported: > > [ 10.693310][ T1] tpm_tis STM0925:00: 2.0 TPM (device-id 0x3, rev-id 0) > [ 10.848132][ T1] ------------[ cut here ]------------ > [ 10.853559][ T1] WARNING: CPU: 59 PID: 1 at mm/page_alloc.c:4727 __alloc_pages_noprof+0x2ca/0x330 > [ 10.862827][ T1] Modules linked in: > [ 10.866671][ T1] CPU: 59 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.12.0-lp155.2.g52785e2-default #1 openSUSE Tumbleweed (unreleased) 588cd98293a7c9eba9013378d807364c088c9375 > [ 10.882741][ T1] Hardware name: HPE ProLiant DL320 Gen12/ProLiant DL320 Gen12, BIOS 1.20 10/28/2024 > [ 10.892170][ T1] RIP: 0010:__alloc_pages_noprof+0x2ca/0x330 > [ 10.898103][ T1] Code: 24 08 e9 4a fe ff ff e8 34 36 fa ff e9 88 fe ff ff 83 fe 0a 0f 86 b3 fd ff ff 80 3d 01 e7 ce 01 00 75 09 c6 05 f8 e6 ce 01 01 <0f> 0b 45 31 ff e9 e5 fe ff ff f7 c2 00 00 08 00 75 42 89 d9 80 e1 > [ 10.917750][ T1] RSP: 0000:ffffb7cf40077980 EFLAGS: 00010246 > [ 10.923777][ T1] RAX: 0000000000000000 RBX: 0000000000040cc0 RCX: 0000000000000000 > [ 10.931727][ T1] RDX: 0000000000000000 RSI: 000000000000000c RDI: 0000000000040cc0 > > Above shows that ACPI pointed a 16 MiB buffer for the log events because > RSI maps to the 'order' parameter of __alloc_pages_noprof(). Address the > bug by mapping the region when needed instead of copying. > > Earlier fix does address the warning but wastes 16 MiB of memory. This > finalizes the fix. > > Suggested-by: Matthew Garrett <mjg59@srcf.ucam.org> > Tested-by: Andy Liang <andy.liang@hpe.com> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org> Where are patch 1/2 and the cover letter? > --- > v7: > * Fixed up tags. > * I did consider traverse and thus get smaller buffer, and then using > devm_kmalloc() but looking at e.g. drivers/acpi/apei/einj-core.c > it is not necessary. > v6: > * Updated commit message. > v5: > * Spotted this right after sending: remove extra acpi_os_unmap_iomem() > call. > v4: > * Added tested-by from Andy Liang. > v3: > * Flag mapping code in tpm{1,2}.c with CONFIG_ACPI (nios2 compilation > fix). > v2: > * There was some extra cruft (irrelevant diff), which is now wiped away. > * Added missing tags (fixes, stable). > --- > drivers/char/tpm/eventlog/acpi.c | 33 ++++++-------------------- > drivers/char/tpm/eventlog/common.c | 25 +++++++++++++------- > drivers/char/tpm/eventlog/common.h | 28 ++++++++++++++++++++++ > drivers/char/tpm/eventlog/tpm1.c | 30 ++++++++++++++--------- > drivers/char/tpm/eventlog/tpm2.c | 38 +++++++++++++++++------------- > include/linux/tpm.h | 1 + > 6 files changed, 94 insertions(+), 61 deletions(-) > > diff --git a/drivers/char/tpm/eventlog/acpi.c b/drivers/char/tpm/eventlog/acpi.c > index 394c8302cefd..a11f78e41e1e 100644 > --- a/drivers/char/tpm/eventlog/acpi.c > +++ b/drivers/char/tpm/eventlog/acpi.c > @@ -75,14 +75,11 @@ int tpm_read_log_acpi(struct tpm_chip *chip) > acpi_status status; > void __iomem *virt; > u64 len, start; > - struct tpm_bios_log *log; > struct acpi_table_tpm2 *tbl; > struct acpi_tpm2_phy *tpm2_phy; > int format; > int ret; > > - log = &chip->log; > - > /* Unfortuntely ACPI does not associate the event log with a specific > * TPM, like PPI. Thus all ACPI TPMs will read the same log. > */ > @@ -140,42 +137,26 @@ int tpm_read_log_acpi(struct tpm_chip *chip) > return -EIO; > } > > - /* malloc EventLog space */ > - log->bios_event_log = kvmalloc(len, GFP_KERNEL); > - if (!log->bios_event_log) > - return -ENOMEM; > - > - ret = devm_add_action_or_reset(&chip->dev, tpm_bios_log_free, log->bios_event_log); > - if (ret) { > - log->bios_event_log = NULL; > - return ret; > - } > - > - log->bios_event_log_end = log->bios_event_log + len; > - > virt = acpi_os_map_iomem(start, len); > if (!virt) { > dev_warn(&chip->dev, "%s: Failed to map ACPI memory\n", __func__); > /* try EFI log next */ > - ret = -ENODEV; > - goto err; > + return -ENODEV; > } > > - memcpy_fromio(log->bios_event_log, virt, len); > - > - acpi_os_unmap_iomem(virt, len); > - > - if (chip->flags & TPM_CHIP_FLAG_TPM2 && > - !tpm_is_tpm2_log(log->bios_event_log, len)) { > + if (chip->flags & TPM_CHIP_FLAG_TPM2 && !tpm_is_tpm2_log(virt, len)) { > /* try EFI log next */ > ret = -ENODEV; > goto err; > } > > + acpi_os_unmap_iomem(virt, len); > + chip->flags |= TPM_CHIP_FLAG_ACPI_LOG; > + chip->log.bios_event_log = (void *)start; > + chip->log.bios_event_log_end = (void *)start + len; > return format; > > err: > - devm_kfree(&chip->dev, log->bios_event_log); > - log->bios_event_log = NULL; > + acpi_os_unmap_iomem(virt, len); > return ret; > } > diff --git a/drivers/char/tpm/eventlog/common.c b/drivers/char/tpm/eventlog/common.c > index 4c0bbba64ee5..44340ca6e2ac 100644 > --- a/drivers/char/tpm/eventlog/common.c > +++ b/drivers/char/tpm/eventlog/common.c > @@ -27,6 +27,7 @@ static int tpm_bios_measurements_open(struct inode *inode, > { > int err; > struct seq_file *seq; > + struct tpm_measurements *priv; > struct tpm_chip_seqops *chip_seqops; > const struct seq_operations *seqops; > struct tpm_chip *chip; > @@ -42,13 +43,18 @@ static int tpm_bios_measurements_open(struct inode *inode, > get_device(&chip->dev); > inode_unlock(inode); > > - /* now register seq file */ > + priv = kzalloc(sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + priv->chip = chip; > + > err = seq_open(file, seqops); > - if (!err) { > - seq = file->private_data; > - seq->private = chip; > - } else { > + if (err) { > + kfree(priv); > put_device(&chip->dev); > + } else { > + seq = file->private_data; > + seq->private = priv; > } > > return err; > @@ -58,11 +64,14 @@ static int tpm_bios_measurements_release(struct inode *inode, > struct file *file) > { > struct seq_file *seq = file->private_data; > - struct tpm_chip *chip = seq->private; > + struct tpm_measurements *priv = seq->private; > + int ret; > > - put_device(&chip->dev); > + put_device(&priv->chip->dev); > + ret = seq_release(inode, file); > + kfree(priv); > > - return seq_release(inode, file); > + return ret; > } > > static const struct file_operations tpm_bios_measurements_ops = { > diff --git a/drivers/char/tpm/eventlog/common.h b/drivers/char/tpm/eventlog/common.h > index 47ff8136ceb5..b98fd6d9a6e9 100644 > --- a/drivers/char/tpm/eventlog/common.h > +++ b/drivers/char/tpm/eventlog/common.h > @@ -1,12 +1,40 @@ > #ifndef __TPM_EVENTLOG_COMMON_H__ > #define __TPM_EVENTLOG_COMMON_H__ > > +#include <linux/acpi.h> > #include "../tpm.h" > > extern const struct seq_operations tpm1_ascii_b_measurements_seqops; > extern const struct seq_operations tpm1_binary_b_measurements_seqops; > extern const struct seq_operations tpm2_binary_b_measurements_seqops; > > +struct tpm_measurements { > + struct tpm_chip *chip; > + void *start; > + void *end; > +}; > + > +static inline bool tpm_measurements_map(struct tpm_measurements *measurements) > +{ > + struct tpm_chip *chip = measurements->chip; > + struct tpm_bios_log *log = &chip->log; > + size_t size; > + > + size = log->bios_event_log_end - log->bios_event_log; > + measurements->start = log->bios_event_log; > + > +#ifdef CONFIG_ACPI > + if (chip->flags & TPM_CHIP_FLAG_ACPI_LOG) > + measurements->start = acpi_os_map_iomem((unsigned long)log->bios_event_log, size); > +#endif > + > + if (!measurements->start) > + return false; > + > + measurements->end = measurements->start + size; > + return true; > +} > + > #if defined(CONFIG_ACPI) > int tpm_read_log_acpi(struct tpm_chip *chip); > #else > diff --git a/drivers/char/tpm/eventlog/tpm1.c b/drivers/char/tpm/eventlog/tpm1.c > index 12ee42a31c71..aef6ee39423a 100644 > --- a/drivers/char/tpm/eventlog/tpm1.c > +++ b/drivers/char/tpm/eventlog/tpm1.c > @@ -70,20 +70,23 @@ static const char* tcpa_pc_event_id_strings[] = { > static void *tpm1_bios_measurements_start(struct seq_file *m, loff_t *pos) > { > loff_t i = 0; > - struct tpm_chip *chip = m->private; > - struct tpm_bios_log *log = &chip->log; > - void *addr = log->bios_event_log; > - void *limit = log->bios_event_log_end; > + struct tpm_measurements *priv = m->private; > struct tcpa_event *event; > u32 converted_event_size; > u32 converted_event_type; > + void *addr; > + > + if (!tpm_measurements_map(priv)) > + return NULL; > + > + addr = priv->start; > > /* read over *pos measurements */ > do { > event = addr; > > /* check if current entry is valid */ > - if (addr + sizeof(struct tcpa_event) > limit) > + if (addr + sizeof(struct tcpa_event) > priv->end) > return NULL; > > converted_event_size = > @@ -93,7 +96,7 @@ static void *tpm1_bios_measurements_start(struct seq_file *m, loff_t *pos) > > if (((converted_event_type == 0) && (converted_event_size == 0)) > || ((addr + sizeof(struct tcpa_event) + converted_event_size) > - > limit)) > + > priv->end)) > return NULL; > > if (i++ == *pos) > @@ -109,9 +112,7 @@ static void *tpm1_bios_measurements_next(struct seq_file *m, void *v, > loff_t *pos) > { > struct tcpa_event *event = v; > - struct tpm_chip *chip = m->private; > - struct tpm_bios_log *log = &chip->log; > - void *limit = log->bios_event_log_end; > + struct tpm_measurements *priv = m->private; > u32 converted_event_size; > u32 converted_event_type; > > @@ -121,7 +122,7 @@ static void *tpm1_bios_measurements_next(struct seq_file *m, void *v, > v += sizeof(struct tcpa_event) + converted_event_size; > > /* now check if current entry is valid */ > - if ((v + sizeof(struct tcpa_event)) > limit) > + if ((v + sizeof(struct tcpa_event)) > priv->end) > return NULL; > > event = v; > @@ -130,7 +131,7 @@ static void *tpm1_bios_measurements_next(struct seq_file *m, void *v, > converted_event_type = do_endian_conversion(event->event_type); > > if (((converted_event_type == 0) && (converted_event_size == 0)) || > - ((v + sizeof(struct tcpa_event) + converted_event_size) > limit)) > + ((v + sizeof(struct tcpa_event) + converted_event_size) > priv->end)) > return NULL; > > return v; > @@ -138,6 +139,13 @@ static void *tpm1_bios_measurements_next(struct seq_file *m, void *v, > > static void tpm1_bios_measurements_stop(struct seq_file *m, void *v) > { > +#ifdef CONFIG_ACPI > + struct tpm_measurements *priv = m->private; > + struct tpm_chip *chip = priv->chip; > + > + if (chip->flags & TPM_CHIP_FLAG_ACPI_LOG) > + acpi_os_unmap_iomem(priv->start, priv->end - priv->start); > +#endif > } > > static int get_event_name(char *dest, struct tcpa_event *event, > diff --git a/drivers/char/tpm/eventlog/tpm2.c b/drivers/char/tpm/eventlog/tpm2.c > index 37a05800980c..6289d8893e46 100644 > --- a/drivers/char/tpm/eventlog/tpm2.c > +++ b/drivers/char/tpm/eventlog/tpm2.c > @@ -41,20 +41,22 @@ static size_t calc_tpm2_event_size(struct tcg_pcr_event2_head *event, > > static void *tpm2_bios_measurements_start(struct seq_file *m, loff_t *pos) > { > - struct tpm_chip *chip = m->private; > - struct tpm_bios_log *log = &chip->log; > - void *addr = log->bios_event_log; > - void *limit = log->bios_event_log_end; > + struct tpm_measurements *priv = m->private; > struct tcg_pcr_event *event_header; > struct tcg_pcr_event2_head *event; > size_t size; > + void *addr; > int i; > > + if (!tpm_measurements_map(priv)) > + return NULL; > + > + addr = priv->start; > event_header = addr; > size = struct_size(event_header, event, event_header->event_size); > > if (*pos == 0) { > - if (addr + size < limit) { > + if (addr + size < priv->end) { > if ((event_header->event_type == 0) && > (event_header->event_size == 0)) > return NULL; > @@ -66,7 +68,7 @@ static void *tpm2_bios_measurements_start(struct seq_file *m, loff_t *pos) > addr += size; > event = addr; > size = calc_tpm2_event_size(event, event_header); > - if ((addr + size >= limit) || (size == 0)) > + if ((addr + size >= priv->end) || !size) > return NULL; > } > > @@ -74,7 +76,7 @@ static void *tpm2_bios_measurements_start(struct seq_file *m, loff_t *pos) > event = addr; > size = calc_tpm2_event_size(event, event_header); > > - if ((addr + size >= limit) || (size == 0)) > + if ((addr + size >= priv->end) || !size) > return NULL; > addr += size; > } > @@ -87,14 +89,12 @@ static void *tpm2_bios_measurements_next(struct seq_file *m, void *v, > { > struct tcg_pcr_event *event_header; > struct tcg_pcr_event2_head *event; > - struct tpm_chip *chip = m->private; > - struct tpm_bios_log *log = &chip->log; > - void *limit = log->bios_event_log_end; > + struct tpm_measurements *priv = m->private; > size_t event_size; > void *marker; > > (*pos)++; > - event_header = log->bios_event_log; > + event_header = priv->start; > > if (v == SEQ_START_TOKEN) { > event_size = struct_size(event_header, event, > @@ -109,13 +109,13 @@ static void *tpm2_bios_measurements_next(struct seq_file *m, void *v, > } > > marker = marker + event_size; > - if (marker >= limit) > + if (marker >= priv->end) > return NULL; > v = marker; > event = v; > > event_size = calc_tpm2_event_size(event, event_header); > - if (((v + event_size) >= limit) || (event_size == 0)) > + if (((v + event_size) >= priv->end) || !event_size) > return NULL; > > return v; > @@ -123,13 +123,19 @@ static void *tpm2_bios_measurements_next(struct seq_file *m, void *v, > > static void tpm2_bios_measurements_stop(struct seq_file *m, void *v) > { > +#ifdef CONFIG_ACPI > + struct tpm_measurements *priv = m->private; > + struct tpm_chip *chip = priv->chip; > + > + if (chip->flags & TPM_CHIP_FLAG_ACPI_LOG) > + acpi_os_unmap_iomem(priv->start, priv->end - priv->start); > +#endif > } > > static int tpm2_binary_bios_measurements_show(struct seq_file *m, void *v) > { > - struct tpm_chip *chip = m->private; > - struct tpm_bios_log *log = &chip->log; > - struct tcg_pcr_event *event_header = log->bios_event_log; > + struct tpm_measurements *priv = m->private; > + struct tcg_pcr_event *event_header = priv->start; > struct tcg_pcr_event2_head *event = v; > void *temp_ptr; > size_t size; > diff --git a/include/linux/tpm.h b/include/linux/tpm.h > index 20a40ade8030..f3d12738b93b 100644 > --- a/include/linux/tpm.h > +++ b/include/linux/tpm.h > @@ -348,6 +348,7 @@ enum tpm_chip_flags { > TPM_CHIP_FLAG_SUSPENDED = BIT(8), > TPM_CHIP_FLAG_HWRNG_DISABLED = BIT(9), > TPM_CHIP_FLAG_DISABLE = BIT(10), > + TPM_CHIP_FLAG_ACPI_LOG = BIT(11), > }; > > #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev) > -- > 2.47.1 >
On Fri Dec 27, 2024 at 10:43 AM EET, Ard Biesheuvel wrote: > On Fri, 27 Dec 2024 at 07:01, Jarkko Sakkinen <jarkko@kernel.org> wrote: > > > > The following failure was reported: > > > > [ 10.693310][ T1] tpm_tis STM0925:00: 2.0 TPM (device-id 0x3, rev-id 0) > > [ 10.848132][ T1] ------------[ cut here ]------------ > > [ 10.853559][ T1] WARNING: CPU: 59 PID: 1 at mm/page_alloc.c:4727 __alloc_pages_noprof+0x2ca/0x330 > > [ 10.862827][ T1] Modules linked in: > > [ 10.866671][ T1] CPU: 59 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.12.0-lp155.2.g52785e2-default #1 openSUSE Tumbleweed (unreleased) 588cd98293a7c9eba9013378d807364c088c9375 > > [ 10.882741][ T1] Hardware name: HPE ProLiant DL320 Gen12/ProLiant DL320 Gen12, BIOS 1.20 10/28/2024 > > [ 10.892170][ T1] RIP: 0010:__alloc_pages_noprof+0x2ca/0x330 > > [ 10.898103][ T1] Code: 24 08 e9 4a fe ff ff e8 34 36 fa ff e9 88 fe ff ff 83 fe 0a 0f 86 b3 fd ff ff 80 3d 01 e7 ce 01 00 75 09 c6 05 f8 e6 ce 01 01 <0f> 0b 45 31 ff e9 e5 fe ff ff f7 c2 00 00 08 00 75 42 89 d9 80 e1 > > [ 10.917750][ T1] RSP: 0000:ffffb7cf40077980 EFLAGS: 00010246 > > [ 10.923777][ T1] RAX: 0000000000000000 RBX: 0000000000040cc0 RCX: 0000000000000000 > > [ 10.931727][ T1] RDX: 0000000000000000 RSI: 000000000000000c RDI: 0000000000040cc0 > > > > Above shows that ACPI pointed a 16 MiB buffer for the log events because > > RSI maps to the 'order' parameter of __alloc_pages_noprof(). Address the > > bug by mapping the region when needed instead of copying. > > > > Earlier fix does address the warning but wastes 16 MiB of memory. This > > finalizes the fix. > > > > Suggested-by: Matthew Garrett <mjg59@srcf.ucam.org> > > Tested-by: Andy Liang <andy.liang@hpe.com> > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org> > > Where are patch 1/2 and the cover letter? https://lore.kernel.org/linux-integrity/20241227060053.174314-1-jarkko@kernel.org/T/#mc34835f6d6f946c50a580f895f24b7b1dab85204 You did not request a cover letter, and I did not think it was really necessary. I can do it for next version on request. BR, Jarkko
On Fri Dec 27, 2024 at 12:49 PM EET, Jarkko Sakkinen wrote: > On Fri Dec 27, 2024 at 10:43 AM EET, Ard Biesheuvel wrote: > > On Fri, 27 Dec 2024 at 07:01, Jarkko Sakkinen <jarkko@kernel.org> wrote: > > > > > > The following failure was reported: > > > > > > [ 10.693310][ T1] tpm_tis STM0925:00: 2.0 TPM (device-id 0x3, rev-id 0) > > > [ 10.848132][ T1] ------------[ cut here ]------------ > > > [ 10.853559][ T1] WARNING: CPU: 59 PID: 1 at mm/page_alloc.c:4727 __alloc_pages_noprof+0x2ca/0x330 > > > [ 10.862827][ T1] Modules linked in: > > > [ 10.866671][ T1] CPU: 59 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.12.0-lp155.2.g52785e2-default #1 openSUSE Tumbleweed (unreleased) 588cd98293a7c9eba9013378d807364c088c9375 > > > [ 10.882741][ T1] Hardware name: HPE ProLiant DL320 Gen12/ProLiant DL320 Gen12, BIOS 1.20 10/28/2024 > > > [ 10.892170][ T1] RIP: 0010:__alloc_pages_noprof+0x2ca/0x330 > > > [ 10.898103][ T1] Code: 24 08 e9 4a fe ff ff e8 34 36 fa ff e9 88 fe ff ff 83 fe 0a 0f 86 b3 fd ff ff 80 3d 01 e7 ce 01 00 75 09 c6 05 f8 e6 ce 01 01 <0f> 0b 45 31 ff e9 e5 fe ff ff f7 c2 00 00 08 00 75 42 89 d9 80 e1 > > > [ 10.917750][ T1] RSP: 0000:ffffb7cf40077980 EFLAGS: 00010246 > > > [ 10.923777][ T1] RAX: 0000000000000000 RBX: 0000000000040cc0 RCX: 0000000000000000 > > > [ 10.931727][ T1] RDX: 0000000000000000 RSI: 000000000000000c RDI: 0000000000040cc0 > > > > > > Above shows that ACPI pointed a 16 MiB buffer for the log events because > > > RSI maps to the 'order' parameter of __alloc_pages_noprof(). Address the > > > bug by mapping the region when needed instead of copying. > > > > > > Earlier fix does address the warning but wastes 16 MiB of memory. This > > > finalizes the fix. > > > > > > Suggested-by: Matthew Garrett <mjg59@srcf.ucam.org> > > > Tested-by: Andy Liang <andy.liang@hpe.com> > > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org> > > > > Where are patch 1/2 and the cover letter? > > https://lore.kernel.org/linux-integrity/20241227060053.174314-1-jarkko@kernel.org/T/#mc34835f6d6f946c50a580f895f24b7b1dab85204 > > You did not request a cover letter, and I did not think it was > really necessary. I can do it for next version on request. Hmm.. right so this is how I generate Cc's: tocmd ="`pwd`/scripts/get_maintainer.pl --norolestats --nol" cccmd ="`pwd`/scripts/get_maintainer.pl --norolestats --nom" It clearly did not pick you (sorry about that). I'll add manual cc to the next version. And yeah, cover letter is fine, but I did not see any (practical) use for it as the patches carry change logs. BR, Jarkko
On Fri Dec 27, 2024 at 12:52 PM EET, Jarkko Sakkinen wrote: > On Fri Dec 27, 2024 at 12:49 PM EET, Jarkko Sakkinen wrote: > > On Fri Dec 27, 2024 at 10:43 AM EET, Ard Biesheuvel wrote: > > > On Fri, 27 Dec 2024 at 07:01, Jarkko Sakkinen <jarkko@kernel.org> wrote: > > > > > > > > The following failure was reported: > > > > > > > > [ 10.693310][ T1] tpm_tis STM0925:00: 2.0 TPM (device-id 0x3, rev-id 0) > > > > [ 10.848132][ T1] ------------[ cut here ]------------ > > > > [ 10.853559][ T1] WARNING: CPU: 59 PID: 1 at mm/page_alloc.c:4727 __alloc_pages_noprof+0x2ca/0x330 > > > > [ 10.862827][ T1] Modules linked in: > > > > [ 10.866671][ T1] CPU: 59 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.12.0-lp155.2.g52785e2-default #1 openSUSE Tumbleweed (unreleased) 588cd98293a7c9eba9013378d807364c088c9375 > > > > [ 10.882741][ T1] Hardware name: HPE ProLiant DL320 Gen12/ProLiant DL320 Gen12, BIOS 1.20 10/28/2024 > > > > [ 10.892170][ T1] RIP: 0010:__alloc_pages_noprof+0x2ca/0x330 > > > > [ 10.898103][ T1] Code: 24 08 e9 4a fe ff ff e8 34 36 fa ff e9 88 fe ff ff 83 fe 0a 0f 86 b3 fd ff ff 80 3d 01 e7 ce 01 00 75 09 c6 05 f8 e6 ce 01 01 <0f> 0b 45 31 ff e9 e5 fe ff ff f7 c2 00 00 08 00 75 42 89 d9 80 e1 > > > > [ 10.917750][ T1] RSP: 0000:ffffb7cf40077980 EFLAGS: 00010246 > > > > [ 10.923777][ T1] RAX: 0000000000000000 RBX: 0000000000040cc0 RCX: 0000000000000000 > > > > [ 10.931727][ T1] RDX: 0000000000000000 RSI: 000000000000000c RDI: 0000000000040cc0 > > > > > > > > Above shows that ACPI pointed a 16 MiB buffer for the log events because > > > > RSI maps to the 'order' parameter of __alloc_pages_noprof(). Address the > > > > bug by mapping the region when needed instead of copying. > > > > > > > > Earlier fix does address the warning but wastes 16 MiB of memory. This > > > > finalizes the fix. > > > > > > > > Suggested-by: Matthew Garrett <mjg59@srcf.ucam.org> > > > > Tested-by: Andy Liang <andy.liang@hpe.com> > > > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org> > > > > > > Where are patch 1/2 and the cover letter? > > > > https://lore.kernel.org/linux-integrity/20241227060053.174314-1-jarkko@kernel.org/T/#mc34835f6d6f946c50a580f895f24b7b1dab85204 > > > > You did not request a cover letter, and I did not think it was > > really necessary. I can do it for next version on request. > > Hmm.. right so this is how I generate Cc's: > > tocmd ="`pwd`/scripts/get_maintainer.pl --norolestats --nol" > cccmd ="`pwd`/scripts/get_maintainer.pl --norolestats --nom" > > It clearly did not pick you (sorry about that). I'll add manual cc to > the next version. And yeah, cover letter is fine, but I did not see any > (practical) use for it as the patches carry change logs. I'll reduce this to quick fix after reconsidering. We have the mapped version drafted and backed up in lore. I'll re-send just that with and CC you. Or actually I'll put suggested-by tag. BR, Jarkko
diff --git a/drivers/char/tpm/eventlog/acpi.c b/drivers/char/tpm/eventlog/acpi.c index 394c8302cefd..a11f78e41e1e 100644 --- a/drivers/char/tpm/eventlog/acpi.c +++ b/drivers/char/tpm/eventlog/acpi.c @@ -75,14 +75,11 @@ int tpm_read_log_acpi(struct tpm_chip *chip) acpi_status status; void __iomem *virt; u64 len, start; - struct tpm_bios_log *log; struct acpi_table_tpm2 *tbl; struct acpi_tpm2_phy *tpm2_phy; int format; int ret; - log = &chip->log; - /* Unfortuntely ACPI does not associate the event log with a specific * TPM, like PPI. Thus all ACPI TPMs will read the same log. */ @@ -140,42 +137,26 @@ int tpm_read_log_acpi(struct tpm_chip *chip) return -EIO; } - /* malloc EventLog space */ - log->bios_event_log = kvmalloc(len, GFP_KERNEL); - if (!log->bios_event_log) - return -ENOMEM; - - ret = devm_add_action_or_reset(&chip->dev, tpm_bios_log_free, log->bios_event_log); - if (ret) { - log->bios_event_log = NULL; - return ret; - } - - log->bios_event_log_end = log->bios_event_log + len; - virt = acpi_os_map_iomem(start, len); if (!virt) { dev_warn(&chip->dev, "%s: Failed to map ACPI memory\n", __func__); /* try EFI log next */ - ret = -ENODEV; - goto err; + return -ENODEV; } - memcpy_fromio(log->bios_event_log, virt, len); - - acpi_os_unmap_iomem(virt, len); - - if (chip->flags & TPM_CHIP_FLAG_TPM2 && - !tpm_is_tpm2_log(log->bios_event_log, len)) { + if (chip->flags & TPM_CHIP_FLAG_TPM2 && !tpm_is_tpm2_log(virt, len)) { /* try EFI log next */ ret = -ENODEV; goto err; } + acpi_os_unmap_iomem(virt, len); + chip->flags |= TPM_CHIP_FLAG_ACPI_LOG; + chip->log.bios_event_log = (void *)start; + chip->log.bios_event_log_end = (void *)start + len; return format; err: - devm_kfree(&chip->dev, log->bios_event_log); - log->bios_event_log = NULL; + acpi_os_unmap_iomem(virt, len); return ret; } diff --git a/drivers/char/tpm/eventlog/common.c b/drivers/char/tpm/eventlog/common.c index 4c0bbba64ee5..44340ca6e2ac 100644 --- a/drivers/char/tpm/eventlog/common.c +++ b/drivers/char/tpm/eventlog/common.c @@ -27,6 +27,7 @@ static int tpm_bios_measurements_open(struct inode *inode, { int err; struct seq_file *seq; + struct tpm_measurements *priv; struct tpm_chip_seqops *chip_seqops; const struct seq_operations *seqops; struct tpm_chip *chip; @@ -42,13 +43,18 @@ static int tpm_bios_measurements_open(struct inode *inode, get_device(&chip->dev); inode_unlock(inode); - /* now register seq file */ + priv = kzalloc(sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + priv->chip = chip; + err = seq_open(file, seqops); - if (!err) { - seq = file->private_data; - seq->private = chip; - } else { + if (err) { + kfree(priv); put_device(&chip->dev); + } else { + seq = file->private_data; + seq->private = priv; } return err; @@ -58,11 +64,14 @@ static int tpm_bios_measurements_release(struct inode *inode, struct file *file) { struct seq_file *seq = file->private_data; - struct tpm_chip *chip = seq->private; + struct tpm_measurements *priv = seq->private; + int ret; - put_device(&chip->dev); + put_device(&priv->chip->dev); + ret = seq_release(inode, file); + kfree(priv); - return seq_release(inode, file); + return ret; } static const struct file_operations tpm_bios_measurements_ops = { diff --git a/drivers/char/tpm/eventlog/common.h b/drivers/char/tpm/eventlog/common.h index 47ff8136ceb5..b98fd6d9a6e9 100644 --- a/drivers/char/tpm/eventlog/common.h +++ b/drivers/char/tpm/eventlog/common.h @@ -1,12 +1,40 @@ #ifndef __TPM_EVENTLOG_COMMON_H__ #define __TPM_EVENTLOG_COMMON_H__ +#include <linux/acpi.h> #include "../tpm.h" extern const struct seq_operations tpm1_ascii_b_measurements_seqops; extern const struct seq_operations tpm1_binary_b_measurements_seqops; extern const struct seq_operations tpm2_binary_b_measurements_seqops; +struct tpm_measurements { + struct tpm_chip *chip; + void *start; + void *end; +}; + +static inline bool tpm_measurements_map(struct tpm_measurements *measurements) +{ + struct tpm_chip *chip = measurements->chip; + struct tpm_bios_log *log = &chip->log; + size_t size; + + size = log->bios_event_log_end - log->bios_event_log; + measurements->start = log->bios_event_log; + +#ifdef CONFIG_ACPI + if (chip->flags & TPM_CHIP_FLAG_ACPI_LOG) + measurements->start = acpi_os_map_iomem((unsigned long)log->bios_event_log, size); +#endif + + if (!measurements->start) + return false; + + measurements->end = measurements->start + size; + return true; +} + #if defined(CONFIG_ACPI) int tpm_read_log_acpi(struct tpm_chip *chip); #else diff --git a/drivers/char/tpm/eventlog/tpm1.c b/drivers/char/tpm/eventlog/tpm1.c index 12ee42a31c71..aef6ee39423a 100644 --- a/drivers/char/tpm/eventlog/tpm1.c +++ b/drivers/char/tpm/eventlog/tpm1.c @@ -70,20 +70,23 @@ static const char* tcpa_pc_event_id_strings[] = { static void *tpm1_bios_measurements_start(struct seq_file *m, loff_t *pos) { loff_t i = 0; - struct tpm_chip *chip = m->private; - struct tpm_bios_log *log = &chip->log; - void *addr = log->bios_event_log; - void *limit = log->bios_event_log_end; + struct tpm_measurements *priv = m->private; struct tcpa_event *event; u32 converted_event_size; u32 converted_event_type; + void *addr; + + if (!tpm_measurements_map(priv)) + return NULL; + + addr = priv->start; /* read over *pos measurements */ do { event = addr; /* check if current entry is valid */ - if (addr + sizeof(struct tcpa_event) > limit) + if (addr + sizeof(struct tcpa_event) > priv->end) return NULL; converted_event_size = @@ -93,7 +96,7 @@ static void *tpm1_bios_measurements_start(struct seq_file *m, loff_t *pos) if (((converted_event_type == 0) && (converted_event_size == 0)) || ((addr + sizeof(struct tcpa_event) + converted_event_size) - > limit)) + > priv->end)) return NULL; if (i++ == *pos) @@ -109,9 +112,7 @@ static void *tpm1_bios_measurements_next(struct seq_file *m, void *v, loff_t *pos) { struct tcpa_event *event = v; - struct tpm_chip *chip = m->private; - struct tpm_bios_log *log = &chip->log; - void *limit = log->bios_event_log_end; + struct tpm_measurements *priv = m->private; u32 converted_event_size; u32 converted_event_type; @@ -121,7 +122,7 @@ static void *tpm1_bios_measurements_next(struct seq_file *m, void *v, v += sizeof(struct tcpa_event) + converted_event_size; /* now check if current entry is valid */ - if ((v + sizeof(struct tcpa_event)) > limit) + if ((v + sizeof(struct tcpa_event)) > priv->end) return NULL; event = v; @@ -130,7 +131,7 @@ static void *tpm1_bios_measurements_next(struct seq_file *m, void *v, converted_event_type = do_endian_conversion(event->event_type); if (((converted_event_type == 0) && (converted_event_size == 0)) || - ((v + sizeof(struct tcpa_event) + converted_event_size) > limit)) + ((v + sizeof(struct tcpa_event) + converted_event_size) > priv->end)) return NULL; return v; @@ -138,6 +139,13 @@ static void *tpm1_bios_measurements_next(struct seq_file *m, void *v, static void tpm1_bios_measurements_stop(struct seq_file *m, void *v) { +#ifdef CONFIG_ACPI + struct tpm_measurements *priv = m->private; + struct tpm_chip *chip = priv->chip; + + if (chip->flags & TPM_CHIP_FLAG_ACPI_LOG) + acpi_os_unmap_iomem(priv->start, priv->end - priv->start); +#endif } static int get_event_name(char *dest, struct tcpa_event *event, diff --git a/drivers/char/tpm/eventlog/tpm2.c b/drivers/char/tpm/eventlog/tpm2.c index 37a05800980c..6289d8893e46 100644 --- a/drivers/char/tpm/eventlog/tpm2.c +++ b/drivers/char/tpm/eventlog/tpm2.c @@ -41,20 +41,22 @@ static size_t calc_tpm2_event_size(struct tcg_pcr_event2_head *event, static void *tpm2_bios_measurements_start(struct seq_file *m, loff_t *pos) { - struct tpm_chip *chip = m->private; - struct tpm_bios_log *log = &chip->log; - void *addr = log->bios_event_log; - void *limit = log->bios_event_log_end; + struct tpm_measurements *priv = m->private; struct tcg_pcr_event *event_header; struct tcg_pcr_event2_head *event; size_t size; + void *addr; int i; + if (!tpm_measurements_map(priv)) + return NULL; + + addr = priv->start; event_header = addr; size = struct_size(event_header, event, event_header->event_size); if (*pos == 0) { - if (addr + size < limit) { + if (addr + size < priv->end) { if ((event_header->event_type == 0) && (event_header->event_size == 0)) return NULL; @@ -66,7 +68,7 @@ static void *tpm2_bios_measurements_start(struct seq_file *m, loff_t *pos) addr += size; event = addr; size = calc_tpm2_event_size(event, event_header); - if ((addr + size >= limit) || (size == 0)) + if ((addr + size >= priv->end) || !size) return NULL; } @@ -74,7 +76,7 @@ static void *tpm2_bios_measurements_start(struct seq_file *m, loff_t *pos) event = addr; size = calc_tpm2_event_size(event, event_header); - if ((addr + size >= limit) || (size == 0)) + if ((addr + size >= priv->end) || !size) return NULL; addr += size; } @@ -87,14 +89,12 @@ static void *tpm2_bios_measurements_next(struct seq_file *m, void *v, { struct tcg_pcr_event *event_header; struct tcg_pcr_event2_head *event; - struct tpm_chip *chip = m->private; - struct tpm_bios_log *log = &chip->log; - void *limit = log->bios_event_log_end; + struct tpm_measurements *priv = m->private; size_t event_size; void *marker; (*pos)++; - event_header = log->bios_event_log; + event_header = priv->start; if (v == SEQ_START_TOKEN) { event_size = struct_size(event_header, event, @@ -109,13 +109,13 @@ static void *tpm2_bios_measurements_next(struct seq_file *m, void *v, } marker = marker + event_size; - if (marker >= limit) + if (marker >= priv->end) return NULL; v = marker; event = v; event_size = calc_tpm2_event_size(event, event_header); - if (((v + event_size) >= limit) || (event_size == 0)) + if (((v + event_size) >= priv->end) || !event_size) return NULL; return v; @@ -123,13 +123,19 @@ static void *tpm2_bios_measurements_next(struct seq_file *m, void *v, static void tpm2_bios_measurements_stop(struct seq_file *m, void *v) { +#ifdef CONFIG_ACPI + struct tpm_measurements *priv = m->private; + struct tpm_chip *chip = priv->chip; + + if (chip->flags & TPM_CHIP_FLAG_ACPI_LOG) + acpi_os_unmap_iomem(priv->start, priv->end - priv->start); +#endif } static int tpm2_binary_bios_measurements_show(struct seq_file *m, void *v) { - struct tpm_chip *chip = m->private; - struct tpm_bios_log *log = &chip->log; - struct tcg_pcr_event *event_header = log->bios_event_log; + struct tpm_measurements *priv = m->private; + struct tcg_pcr_event *event_header = priv->start; struct tcg_pcr_event2_head *event = v; void *temp_ptr; size_t size; diff --git a/include/linux/tpm.h b/include/linux/tpm.h index 20a40ade8030..f3d12738b93b 100644 --- a/include/linux/tpm.h +++ b/include/linux/tpm.h @@ -348,6 +348,7 @@ enum tpm_chip_flags { TPM_CHIP_FLAG_SUSPENDED = BIT(8), TPM_CHIP_FLAG_HWRNG_DISABLED = BIT(9), TPM_CHIP_FLAG_DISABLE = BIT(10), + TPM_CHIP_FLAG_ACPI_LOG = BIT(11), }; #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)