Re: [PATCH v5 8/8] KEYS: trusted: tpm2: Use struct tpm_buf for sized buffers

James Bottomley
2023-11-28T12:24:28Z

On Mon, 2023-11-27 at 21:48 -0600, Serge E. Hallyn wrote:
> On Wed, Nov 22, 2023 at 12:31:20AM +0200, Jarkko Sakkinen wrote:
[...]
> > diff --git a/security/keys/trusted-keys/trusted_tpm2.c
> > b/security/keys/trusted-keys/trusted_tpm2.c
> > index bc700f85f80b..97b1dfca2dba 100644
> > --- a/security/keys/trusted-keys/trusted_tpm2.c
> > +++ b/security/keys/trusted-keys/trusted_tpm2.c
> > @@ -228,8 +228,9 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> >                       struct trusted_key_payload *payload,
> >                       struct trusted_key_options *options)
> >  {
> > +       off_t offset = TPM_HEADER_SIZE;
> > +       struct tpm_buf buf, sized;
> >         int blob_len = 0;
> > -       struct tpm_buf buf;
> >         u32 hash;
> >         u32 flags;
> >         int i;
> > @@ -258,6 +259,14 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> >                 return rc;
> >         }
> >
> > +       rc = tpm_buf_init_sized(&sized);
> > +       if (rc) {
> > +               tpm_buf_destroy(&buf);
>
> It won't really hurt, but at the moment if tpm_buf_init_sized()
> returns non-zero, then it must be returning -ENOMEM, and
> tpm_buf_destroy(&buf) is not needed, right?

No ... buf was initialized further up in the original code (you seem to
be confusing buf and sized ... they're two separate allocations). We
can't return from here without destroying it otherwise we'll leak a
page.

James