linux-kernel - Re: [PATCH v5] tpm: Managed allocations for tpm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <be1c5bef-7c97-4173-b417-986dc90d779c@linux.ibm.com>
Date: Thu, 3 Jul 2025 16:21:05 -0400
From: Stefan Berger <stefanb@...ux.ibm.com>
To: Jarkko Sakkinen <jarkko@...nel.org>, linux-kernel@...r.kernel.org
Cc: keyrings@...r.kernel.org, Jarkko Sakkinen <jarkko.sakkinen@...nsys.com>,
        Peter Huewe <peterhuewe@....de>, Jason Gunthorpe <jgg@...pe.ca>,
        James Bottomley <James.Bottomley@...senPartnership.com>,
        Mimi Zohar <zohar@...ux.ibm.com>, David Howells <dhowells@...hat.com>,
        Paul Moore <paul@...l-moore.com>, James Morris <jmorris@...ei.org>,
        "Serge E. Hallyn" <serge@...lyn.com>,
        "open list:TPM DEVICE DRIVER" <linux-integrity@...r.kernel.org>,
        "open list:SECURITY SUBSYSTEM" <linux-security-module@...r.kernel.org>
Subject: Re: [PATCH v5] tpm: Managed allocations for tpm_buf instances



On 7/3/25 2:17 PM, Jarkko Sakkinen wrote:
> From: Jarkko Sakkinen <jarkko.sakkinen@...nsys.com>
> 
> Repeal and replace tpm_buf_init() and tpm_buf_init_sized() with
> tpm_buf_alloc(), which returns a buffer of  memory with the struct tpm_buf
> header at the beginning of the returned buffer. This leaves 4092 bytes of
> free space for the payload.
> 
> Given that kfree() becomes the destructor for struct tpm_buf instances,
> tpm_buf_destroy() is now obsolete, and can be removed.
> 
> The actual gist is that a struct tpm_buf instance can be declared using
> __free(kfree) from linux/slab.h:
> 
> 	struct tpm_buf *buf __free(kfree) buf = tpm_buf_alloc();
> 
> Doing this has two-folded benefits associated with struct tpm_buf:
> 
> 1. New features will not introduce memory leaks.
> 2. It addresses undiscovered memory leaks.
> 
> In addition, the barrier to contribute is lowered given that managing
> memory is a factor easier.
> 
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@...nsys.com>
> ---

> @@ -374,20 +362,18 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
>    */
>   void tpm2_flush_context(struct tpm_chip *chip, u32 handle)
>   {
> -	struct tpm_buf buf;
> -	int rc;
> +	struct tpm_buf *buf __free(kfree) = tpm_buf_alloc();
 >

Remove empty line?

> -	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
> -	if (rc) {
> +	if (!buf) {
>   		dev_warn(&chip->dev, "0x%08x was not flushed, out of memory\n",
>   			 handle);
>   		return;
>   	}
>   
> -	tpm_buf_append_u32(&buf, handle);
> +	tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
> +	tpm_buf_append_u32(buf, handle);
>   
> -	tpm_transmit_cmd(chip, &buf, 0, "flushing context");
> -	tpm_buf_destroy(&buf);
> +	tpm_transmit_cmd(chip, buf, 0, "flushing context");
>   }
>   EXPORT_SYMBOL_GPL(tpm2_flush_context);
>   
> @@ -414,19 +400,20 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 *value,
>   			const char *desc)
>   {
>   	struct tpm2_get_cap_out *out;
> -	struct tpm_buf buf;
>   	int rc;
>   
> -	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
> -	if (rc)
> -		return rc;
> -	tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES);
> -	tpm_buf_append_u32(&buf, property_id);
> -	tpm_buf_append_u32(&buf, 1);
> -	rc = tpm_transmit_cmd(chip, &buf, 0, NULL);
> +	struct tpm_buf *buf __free(kfree) = tpm_buf_alloc();
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
> +	tpm_buf_append_u32(buf, TPM2_CAP_TPM_PROPERTIES);
> +	tpm_buf_append_u32(buf, property_id);
> +	tpm_buf_append_u32(buf, 1);
> +	rc = tpm_transmit_cmd(chip, buf, 0, NULL);
>   	if (!rc) {
>   		out = (struct tpm2_get_cap_out *)
> -			&buf.data[TPM_HEADER_SIZE];
> +			&buf->data[TPM_HEADER_SIZE];
>   		/*
>   		 * To prevent failing boot up of some systems, Infineon TPM2.0
>   		 * returns SUCCESS on TPM2_Startup in field upgrade mode. Also
> @@ -438,7 +425,6 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 *value,
>   		else
>   			rc = -ENODATA;
>   	}
> -	tpm_buf_destroy(&buf);
>   	return rc;
>   }
>   EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
> @@ -455,15 +441,14 @@ EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
>    */
>   void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
>   {
> -	struct tpm_buf buf;
> -	int rc;
> +	struct tpm_buf *buf __free(kfree) = tpm_buf_alloc();
>   

Remove empty line here.

With this nit fixed:
Reviewed-by: Stefan Berger <stefanb@...ux.ibm.com>