On Wed Oct 23, 2024 at 10:15 PM EEST, Stefan Berger wrote:
>
>
> On 10/21/24 1:39 AM, 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@xxxxxxxxxxxxxxx # v6.10+
> > Fixes: 699e3efd6c64 ("tpm: Add HMAC session start and end functions")
> > Signed-off-by: Jarkko Sakkinen <jarkko@xxxxxxxxxx>
> > ---
> > 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 78c650ce4c9f..6e52785de9fd 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)
> > {
> > struct crypto_kpp *kpp;
> > struct kpp_request *req;
> > @@ -543,7 +544,7 @@ static void tpm_buf_append_salt(struct tpm_buf *buf, struct tpm_chip *chip)
> > sg_set_buf(&s[0], chip->null_ec_key_x, EC_PT_SZ);
> > sg_set_buf(&s[1], chip->null_ec_key_y, EC_PT_SZ);
> > kpp_request_set_input(req, s, EC_PT_SZ*2);
> > - sg_init_one(d, chip->auth->salt, EC_PT_SZ);
> > + sg_init_one(d, auth->salt, EC_PT_SZ);
> > kpp_request_set_output(req, d, EC_PT_SZ);
> > crypto_kpp_compute_shared_secret(req);
> > kpp_request_free(req);
> > @@ -554,8 +555,7 @@ static void tpm_buf_append_salt(struct tpm_buf *buf, struct tpm_chip *chip)
> > * This works because KDFe fully consumes the secret before it
> > * writes the salt
> > */
> > - tpm2_KDFe(chip->auth->salt, "SECRET", x, chip->null_ec_key_x,
> > - chip->auth->salt);
> > + tpm2_KDFe(auth->salt, "SECRET", x, chip->null_ec_key_x, auth->salt);
> >
> > out:
> > crypto_free_kpp(kpp);
> > @@ -854,6 +854,8 @@ int tpm_buf_check_hmac_response(struct tpm_chip *chip, struct tpm_buf *buf,
> > /* manually close the session if it wasn't consumed */
> > tpm2_flush_context(chip, auth->handle);
> > memzero_explicit(auth, sizeof(*auth));
> > + kfree(auth);
> > + chip->auth = NULL;
> > } else {
> > /* reset for next use */
> > auth->session = TPM_HEADER_SIZE;
> > @@ -882,6 +884,8 @@ void tpm2_end_auth_session(struct tpm_chip *chip)
> >
> > tpm2_flush_context(chip, auth->handle);
> > memzero_explicit(auth, sizeof(*auth));
> > + kfree(auth);
> > + chip->auth = NULL;
> > }
> > EXPORT_SYMBOL(tpm2_end_auth_session);
> >
> > @@ -970,25 +974,29 @@ static int tpm2_load_null(struct tpm_chip *chip, u32 *null_key)
> > */
> > int tpm2_start_auth_session(struct tpm_chip *chip)
> > {
> > + struct tpm2_auth *auth;
> > struct tpm_buf buf;
> > - struct tpm2_auth *auth = chip->auth;
> > - int rc;
> > u32 null_key;
> > + int rc;
> >
> > - if (!auth) {
> > - dev_warn_once(&chip->dev, "auth session is not active\n");
> > + if (chip->auth) {
> > + dev_warn_once(&chip->dev, "auth session is active\n");
> > return 0;
> > }
> >
> > + auth = kzalloc(sizeof(*auth), GFP_KERNEL);
> > + if (!auth)
> > + return -ENOMEM;
> > +
> > rc = tpm2_load_null(chip, &null_key);
> > if (rc)
> > - goto out;
> > + goto err;
> >
> > auth->session = TPM_HEADER_SIZE;
> >
> > rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_START_AUTH_SESS);
> > if (rc)
> > - goto out;
> > + goto err;
> >
> > /* salt key handle */
> > tpm_buf_append_u32(&buf, null_key);
> > @@ -1000,7 +1008,7 @@ int tpm2_start_auth_session(struct tpm_chip *chip)
> > tpm_buf_append(&buf, auth->our_nonce, sizeof(auth->our_nonce));
> >
> > /* append encrypted salt and squirrel away unencrypted in auth */
> > - tpm_buf_append_salt(&buf, chip);
> > + tpm_buf_append_salt(&buf, chip, auth);
> > /* session type (HMAC, audit or policy) */
> > tpm_buf_append_u8(&buf, TPM2_SE_HMAC);
> >
> > @@ -1021,10 +1029,13 @@ int tpm2_start_auth_session(struct tpm_chip *chip)
> >
> > tpm_buf_destroy(&buf);
> >
> > - if (rc)
> > - goto out;
> > + if (rc == TPM2_RC_SUCCESS) {
> > + chip->auth = auth;
> > + return 0;
> > + }
> >
> > - out:
> > +err:
>
> like in many other cases before kfree(auth):
> memzero_explicit(auth, sizeof(*auth));
>
> With this:
>
> Reviewed-by: Stefan Berger <stefanb@xxxxxxxxxxxxx>
Thanks, or should we use kfree_sensitive()?
It has some additional functionality, which is missed now:
https://elixir.bootlin.com/linux/v6.11.5/source/mm/slab_common.c#L1339
I.e. kasan_unpoison().
BR, Jarkko
