On Thu, 07 Nov 2024 13:17:33 +0100, Paul Menzel wrote: > > Dear Takashi, > > > Thank you for the patch. > > Am 07.11.24 um 12:18 schrieb Takashi Iwai: > > The TPM2 ACPI table may request a large size for the event log, and it > > may be over the max size of kmalloc(). When this happens, the driver > > What is kmalloc()’s maximum size?
128kB or so, IIRC. And according Andy, the table can be over 4MB. > > spews the kernel WARNING at the probe time, but the error is > > eventually ignored in the caller side, and it results in the missing > > TPM event log exposure. > > > > This patch replaces the devm_kmalloc() call with kvmalloc() to allow > > larger sizes. Since there is no devm variant for kvmalloc(), now it's > > managed manually via devres_alloc() and devres_add(). > > As the access to the bug report is restricted, are you at liberty to > share the system you’ve seen this on? Likely yes, as it was reported to SLE15. Sorry for that. Basically the info provided there was almost what I put in the description; the driver got the kernel WARNING and Andy tested my patch. If any further info is required, at best ask HPE people here in Cc. thanks, Takashi > > Reported-and-tested-by: Andy Liang <[email protected]> > > Cc: [email protected] > > Link: https://bugzilla.suse.com/show_bug.cgi?id=1232421 > > Signed-off-by: Takashi Iwai <[email protected]> > > --- > > drivers/char/tpm/eventlog/acpi.c | 21 ++++++++++++++++++--- > > 1 file changed, 18 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/char/tpm/eventlog/acpi.c > > b/drivers/char/tpm/eventlog/acpi.c > > index 69533d0bfb51..56f7d73fa6bf 100644 > > --- a/drivers/char/tpm/eventlog/acpi.c > > +++ b/drivers/char/tpm/eventlog/acpi.c > > @@ -63,6 +63,13 @@ static bool tpm_is_tpm2_log(void *bios_event_log, u64 > > len) > > return n == 0; > > } > > +static void bios_event_log_release(struct device *dev, void *res) > > +{ > > + void **logp = res; > > + > > + kvfree(*logp); > > +} > > + > > /* read binary bios log */ > > int tpm_read_log_acpi(struct tpm_chip *chip) > > { > > @@ -71,6 +78,7 @@ int tpm_read_log_acpi(struct tpm_chip *chip) > > void __iomem *virt; > > u64 len, start; > > struct tpm_bios_log *log; > > + void **logp; > > struct acpi_table_tpm2 *tbl; > > struct acpi_tpm2_phy *tpm2_phy; > > int format; > > @@ -136,9 +144,16 @@ int tpm_read_log_acpi(struct tpm_chip *chip) > > } > > /* malloc EventLog space */ > > - log->bios_event_log = devm_kmalloc(&chip->dev, len, GFP_KERNEL); > > - if (!log->bios_event_log) > > + logp = devres_alloc(bios_event_log_release, sizeof(*logp), GFP_KERNEL); > > + if (!logp) > > return -ENOMEM; > > + devres_add(&chip->dev, logp); > > + log->bios_event_log = kvmalloc(len, GFP_KERNEL); > > + if (!log->bios_event_log) { > > + ret = -ENOMEM; > > + goto err; > > + } > > + *logp = log->bios_event_log; > > log->bios_event_log_end = log->bios_event_log + len; > > @@ -164,7 +179,7 @@ int tpm_read_log_acpi(struct tpm_chip *chip) > > return format; > > err: > > - devm_kfree(&chip->dev, log->bios_event_log); > > + devres_release(&chip->dev, bios_event_log_release, NULL, NULL); > > log->bios_event_log = NULL; > > return ret; > > } > > The diff looks good to me. > > > Kind regards, > > Paul
