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

James Bottomley
2023-11-28T14:37:41Z

On Tue, 2023-11-28 at 08:34 -0600, Serge E. Hallyn wrote:
> On Tue, Nov 28, 2023 at 07:24:16AM -0500, James Bottomley wrote:
> > 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 ...
>
> You're right, I was.  Thanks.

No problem, it's actually an issue with reviewing patches. The
original code that inits buf isn't in the patch, so you only see one
init and one destroy and we mostly go by patterns.

James