Re: [PATCH v2 2/3] tpm: tpm_crb: Add the missed acpi_put_table() to fix memory leak

From: Jarkko Sakkinen
Date: Sun Nov 27 2022 - 12:19:36 EST


On Thu, Nov 17, 2022 at 07:23:41PM +0800, Hanjun Guo wrote:
> In crb_acpi_add(), we get the TPM2 table to retrieve information
> like start method, and then assign them to the priv data, so the
> TPM2 table is not used after the init, should be freed, call
> acpi_put_table() to fix the memory leak.
>
> Fixes: 30fc8d138e91 ("tpm: TPM 2.0 CRB Interface")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Hanjun Guo <guohanjun@xxxxxxxxxx>
> ---
> drivers/char/tpm/tpm_crb.c | 29 ++++++++++++++++++++---------
> 1 file changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index 1860665..5bfb00f 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -676,12 +676,16 @@ static int crb_acpi_add(struct acpi_device *device)
>
> /* Should the FIFO driver handle this? */
> sm = buf->start_method;
> - if (sm == ACPI_TPM2_MEMORY_MAPPED)
> - return -ENODEV;
> + if (sm == ACPI_TPM2_MEMORY_MAPPED) {
> + rc = -ENODEV;
> + goto out;
> + }
>
> priv = devm_kzalloc(dev, sizeof(struct crb_priv), GFP_KERNEL);
> - if (!priv)
> - return -ENOMEM;
> + if (!priv) {
> + rc = -ENOMEM;
> + goto out;
> + }
>
> if (sm == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC) {
> if (buf->header.length < (sizeof(*buf) + sizeof(*crb_smc))) {
> @@ -689,7 +693,8 @@ static int crb_acpi_add(struct acpi_device *device)
> FW_BUG "TPM2 ACPI table has wrong size %u for start method type %d\n",
> buf->header.length,
> ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC);
> - return -EINVAL;
> + rc = -EINVAL;
> + goto out;
> }
> crb_smc = ACPI_ADD_PTR(struct tpm2_crb_smc, buf, sizeof(*buf));
> priv->smc_func_id = crb_smc->smc_func_id;
> @@ -700,17 +705,23 @@ static int crb_acpi_add(struct acpi_device *device)
>
> rc = crb_map_io(device, priv, buf);
> if (rc)
> - return rc;
> + goto out;
>
> chip = tpmm_chip_alloc(dev, &tpm_crb);
> - if (IS_ERR(chip))
> - return PTR_ERR(chip);
> + if (IS_ERR(chip)) {
> + rc = PTR_ERR(chip);
> + goto out;
> + }
>
> dev_set_drvdata(&chip->dev, priv);
> chip->acpi_dev_handle = device->handle;
> chip->flags = TPM_CHIP_FLAG_TPM2;
>
> - return tpm_chip_register(chip);
> + rc = tpm_chip_register(chip);
> +
> +out:
> + acpi_put_table((struct acpi_table_header *)buf);
> + return rc;
> }
>
> static int crb_acpi_remove(struct acpi_device *device)
> --
> 1.7.12.4
>

Reviewed-by: Jarkko Sakkinen <jarkko@xxxxxxxxxx>

BR, Jarkko