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

[Jarkko Sakkinen <jarkko@kernel.org>]

On Wed, Nov 09, 2022 at 12:03: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")
> Signed-off-by: Hanjun Guo <guohanjun@huawei.com>
> ---
>  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
> 

Same comments apply, as for the first patch.

BR, Jarkko