Re: [PATCH v5 4/5] tpm: Allocate chip->auth in tpm2_start_auth_session()

["Jarkko Sakkinen" <jarkko@kernel.org>]

On Tue Sep 24, 2024 at 4:33 PM EEST, James Bottomley wrote:
> On Sat, 2024-09-21 at 15:08 +0300, Jarkko Sakkinen wrote:
> > Move allocation of chip->auth to tpm2_start_auth_session() so that
> > the field can be used as flag to tell whether auth session is active
> > or not.
> > 
> > Cc: stable@vger.kernel.org # v6.10+
> > Fixes: 699e3efd6c64 ("tpm: Add HMAC session start and end functions")
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > ---
> > v5:
> > - No changes.
> > v4:
> > - Change to bug.
> > v3:
> > - No changes.
> > v2:
> > - A new patch.
> > ---
> >  drivers/char/tpm/tpm2-sessions.c | 43 +++++++++++++++++++-----------
> > --
> >  1 file changed, 25 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/tpm2-sessions.c
> > b/drivers/char/tpm/tpm2-sessions.c
> > index 1aef5b1f9c90..a8d3d5d52178 100644
> > --- a/drivers/char/tpm/tpm2-sessions.c
> > +++ b/drivers/char/tpm/tpm2-sessions.c
> > @@ -484,7 +484,8 @@ static void tpm2_KDFe(u8 z[EC_PT_SZ], const char
> > *str, u8 *pt_u, u8 *pt_v,
> >         sha256_final(&sctx, out);
> >  }
> >  
> > -static void tpm_buf_append_salt(struct tpm_buf *buf, struct tpm_chip
> > *chip)
> > +static void tpm_buf_append_salt(struct tpm_buf *buf, struct tpm_chip
> > *chip,
> > +                               struct tpm2_auth *auth)
>
> This addition of auth as an argument is a bit unnecessary.  You can set
> chip->auth before calling this and it will all function.  Since there's
> no error leg in tpm2_start_auth_session unless the session creation
> itself fails and the guarantee of the ops lock is single threading this
> chip->auth can be nulled again in that error leg.
>
> If you want to keep the flow proposed in the patch, the change from how
> it works now to how it works with this patch needs documenting in the
> change log

I checked this through and have to disagree with it. We don't want
to set chip->auth before the whole start auth session is successful

BR, Jarkko