On Thu, Jul 03, 2025 at 04:21:05PM -0400, Stefan Berger wrote:
>
>
> 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?
I recalled from the past that checkpatch.pl would complain if there was
no empty line after the declarations.
Now that I tested removing that line, it did not so I guess I can remove
that empty line. The presumed checkpatch error was the only reason for
having it.
>
> > - 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>
>
Thanks and I'm happy to fixup those. They did look also silly to me :-)
BR, Jarkko
