[PATCH] tpm: acpi: Fix memory leak at tpm_read_log_acpi() error path

[PATCH] tpm: acpi: Fix memory leak at tpm_read_log_acpi() error path

[Takashi Iwai]

When devm_add_action() returns an error, we do NULL-clear
log->bios_event_log unnecessarily before calling tpm_bios_log_free(),
and it would lead to a memory leak of the allocated log buffer.

Fix it by simply jumping to the err.

Fixes: a3a860bc0fd6 ("tpm: Change to kvalloc() in eventlog/acpi.c")
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/char/tpm/eventlog/acpi.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/char/tpm/eventlog/acpi.c b/drivers/char/tpm/eventlog/acpi.c
index cf02ec646f46..50770cafa835 100644
--- a/drivers/char/tpm/eventlog/acpi.c
+++ b/drivers/char/tpm/eventlog/acpi.c
@@ -167,10 +167,8 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
 	}
 
 	ret = devm_add_action(&chip->dev, tpm_bios_log_free, log->bios_event_log);
-	if (ret) {
-		log->bios_event_log = NULL;
+	if (ret)
 		goto err;
-	}
 
 	return format;
 
-- 
2.53.0

Re: [PATCH] tpm: acpi: Fix memory leak at tpm_read_log_acpi() error path

[Jarkko Sakkinen]

On Mon, Nov 09, 2026 at 09:25:24AM +0100, Takashi Iwai wrote:
> When devm_add_action() returns an error, we do NULL-clear
> log->bios_event_log unnecessarily before calling tpm_bios_log_free(),
> and it would lead to a memory leak of the allocated log buffer.

I don't understand this. "NULL-clear" is gibberish when the actual
bug is just spurious clear of log->bios_event_log. Also we are not
doing anything we are not computers.


> 
> Fix it by simply jumping to the err.
> 
> Fixes: a3a860bc0fd6 ("tpm: Change to kvalloc() in eventlog/acpi.c")
> Cc: <stable@vger.kernel.org>

Put Cc before fixes.

> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  drivers/char/tpm/eventlog/acpi.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/char/tpm/eventlog/acpi.c b/drivers/char/tpm/eventlog/acpi.c
> index cf02ec646f46..50770cafa835 100644
> --- a/drivers/char/tpm/eventlog/acpi.c
> +++ b/drivers/char/tpm/eventlog/acpi.c
> @@ -167,10 +167,8 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
>  	}
>  
>  	ret = devm_add_action(&chip->dev, tpm_bios_log_free, log->bios_event_log);
> -	if (ret) {
> -		log->bios_event_log = NULL;
> +	if (ret)
>  		goto err;
> -	}
>  
>  	return format;
>  
> -- 
> 2.53.0
> 

Br, Jarkko

Re: [PATCH] tpm: acpi: Fix memory leak at tpm_read_log_acpi() error path

[Jarkko Sakkinen]

On Mon, Nov 09, 2026 at 05:25:49PM +0200, Jarkko Sakkinen wrote:
> On Mon, Nov 09, 2026 at 09:25:24AM +0100, Takashi Iwai wrote:
> > When devm_add_action() returns an error, we do NULL-clear
> > log->bios_event_log unnecessarily before calling tpm_bios_log_free(),
> > and it would lead to a memory leak of the allocated log buffer.
> 
> I don't understand this. "NULL-clear" is gibberish when the actual
> bug is just spurious clear of log->bios_event_log. Also we are not
> doing anything we are not computers.

I.e. in place of "we", use the name of the function and this should
be good. The fix itself is legit.

Br, Jarkko

Re: [PATCH] tpm: acpi: Fix memory leak at tpm_read_log_acpi() error path

[Takashi Iwai]

On Mon, 09 Nov 2026 16:25:45 +0100,
Jarkko Sakkinen wrote:
> 
> On Mon, Nov 09, 2026 at 09:25:24AM +0100, Takashi Iwai wrote:
> > When devm_add_action() returns an error, we do NULL-clear
> > log->bios_event_log unnecessarily before calling tpm_bios_log_free(),
> > and it would lead to a memory leak of the allocated log buffer.
> 
> I don't understand this. "NULL-clear" is gibberish when the actual
> bug is just spurious clear of log->bios_event_log.

It's not "spurious", but rather ill-behaving.
Because it sets NULL before calling tpm_bios_log_free(), the actually
allocated buffer won't be freed at all.


Takashi

Re: [PATCH] tpm: acpi: Fix memory leak at tpm_read_log_acpi() error path

[Jarkko Sakkinen]

On Mon, Nov 09, 2026 at 04:55:03PM +0100, Takashi Iwai wrote:
> On Mon, 09 Nov 2026 16:25:45 +0100,
> Jarkko Sakkinen wrote:
> > 
> > On Mon, Nov 09, 2026 at 09:25:24AM +0100, Takashi Iwai wrote:
> > > When devm_add_action() returns an error, we do NULL-clear
> > > log->bios_event_log unnecessarily before calling tpm_bios_log_free(),
> > > and it would lead to a memory leak of the allocated log buffer.
> > 
> > I don't understand this. "NULL-clear" is gibberish when the actual
> > bug is just spurious clear of log->bios_event_log.
> 
> It's not "spurious", but rather ill-behaving.
> Because it sets NULL before calling tpm_bios_log_free(), the actually
> allocated buffer won't be freed at all.

I agree. That was wrong choice of the word from my side :-)

Since in this case it simple, I'd just want to apples and oranges
pointed out, that's all.


> 
> 
> Takashi

Br, Jarkko