Re: [PATCH v5] tpm: Managed allocations for tpm_buf instances

[Stefan Berger <stefanb@linux.ibm.com>]

On 7/3/25 2:17 PM, Jarkko Sakkinen wrote:
> From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.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@opinsys.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@linux.ibm.com>