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

[Stefan Berger]

On 7/3/25 2:17 PM, Jarkko Sakkinen wrote:

From: Jarkko Sakkinen <jarkko.sakki...@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.sakki...@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 <stef...@linux.ibm.com>