Re: [PATCH] tpm/eventlog: Use kvmalloc() for event log buffer

["Jarkko Sakkinen" <jarkko@kernel.org>]

On Thu Nov 7, 2024 at 1:18 PM EET, Takashi Iwai wrote:
> 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
> 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.


TPM2 ACPI table is data structure ;-) Just to make the commit message
less confusing, please refer to active actors.


>
> 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().
>
> Reported-and-tested-by: Andy Liang <andy.liang@hpe.com>
> Cc: jenifer.golmitz@hpe.com
> Link: https://bugzilla.suse.com/show_bug.cgi?id=1232421

"You are not authorized to access bug #1232421. To see this bug, you must
first log in to an account with the appropriate permissions."

Please remove  this link as it gives no information without login
access, *or* make it available w/o acocunt, *or* repost a bug to the
kernel bugzilla.

I've been cursing SUSE accounts for over a year now. Never been able
to successfully get either to the bugzilla or forums (still I get
some weekly spam about the forums). And no, no interest to recall
or figure out this problem.

> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  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;

How big is it?

> +	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;
>  }

BR, Jarkko