* [PATCH v2 1/3] tpm: acpi: Call acpi_put_table() to fix memory leak
2022-11-17 11:23 [PATCH v2 0/3] ACPI table release for TPM drivers Hanjun Guo
@ 2022-11-17 11:23 ` Hanjun Guo
2022-11-27 17:18 ` Jarkko Sakkinen
2022-11-17 11:23 ` [PATCH v2 2/3] tpm: tpm_crb: Add the missed " Hanjun Guo
2022-11-17 11:23 ` [PATCH v2 3/3] tpm: tpm_tis: " Hanjun Guo
2 siblings, 1 reply; 7+ messages in thread
From: Hanjun Guo @ 2022-11-17 11:23 UTC (permalink / raw)
To: Jarkko Sakkinen, Peter Huewe; +Cc: linux-integrity, linux-kernel, Hanjun Guo
The start and length of the event log area are obtained from
TPM2 or TCPA table, so we call acpi_get_table() to get the
ACPI information, but the acpi_get_table() should be coupled with
acpi_put_table() to release the ACPI memory, add the acpi_put_table()
properly to fix the memory leak.
While we are at it, remove the redundant empty line at the
end of the tpm_read_log_acpi().
Fixes: 0bfb23746052 ("tpm: Move eventlog files to a subdirectory")
Fixes: 85467f63a05c ("tpm: Add support for event log pointer found in TPM2 ACPI table")
Cc: stable@vger.kernel.org
Signed-off-by: Hanjun Guo <guohanjun@huawei.com>
---
drivers/char/tpm/eventlog/acpi.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/char/tpm/eventlog/acpi.c b/drivers/char/tpm/eventlog/acpi.c
index 1b18ce5..0913d3eb 100644
--- a/drivers/char/tpm/eventlog/acpi.c
+++ b/drivers/char/tpm/eventlog/acpi.c
@@ -90,16 +90,21 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
return -ENODEV;
if (tbl->header.length <
- sizeof(*tbl) + sizeof(struct acpi_tpm2_phy))
+ sizeof(*tbl) + sizeof(struct acpi_tpm2_phy)) {
+ acpi_put_table((struct acpi_table_header *)tbl);
return -ENODEV;
+ }
tpm2_phy = (void *)tbl + sizeof(*tbl);
len = tpm2_phy->log_area_minimum_length;
start = tpm2_phy->log_area_start_address;
- if (!start || !len)
+ if (!start || !len) {
+ acpi_put_table((struct acpi_table_header *)tbl);
return -ENODEV;
+ }
+ acpi_put_table((struct acpi_table_header *)tbl);
format = EFI_TCG2_EVENT_LOG_FORMAT_TCG_2;
} else {
/* Find TCPA entry in RSDT (ACPI_LOGICAL_ADDRESSING) */
@@ -120,8 +125,10 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
break;
}
+ acpi_put_table((struct acpi_table_header *)buff);
format = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2;
}
+
if (!len) {
dev_warn(&chip->dev, "%s: TCPA log area empty\n", __func__);
return -EIO;
@@ -156,5 +163,4 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
kfree(log->bios_event_log);
log->bios_event_log = NULL;
return ret;
-
}
--
1.7.12.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2 1/3] tpm: acpi: Call acpi_put_table() to fix memory leak
2022-11-17 11:23 ` [PATCH v2 1/3] tpm: acpi: Call acpi_put_table() to fix memory leak Hanjun Guo
@ 2022-11-27 17:18 ` Jarkko Sakkinen
0 siblings, 0 replies; 7+ messages in thread
From: Jarkko Sakkinen @ 2022-11-27 17:18 UTC (permalink / raw)
To: Hanjun Guo; +Cc: Peter Huewe, linux-integrity, linux-kernel
On Thu, Nov 17, 2022 at 07:23:40PM +0800, Hanjun Guo wrote:
> The start and length of the event log area are obtained from
> TPM2 or TCPA table, so we call acpi_get_table() to get the
> ACPI information, but the acpi_get_table() should be coupled with
> acpi_put_table() to release the ACPI memory, add the acpi_put_table()
> properly to fix the memory leak.
>
> While we are at it, remove the redundant empty line at the
> end of the tpm_read_log_acpi().
>
> Fixes: 0bfb23746052 ("tpm: Move eventlog files to a subdirectory")
> Fixes: 85467f63a05c ("tpm: Add support for event log pointer found in TPM2 ACPI table")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hanjun Guo <guohanjun@huawei.com>
> ---
> drivers/char/tpm/eventlog/acpi.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/char/tpm/eventlog/acpi.c b/drivers/char/tpm/eventlog/acpi.c
> index 1b18ce5..0913d3eb 100644
> --- a/drivers/char/tpm/eventlog/acpi.c
> +++ b/drivers/char/tpm/eventlog/acpi.c
> @@ -90,16 +90,21 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
> return -ENODEV;
>
> if (tbl->header.length <
> - sizeof(*tbl) + sizeof(struct acpi_tpm2_phy))
> + sizeof(*tbl) + sizeof(struct acpi_tpm2_phy)) {
> + acpi_put_table((struct acpi_table_header *)tbl);
> return -ENODEV;
> + }
>
> tpm2_phy = (void *)tbl + sizeof(*tbl);
> len = tpm2_phy->log_area_minimum_length;
>
> start = tpm2_phy->log_area_start_address;
> - if (!start || !len)
> + if (!start || !len) {
> + acpi_put_table((struct acpi_table_header *)tbl);
> return -ENODEV;
> + }
>
> + acpi_put_table((struct acpi_table_header *)tbl);
> format = EFI_TCG2_EVENT_LOG_FORMAT_TCG_2;
> } else {
> /* Find TCPA entry in RSDT (ACPI_LOGICAL_ADDRESSING) */
> @@ -120,8 +125,10 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
> break;
> }
>
> + acpi_put_table((struct acpi_table_header *)buff);
> format = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2;
> }
> +
> if (!len) {
> dev_warn(&chip->dev, "%s: TCPA log area empty\n", __func__);
> return -EIO;
> @@ -156,5 +163,4 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
> kfree(log->bios_event_log);
> log->bios_event_log = NULL;
> return ret;
> -
> }
> --
> 1.7.12.4
>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
BR, Jarkko
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 2/3] tpm: tpm_crb: Add the missed acpi_put_table() to fix memory leak
2022-11-17 11:23 [PATCH v2 0/3] ACPI table release for TPM drivers Hanjun Guo
2022-11-17 11:23 ` [PATCH v2 1/3] tpm: acpi: Call acpi_put_table() to fix memory leak Hanjun Guo
@ 2022-11-17 11:23 ` Hanjun Guo
2022-11-27 17:19 ` Jarkko Sakkinen
2022-11-17 11:23 ` [PATCH v2 3/3] tpm: tpm_tis: " Hanjun Guo
2 siblings, 1 reply; 7+ messages in thread
From: Hanjun Guo @ 2022-11-17 11:23 UTC (permalink / raw)
To: Jarkko Sakkinen, Peter Huewe; +Cc: linux-integrity, linux-kernel, Hanjun Guo
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@vger.kernel.org
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2 2/3] tpm: tpm_crb: Add the missed acpi_put_table() to fix memory leak
2022-11-17 11:23 ` [PATCH v2 2/3] tpm: tpm_crb: Add the missed " Hanjun Guo
@ 2022-11-27 17:19 ` Jarkko Sakkinen
0 siblings, 0 replies; 7+ messages in thread
From: Jarkko Sakkinen @ 2022-11-27 17:19 UTC (permalink / raw)
To: Hanjun Guo; +Cc: Peter Huewe, linux-integrity, linux-kernel
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@vger.kernel.org
> 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
>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
BR, Jarkko
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 3/3] tpm: tpm_tis: Add the missed acpi_put_table() to fix memory leak
2022-11-17 11:23 [PATCH v2 0/3] ACPI table release for TPM drivers Hanjun Guo
2022-11-17 11:23 ` [PATCH v2 1/3] tpm: acpi: Call acpi_put_table() to fix memory leak Hanjun Guo
2022-11-17 11:23 ` [PATCH v2 2/3] tpm: tpm_crb: Add the missed " Hanjun Guo
@ 2022-11-17 11:23 ` Hanjun Guo
2022-11-27 17:21 ` Jarkko Sakkinen
2 siblings, 1 reply; 7+ messages in thread
From: Hanjun Guo @ 2022-11-17 11:23 UTC (permalink / raw)
To: Jarkko Sakkinen, Peter Huewe; +Cc: linux-integrity, linux-kernel, Hanjun Guo
In check_acpi_tpm2(), we get the TPM2 table just to make
sure the table is there, not used after the init, so the
acpi_put_table() should be added to release the ACPI memory.
Fixes: 4cb586a188d4 ("tpm_tis: Consolidate the platform and acpi probe flow")
Cc: stable@vger.kernel.org
Signed-off-by: Hanjun Guo <guohanjun@huawei.com>
---
drivers/char/tpm/tpm_tis.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index bcff642..ed5dabd 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -125,6 +125,7 @@ static int check_acpi_tpm2(struct device *dev)
const struct acpi_device_id *aid = acpi_match_device(tpm_acpi_tbl, dev);
struct acpi_table_tpm2 *tbl;
acpi_status st;
+ int ret = 0;
if (!aid || aid->driver_data != DEVICE_IS_TPM2)
return 0;
@@ -132,8 +133,7 @@ static int check_acpi_tpm2(struct device *dev)
/* If the ACPI TPM2 signature is matched then a global ACPI_SIG_TPM2
* table is mandatory
*/
- st =
- acpi_get_table(ACPI_SIG_TPM2, 1, (struct acpi_table_header **)&tbl);
+ st = acpi_get_table(ACPI_SIG_TPM2, 1, (struct acpi_table_header **)&tbl);
if (ACPI_FAILURE(st) || tbl->header.length < sizeof(*tbl)) {
dev_err(dev, FW_BUG "failed to get TPM2 ACPI table\n");
return -EINVAL;
@@ -141,9 +141,10 @@ static int check_acpi_tpm2(struct device *dev)
/* The tpm2_crb driver handles this device */
if (tbl->start_method != ACPI_TPM2_MEMORY_MAPPED)
- return -ENODEV;
+ ret = -ENODEV;
- return 0;
+ acpi_put_table((struct acpi_table_header *)tbl);
+ return ret;
}
#else
static int check_acpi_tpm2(struct device *dev)
--
1.7.12.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2 3/3] tpm: tpm_tis: Add the missed acpi_put_table() to fix memory leak
2022-11-17 11:23 ` [PATCH v2 3/3] tpm: tpm_tis: " Hanjun Guo
@ 2022-11-27 17:21 ` Jarkko Sakkinen
0 siblings, 0 replies; 7+ messages in thread
From: Jarkko Sakkinen @ 2022-11-27 17:21 UTC (permalink / raw)
To: Hanjun Guo; +Cc: Peter Huewe, linux-integrity, linux-kernel
On Thu, Nov 17, 2022 at 07:23:42PM +0800, Hanjun Guo wrote:
> In check_acpi_tpm2(), we get the TPM2 table just to make
> sure the table is there, not used after the init, so the
> acpi_put_table() should be added to release the ACPI memory.
>
> Fixes: 4cb586a188d4 ("tpm_tis: Consolidate the platform and acpi probe flow")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hanjun Guo <guohanjun@huawei.com>
> ---
> drivers/char/tpm/tpm_tis.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
> index bcff642..ed5dabd 100644
> --- a/drivers/char/tpm/tpm_tis.c
> +++ b/drivers/char/tpm/tpm_tis.c
> @@ -125,6 +125,7 @@ static int check_acpi_tpm2(struct device *dev)
> const struct acpi_device_id *aid = acpi_match_device(tpm_acpi_tbl, dev);
> struct acpi_table_tpm2 *tbl;
> acpi_status st;
> + int ret = 0;
>
> if (!aid || aid->driver_data != DEVICE_IS_TPM2)
> return 0;
> @@ -132,8 +133,7 @@ static int check_acpi_tpm2(struct device *dev)
> /* If the ACPI TPM2 signature is matched then a global ACPI_SIG_TPM2
> * table is mandatory
> */
> - st =
> - acpi_get_table(ACPI_SIG_TPM2, 1, (struct acpi_table_header **)&tbl);
> + st = acpi_get_table(ACPI_SIG_TPM2, 1, (struct acpi_table_header **)&tbl);
> if (ACPI_FAILURE(st) || tbl->header.length < sizeof(*tbl)) {
> dev_err(dev, FW_BUG "failed to get TPM2 ACPI table\n");
> return -EINVAL;
> @@ -141,9 +141,10 @@ static int check_acpi_tpm2(struct device *dev)
>
> /* The tpm2_crb driver handles this device */
> if (tbl->start_method != ACPI_TPM2_MEMORY_MAPPED)
> - return -ENODEV;
> + ret = -ENODEV;
>
> - return 0;
> + acpi_put_table((struct acpi_table_header *)tbl);
> + return ret;
> }
> #else
> static int check_acpi_tpm2(struct device *dev)
> --
> 1.7.12.4
>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
BR, Jarkko
^ permalink raw reply [flat|nested] 7+ messages in thread