On Wed Oct 30, 2024 at 5:47 PM EET, James Bottomley wrote: > On Mon, 2024-10-28 at 07:50 +0200, Jarkko Sakkinen wrote: > [...] > > --- a/drivers/char/tpm/tpm2-sessions.c > > +++ b/drivers/char/tpm/tpm2-sessions.c > > @@ -915,33 +915,37 @@ static int tpm2_parse_start_auth_session(struct > > tpm2_auth *auth, > > > > static int tpm2_load_null(struct tpm_chip *chip, u32 *null_key) > > { > > - int rc; > > unsigned int offset = 0; /* dummy offset for null seed > > context */ > > u8 name[SHA256_DIGEST_SIZE + 2]; > > + u32 tmp_null_key; > > + int rc; > > > > rc = tpm2_load_context(chip, chip->null_key_context, &offset, > > - null_key); > > - if (rc != -EINVAL) > > - return rc; > > + &tmp_null_key); > > + if (rc != -EINVAL) { > > + if (!rc) > > + *null_key = tmp_null_key; > > + goto err; > > + } > > > > - /* an integrity failure may mean the TPM has been reset */ > > - dev_err(&chip->dev, "NULL key integrity failure!\n"); > > - /* check the null name against what we know */ > > - tpm2_create_primary(chip, TPM2_RH_NULL, NULL, name); > > - if (memcmp(name, chip->null_key_name, sizeof(name)) == 0) > > - /* name unchanged, assume transient integrity failure > > */ > > - return rc; > > - /* > > - * Fatal TPM failure: the NULL seed has actually changed, so > > - * the TPM must have been illegally reset. All in-kernel TPM > > - * operations will fail because the NULL primary can't be > > - * loaded to salt the sessions, but disable the TPM anyway so > > - * userspace programmes can't be compromised by it. > > - */ > > - dev_err(&chip->dev, "NULL name has changed, disabling TPM due > > to interference\n"); > > + /* Try to re-create null key, given the integrity failure: */ > > + rc = tpm2_create_primary(chip, TPM2_RH_NULL, &tmp_null_key, > > name); > > + if (rc) > > + goto err; > > From a security point of view, this probably isn't such a good idea: > the reason the context load failed above is likely the security > condition we're checking for: the null seed changed because an > interposer did a reset. That means that if the interposer knows about > this error leg, it would simply error out the create primary here and > the TPM wouldn't be disabled. If you think there is something that should be still addressed, or there is overlooked issue please do send a patch, and we will review that. There's been plenty of time to comment on patches. Neither in previous TPM_CHIP_FLAG_DISABLED was set tpm2_load_context() failed. It went like this: rc = tpm2_load_context(chip, chip->null_key_context, &offset, null_key); if (rc != -EINVAL) return rc; If you think that this should be addressed, do send a patch but point out the fixes-tag to your original patch then. BR, Jarkko